10 Aug
2021
10 Aug
'21
2:50 p.m.
Hi Serg,
On Mon, Jul 12, 2021 at 9:13 AM Sergei Golubchik <serg@mariadb.org> 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@mariadb.com>
> > committer: Sachin Kumar <sachin.setiya@mariadb.com>
> > 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@mariadb.org
--
Regards
Sachin Setiya
Software Engineer at MariaDB