maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Sachin!

On Jul 12, Sachin Kumar wrote:
> revision-id: cdb6db8ce38 (mariadb-10.3.26-155-gcdb6db8ce38)
> parent(s): 72753d2b65f
> author: Sachin Kumar <sachin.setiya@xxxxxxxxxxx>
> committer: Sachin Kumar <sachin.setiya@xxxxxxxxxxx>
> timestamp: 2021-05-19 15:46:57 +0100
> message:
> 
> MDEV-25444 mysql --binary-mode is not able to replay some mysqlbinlog outputs
> 
> Problem:- Some binary data is inserted into the table using Jconnector. When
> binlog dump of the data is applied using mysql cleint it gives syntax error.
> 
> Reason:-
> After investigating it turns out to be a issue of mysql client not able to properly
> handle  \\\0 <0 in binary>. In all binary files where mysql client fails to insert
> these 2 bytes are commom (0x5c00)
> 
> Solution:-
> I have changed mysql.cc to include for the possibility that binary string can
> have \\\0 in it.
> 
> ---
>  client/mysql.cc                            |   8 ++++++--
>  mysql-test/main/binary_zero_insert.result  |   6 ++++++
>  mysql-test/main/binary_zero_insert.test    |  15 +++++++++++++++
>  mysql-test/std_data/binary_zero_insert.bin | Bin 0 -> 87 bytes
>  4 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/client/mysql.cc b/client/mysql.cc
> index 2a7c4eaf3e5..8530c105820 100644
> --- a/client/mysql.cc
> +++ b/client/mysql.cc
> @@ -2319,8 +2319,12 @@ static bool add_line(String &buffer, char *line, size_t line_length,
>      {
>        // Found possbile one character command like \c
>  
> +      inchar = (uchar) *++pos;
> +      // In Binary mode , when in_string is not null \0 should not be treated as
> +      // end statement. This can happen when we are in middle of binary data which
> +      // can contain \0 and its quoted with ' '.
> +      if (!real_binary_mode && !*in_string && !inchar)

Two comments here.

1. if you check the history where this "real_binary_mode" came from,
you'd arrive at commit 2db4392bf4cb and the test file mysql_binary_mode.test
So, I'd say it make sense to add your test there too. And in the same
style, wthout a separate binary file.

in my experimening I did it as simple as

--- a/mysql-test/main/mysql_binary_mode.test
+++ b/mysql-test/main/mysql_binary_mode.test
@@ -16,14 +16,14 @@ let $table_name_right= `SELECT 0x410D0A42`;
 let $table_name_wrong= `SELECT 0x410A42`;

 # 0x410042 => 'A\0B'
-let $char0= `SELECT 0x410042`;
+let $char0= `SELECT 0x4100425C0043`;

(there already was a test for a \0 byte, but not for an escaped \\\0)
but if you'd like you can add a separate test with a header and
everything.

> -      if (!(inchar = (uchar) *++pos))
> -	break;				// readline adds one '\'

2. What does the old code mean? Why returning if inchar==0 ?
What does the comment mean? It's the code from the last
millenium, does it even apply now? For example, there can be no
\0 bytes in the string, because of

  if (!real_binary_mode && strlen(line) != line_length)
    ...

in other words, shouldn't we just remove the if? I tried, at least the
main suite didn't break.

> +        break;				// readline adds one '\'
>        if (*in_string || inchar == 'N')	// \N is short for NULL
>        {					// Don't allow commands in string
>  	*out++='\\';

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx