Implement mysqlnd_set_persistent_string #7371
Conversation
| mnd_pefree(conn->authentication_plugin_data.s, conn->persistent); | ||
| conn->authentication_plugin_data.s = NULL; | ||
| } | ||
| mysqlnd_set_persistent_string(&conn->authentication_plugin_data, NULL, 0, conn->persistent); | ||
| conn->authentication_plugin_data.l = plugin_data_len; | ||
| conn->authentication_plugin_data.s = mnd_pemalloc(conn->authentication_plugin_data.l, conn->persistent); | ||
| memcpy(conn->authentication_plugin_data.s, plugin_data, plugin_data_len); |
There was a problem hiding this comment.
I am not sure If I can change this
There was a problem hiding this comment.
pemalloc+memcpy is the same as pestrndup, so changing this should be fine.
There was a problem hiding this comment.
Ok, I tried it but I get warnings because it is zend_uchar. I am not sure what is going on with this assignment so I will roll back this one change.
ext/mysqlnd/mysqlnd_auth.c
Outdated
|
|
||
| tmp = mnd_pestrdup(passwd, conn->persistent); | ||
| char * tmp = mnd_pestrdup(passwd, conn->persistent); |
There was a problem hiding this comment.
Shouldn't the conn->password assignment be changed as well?
There was a problem hiding this comment.
It's the non-N version. I wasn't sure if I can touch this given that it is a variable-length assignment and the previous code didn't set the .l property anywhere. Is it safe to use strlen() here?
There was a problem hiding this comment.
pestrdup = pestrndup + strlen basically. You make a good point on this not assigning .l though (also applies to username assignment above), which seems suspect to me. I assume the length just never gets used?
There was a problem hiding this comment.
I noticed there is passwd_len in the signature, so I used it. Is this ok?
I grepped for conn->password but I can't see any usage of this property. To be frank, I don't understand what is the purpose of these properties on the conn object. The connection seems to happen before they are saved. The change_user method also uses the values from the signature and then copies the strings.
There was a problem hiding this comment.
Good point. Let's land this patch first and then remove them. I could see how they might be useful for a hypothetical "automatic reconnect" functionality, but given how such a functionality doesn't exist...
kamil-tekiela
force-pushed
the
mysqlnd-strings
branch
from
6fa01fc to
f2f31c7
Compare
This reverts commit ada3c56.
See #7365