maria-developers team mailing list archive

[Sachin Setiya <sachin.setiya@xxxxxxxxxxx>]

Hi Serg,

On Mon, Jul 12, 2021 at 9:13 AM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
>
> 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.
Okay , I have changed the testcase.
>
> > -      if (!(inchar = (uchar) *++pos))
> > -     break;                          // readline adds one '\'
>
> 2. What does the old code mean? Why returning if inchar==0 ?
I am not sure , it kind of looks like break in the case of \0 , which is wrong.
> 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)
There can be \0 bytes in the string in the case of binary mode.
>     ...
>
> in other words, shouldn't we just remove the if? I tried, at least the
> main suite didn't break.
I am also doing the same thing (I am incrementing pos also, without
incrementing, binary_zero_insert.test fails for me)
+      if (!real_binary_mode && !*in_string && !inchar)
+        break;
This is just checking \0 and no binary mode.

Regards
Sachin


>
> > +        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



--
Regards
Sachin Setiya
Software Engineer at  MariaDB