Hi, Alexander,
the fix is good, thanks.
a couple of notes about comments, see below.
ok to push after fixing them
On Feb 25, Alexander Barkov wrote:
> revision-id: 7ff9e91a7d2 (mariadb-10.4.32-179-g7ff9e91a7d2)
> parent(s): 1070575a890
> author: Alexander Barkov
> committer: Alexander Barkov
> timestamp: 2024-01-26 13:12:03 +0400
> message:
>
> MDEV-32975 Default charset doesn't work with PHP MySQLi extension
>
> When sending the server default collation ID to the client
> in the handshake packet, translate a 2-byte collation ID
> to the ID of the default collation for the character set.
>
> diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
> index 1949a0c02b6..5103ff02105 100644
> --- a/sql/sql_acl.cc
> +++ b/sql/sql_acl.cc
> @@ -12831,7 +12831,32 @@ static bool send_server_handshake_packet(MPVIO_EXT
> *mpvio,
>
> int2store(end, thd->client_capabilities);
> /* write server characteristics: up to 16 bytes allowed */
may be move this comment to be directly above end[2]=... line, as
before? Otherwise they're separated by another comment and an if()
block, so the comment looks out of place
> - end[2]= (char) default_charset_info->number;
> +
> + /*
> + Send the default server collation ID.
> + Note, since MySQL-4.1 it's actually the
> + client responsibility to send its character set to the server,
> + so this value means nothing and is ignored by most connectors.
> + But some rare connectors (e.g. PHP) still read it.
where did you get it from?
protocol description doesn't say that the charset field in the handshake
packet is obsolete and means nothing.
better to remove this claim. If you want to obsolete this field, don't
do it in an obscure comment in the code, but change the protocol
description in KB.
> + */
> + CHARSET_INFO *handshake_cs= default_charset_info;
> + if (handshake_cs->number > 0xFF)
> + {
> + /*
> + A workaround for a 2-byte collation ID: translate it into
> + the ID of the primary collation of this character set.
> + */
> + CHARSET_INFO *cs= get_charset_by_csname(handshake_cs->csname,
> + MY_CS_PRIMARY, MYF(MY_WME));
> + /*
> + cs should not normally be NULL, however it may be possible
> + with a dynamic character set incorrectly defined in Index.xml.
> + For safety let's fallback to latin1 in case cs is NULL.
> + */
> + handshake_cs= cs ? cs : &my_charset_latin1;
> + }
> + end[2]= (char) handshake_cs->number;
> +
> int2store(end+3, mpvio->auth_info.thd->server_status);
> int2store(end+5, thd->client_capabilities >> 16);
> end[7]= data_len;
>
Regards,
Sergei
Chief Architect, MariaDB Server
and secur...@mariadb.org
_______________________________________________
developers mailing list -- developers@lists.mariadb.org
To unsubscribe send an email to developers-le...@lists.mariadb.org
