4.042 improperly encoding blobs when sql_type is SQL_UNKNOWN_TYPE

[cthulhuology]

The utf8 encoding changes have resulted in a regression that has unexpected side effects.
Consider the following table:

create table foo (foo longblob);

and script:

use DBI;

my $dbh = DBI->connect('DBI:mysql:database=test','desktop','',{ mysql_enable_utf8 => 1 })
or die $DBI::errstr;

my $sth = $dbh->prepare('INSERT INTO foo (foo) values (?)');
$sth->execute("I18N Web Testing æøå");

my $rth = $dbh->prepare('SELECT foo FROM foo');
$rth->execute();
while (@Row = $rth->fetchrow_array) {
print $row[0],"\n";
}

Now the longblob mysql type should be considered a SQL_BLOB, but because the exec call doesn't call bind_param with the attrib set to SQL_BLOB, the default value of 0 is used SQL_UNKNOWN_TYPE and passed to bind_param. If you sv_dump at 939 in dbdimp.c you'll see:

SV = PV(0x162d950) at 0x15f8630
REFCNT = 1
FLAGS = (POK,IsCOW,READONLY,PROTECT,pPOK)
PV = 0x15dc200 "I18N Web Testing \303\246\303\270\303\245"\0
CUR = 23
LEN = 25
COW_REFCNT = 0

And sql_type is the default value of 0.

the output of the scrip will print the value stored in the database:

I18N Web Testing æøå`

because it is double encoding the characters, and not preserving the blob value because the
sql_type_is_binary check returns false because the SQL_UNKNOWN_TYPE value of sql_type.

One example of widely used code that exhibit this behavior is Apache::Session (Apache::Session::MySQL), which calls bind_param explicitly but never passes the SQL_BLOB at and of the call sites. Since Apache::Session is using a Storable (which is binary data stored in a blob) the additional UTF8 encoding can result in data corruption which can cause perl to crash with an OOM error when materialize is invoked. This is a potential security threat.

[pali]

The whole mysql_enable_utf8 was just experimental and totally broken prior to DBD::mysql 4.042. Experimental status was also written in documentation, see: https://metacpan.org/pod/release/CAPTTOFU/DBD-mysql-4.033/lib/DBD/mysql.pm#mysql_enable_utf8

I'm not sure what we can done here. Basically applications which trying to use experimental broken option provided by DBD::mysql < 4.042 and just broken.

Due to some fundamentals of perl scalars and also MySQL protocol itself, DBD::mysql does not type of bind parameter, so driver can use logic which you already described. When mysql_enable_utf8 is 0, then nothing is encoded/decoded and so treated as binary sequence of octets.

[Grinnz]

In other words: DBD::mysql does not know that a parameter is being used for insert into a blob column. Since you set mysql_enable_utf8 it will encode it, unless bound with the SQL_BLOB or similar type. On retrieval it knows that the column is a binary type, so it does not decode it. The behavior you describe only worked accidentally before.

[ap]

The whole mysql_enable_utf8 was just experimental and totally broken prior to DBD::mysql 4.042.

I used it for 10 years without problems. I’m sure I would have had problems if I had tried things where the incongruity of its design got exposed, but I never needed to. So my code had no bugs – even though DBD::mysql was designed incorrectly.

Worse: http://grep.cpan.me/?q=mysql_enable_utf8+file%3A.pm There’s plenty of infrastructure-level code just on CPAN alone which sets this flag on behalf of the user.

In the face of these realities, the fact that the documentation declared the feature experimental back when it was introduced, 11 years ago, is simply irrelevant today.

But now mysql_enable_utf8 is fixed – I‘m not going argue about that. But users’ code that wasn’t broken before now is. And this was simply unnecessary.

[pali]

So... what do you suggest? As wrote I have no idea what can be done here. Probably the best solution would be to fix broken code.

And about mysql_enable_utf8 usage on cpan, you do not know how many of those modules are broken and how many expect that mysql_enable_utf8 is not buggy? If module is multi-dbd and set mysql_enable_utf8, pg_enable_utf8 and also sqlite_unicode then it expects that mysql_enable_utf8 is working (which means needs DBD::mysql >= 4.042).

[ap]

Of course many of these CPAN modules will produce broken results when used in any somewhat unusual configuration where the broken design actually matters. But a lot of people don’t try that, and for them it ends up not mattering that the design is not sound. For these people, there is no gain from fixing the broken design, it just creates busywork.

I’m not sure what I would suggest, specifically. The general idea is always the same though: don’t change how mysql_enable_utf8 works – for now at least –, but add the more correct encoding approach, and then give users a way to ask for it.

How exactly you do that depends on the code and what you need to achieve long term. New flag e.g. mysql_enable_encoding? New flag value e.g. mysql_enable_utf8 => 2? Some other approach maybe? Also, do you warn when mysql_enable_utf8 is used? Maybe detect problematic uses and only warn then? Depends on what’s easier to implement and cleaner as an interface. Overall I would weigh that choice based on how difficult it is to support both the old and new thing long-term. The easier it is to keep the old stuff around, the less old users should be pushed away from it.

(Of course when it comes to the docs, the old approach should be de-emphasised, and the new way should be shown as the obvious/standard way of doing things, to steer new users in that direction. Also, to get people away from the old way it is important to make sure that the new way doesn’t make too many things harder to do than the old way – esp. commonly needed things. Note however the old way must stay documented – because there is still code that uses it, and people who have to maintain that code need to be able to figure out what it does.)

Something along these lines.

[pali]

Problem with old mysql_enable_utf8 implementation is that it behave "pseudo-randomly". For frozen combination of perl at specific version, all cpan modules at specific version and user application it acts deterministic. But once either perl or any cpan module is updated old mysql_enable_utf8 (prior to 4.042) can change result of DBD::mysql. So result is basically unpredictable.

If users (and authors of other modules) are not going to fix broken code and need current behavior they should stay at specific version of perl and all cpan modules. Otherwise it is not guaranteed that also old DBD::mysql version (prior to 4.042) provide same results. It can lead to results: after updating cpan module XYZ to new version MySQL server in user application started returning wrong result, where XYZ can be module which has nothing to do with MySQL.

Supporting such pseudo-random behavior is not easy and funny.

And I really suggest to conserve such fragile code and disallow updating code around (which can fully break it).

What is probably more important: how to write mysql_enable_utf8 code which tries to be compatible with both old buggy DBD::mysql versions and new.

I created this pull request which adds "hacks" into documentation: #119

Please comment, test or provide another suggestion for it. To improve situation.

[ap]

But once either perl or any cpan module is updated old mysql_enable_utf8 (prior to 4.042) can change result of DBD::mysql.

Any CPAN module? How?

[Grinnz]

If a CPAN module provides a string with a different internal representation, even if it's the same string, previous versions of DBD::mysql will treat it differently. We had this issue with output from Spreadsheet::ParseExcel that returned (perfectly normal) un-upgraded strings with byte values between 128 and 255, probably due to some dependency, causing errors when used as params to DBD::mysql with mysql_enable_utf8 set, as the internal representation it used was not UTF-8 encoded. Any module that provides you with a string could change what representation it gives you and break DBD::mysql while the string value is unchanged.

A more complete discussion of the issue is in https://rt.cpan.org/Ticket/Display.html?id=87428

[pali]

@Grinnz is right. And moreover un-upgraded strings are created by default from string constants if you do not use utf8 in your file from which those strings come from. Also some older perl versions generate un-upgraded string even if \N{U+XX} construction is used. And any perl function can upgrade or downgrade string as needed.

So please look at pull request #119 if it helps you or not...

[ap]

@Grinnz:

If a CPAN module provides a string with a different internal representation, even if it's the same string, previous versions of DBD::mysql will treat it differently.

Yeah, but that isn’t DBD::mysql changing its behaviour. That’s some other module changing its behaviour. If the user of both modules compensates for the change in the other module, DBD::mysql will get the same inputs as before, and will behave the same way as before. The behaviour of DBD::mysql is not changing, just because of an upgrade to some other module.

It’s perfectly predictable what it does, even if it’s not correct. “When the inputs change, the behaviour changes; when the inputs are the same, the behaviour is the same” is the opposite of random.

A more complete discussion of the issue is in https://rt.cpan.org/Ticket/Display.html?id=87428

Funny, there’s an almost verbatim precursor of my suggestion (but better thought out) already in there. As far as I can tell, doing it that would have worked just fine… with no breakage. Taking that approach would have given you what you wanted, without causing anyone else any pain. So why choose to cause pain when you don’t even gain anything from it?

@pali: So…

Supporting such pseudo-random behavior is not easy and funny.

… if this meant what @Grinnz was talking about, then I disagree that that behaviour is random (even just pseudo), I disagree that it’s hard to support solely on that basis, and I disagree about what’s not funny here (namely, changing it).

So please look at pull request #119 if it helps you or not...

That is indeed useful information in order to cope with the problems of the old interface. But I don’t see anything in there which would argue against adding the correct interface under a different flag – rather than changing mysql_enable_utf8 and thereby breaking already-working code.

Has there been any technical argument for not using a different flag? So far, all I have seen are arguments about the correctness of not using the UTF8 flag – which I do not disagree with at all.

[pali]

That’s some other module changing its behaviour.

I cannot agree here with you, because calling utf8::upgrade() or composing string via \x{...}\x{...} sequences, or via \N{...} is not behavior change. It is same behavior with exactly same function API.

What is changing here is low level representation, and you can compare it compiling program with different optimization flags, e.g. -O0 or -O2 in gcc. In both cases you get same program, just compiled with different assembler instructions.

And in perl usage of utf8::upgrade() or \N{...} is similar behavior. Perl just translate source file to different opcodes, but meaning of program is same...

Note that perl operators like . (dot, concatenation) or perl functions like substr calls utf8::upgrade() in some cases. And there is no guarantee that due to optimization in future it would not be changed (under which conditions is utf8::upgrade() called).

I wrote it is pseudo-random (not fully-random), which means it is deterministic. So I could agree with you that behavior can be predicted.

Originally I talk with DBD::mysql maintainers (half a year ago?) and wrote first proposed solution that introduction of new attribute for utf8 with hacking code around with supporting both of them can be possible. But IIRC we dismiss it due to introduction of big mess in code and reason that current behaviour does not make sense in perl.

Due to above fact and also that calling utf8::upgrade() is legal and already done by perl itself at lot of places and also that in documentation was explicitely written "experimental and may change" state, I introduced patches which fixes mysql_enable_utf8, rather then having two different UTF-8 supports... Note that I explicitely stated in pull request #67 that it would change behavior for more then month in review state and about 3 months there was on cpan engineering version. So I though it would be OK as nobody was against.

[sartak]

Originally I talk with DBD::mysql maintainers (half a year ago?) and wrote first proposed solution that
introduction of new attribute for utf8 with hacking code around with supporting both of them can be
possible. But IIRC we dismiss it due to introduction of big mess in code and reason that current
behaviour does not make sense in perl.

Please reconsider this stance. In as much as the new behavior is "correct", the old behavior was widely deployed and has a tremendous amount of inertia. Migrating to the new behavior will require coordination and substantial effort. We (Request Tracker) are willing to undergo that effort, but we do need to prepare for it. Given that we are in the middle of a stable series and DBD-mysql 4.042 is getting traction, we are unable to do such a migration right now. Our only recourse right now is to refuse to install under 4.042, forcing users to install 4.041 by hand.

[pali]

old behavior was widely deployed

What we learnt from this is that developers do not read documentation nor care about states like "This option is experimental and may change in future versions.".

Now I'm thinking how could be experimental features which are not expected to work correctly or which can be removed/changed at any time introduced into modules...

[ap]

What we learnt from this is that developers do not read documentation nor care about states like "This option is experimental and may change in future versions.".

How seriously can you expect them to take that after the feature has been “experimental” without changes in behaviour for 10 years?

[sartak]

How seriously can you expect them to take that after the feature has been “experimental” without changes in behaviour for 10 years?

Furthermore see this commit from a year ago and especially its message: 026fa02