Fix generating authentication response with long strings #988
Conversation
pymysql/connections.py
Outdated
| data += _lenenc_int(len(authresp)) + authresp | ||
| elif self.server_capabilities & CLIENT.SECURE_CONNECTION: | ||
| data += struct.pack("B", len(authresp)) + authresp |
There was a problem hiding this comment.
Don't do this. See https://dev.mysql.com/doc/internals/en/connection-phase-packets.html#packet-Protocol::HandshakeResponse
if capabilities & CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA {
lenenc-int length of auth-response
string[n] auth-response
} else if capabilities & CLIENT_SECURE_CONNECTION {
1 length of auth-response
string[n] auth-response
"length of auth-response" is 1byte, not lenenc when capabilities & CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA is not true.
There was a problem hiding this comment.
Many excuses, Iʼve over-reacted without proper attention.
Iʼve pushed an updated variant which limits changes to the "client connection attributes" section.
Should I also add a test for this? The original issue can be triggered e.g. adding program_name="x"*300 to connect() arguments (in this case, struct.pack() will fail), or cycle of connecting with program_name gradually increasing by 1 byte.
There was a problem hiding this comment.
If you can. I love regression tests. Otherwise, I may create a bug again.
Connection attributes shall be encoded using lenenc-str approach for a separate string and the whole section.
MySQL protocol uses special rules for string longer than 251 characters, listed here: https://dev.mysql.com/doc/internals/en/connection-phase-packets.html#packet-Protocol::HandshakeResponse
PyMySQL has already been using this for unpacking server messages, but was not used for forming "handshake response" from a client.
We have discovered this on failing of automated tests under Jenkins which are executed at very long paths. By default, PyMySQL fills
program_namewith full path of the running script. The current version generates single byte 0xfe as full length of "client connect attrs".The proposed patch utilizes already present _lenenc_int() for prefixing formed strings.