Bug #49029 Do not convert BINARY data to charset representation
Submitted: 24 Nov 2009 11:07 Modified: 8 Jan 2010 15:33
Reporter: Tonci Grgin Email Updates:
Status: Closed Impact on me:
None 
Category:Connector / ODBC Severity:S2 (Serious)
Version: OS:Any
Assigned to: Lawrenty Novitsky
Triage: D2 (Serious)

[24 Nov 2009 11:07] Tonci Grgin
Description:
Pending the resolution of Bug#48698 we need to fix escaping binary data.
So, instead of _binary"A0AAA" it should be _binary 0x8000808080
I think it's rather important not to convert binary data to character representation especially when SQL_NO_BACKSLASHES mode is used. \0 converted to 0x00 byte just terminates the string.

How to repeat:
-

Suggested fix:
-
[24 Nov 2009 12:07] Bogdan Degtyariov
For big amounts of binary data this might work really badly because each original byte is represented as two-byte hex number. So, "INSERT ... VALUES (10_mb_jpeg_image)" will turn into "INSERT ... VALUES (20_mb_hex_numbers_data)".

Do you think we need an option to allow the raw binary data too?
[24 Nov 2009 12:13] Bogdan Degtyariov
Maybe we could analyze the size of the data and switch between hexadecimal or raw data representation for small and large data amounts respectively.
[1 Dec 2009 10:27] Bogdan Degtyariov
Patch, new connect option in the driver and GUI

Attachment: bug49029.diff (text/x-diff), 6.34 KiB.

[2 Dec 2009 12:06] Bogdan Degtyariov
Patch checks sql_mode and converts raw->hex if NO_BACKSLASH_ESCAPES is enabled

Attachment: bug49029v2.diff (text/x-diff), 3.87 KiB.

[8 Dec 2009 18:44] Jim Winstead
You don't need to use SHOW VARIABLES to find out if NO_BACKSLASH_ESCAPES is in use, you can check (mysql->server_status & SERVER_STATUS_NO_BACKSLASH_ESCAPES). Okay to push with the get_server_variables() code removed in favor of that.
[8 Dec 2009 20:09] Lawrenty Novitsky
And I revoke approval of that my change I (sort of:)) approved. It's wrong. But I don't revoke approval of the patch (including the change jim proposed)
and i guess status has to be "approved"
[11 Dec 2009 15:43] Lawrenty Novitsky
Wouldn't be changing SQL mode at session start a better and easier solution for this problem? I can't see downsides of that at the moment. why would we need NO_BACKSLASH_ESCAPES to be active?
[15 Dec 2009 7:31] Bogdan Degtyariov
Lawrin,

changing sql_mode would make Connector/ODBC not friendly to user settings. What if the application fails without NO_BACKSLASH_ESCAPES?
[15 Dec 2009 12:52] Lawrenty Novitsky
Of course that wouldn't be good, but I just can't see how that can happen.
We only change the way driver communicate with server, and not application. All application cares about is that driver will change and fetch data correctly. 

I can see only 2 ways how application can fail if we switch off NO_BACKSLASH_ESCAPES. First - it expects that c/odbc put escaped strings to a db and will get very disappointed, when receives not escaped strings back. Second - it expects that a particular query fails on some data and that way it verifies that string contains "\'" sequence :) Is it enough to bury my idea? or do you see anything else?
[17 Dec 2009 9:51] Sveta Smirnova
Bug #49733 was marked as duplicate of this one.
[29 Dec 2009 20:47] Lawrenty Novitsky
Patch pushed as revNo 861 and will be released in the next release(5.1.7?)

There are following in pushed changes from approved patch:
1). get_server_variable() and no_backslash_escapes_flag field of the dbc struct removed, introduced macro definition is_no_backslashes_escape_mode(db), that tests respective bit in mysql.status
That macro is used instead of removed flag test in the insert_param()
2). Added testcase to my_param
3). Added ChangeLog entry(I'd appreciate if somebody took a look at it)
[30 Dec 2009 20:44] Lawrenty Novitsky
Backported the patch to the 3.51 - the RevNo is 709 in that branch and will be released in 3.51.28.
(Also pushed little change to 5.1 tree - exchanged for binary sql types with is_binary_sql_type macro, which actually expands to the same condition)
[8 Jan 2010 15:33] Tony Bedford
An entry has been added to the 5.1.7 and 3.51.28 changelogs:

If NO_BACKSLASH_ESCAPES mode was used on a server, escaping binary data led to server query parsing errors.