MDEV-25817 proxy protocol: successful login does not reset connect errors#5292
MDEV-25817 proxy protocol: successful login does not reset connect errors#5292vaintroub wants to merge 2 commits into
Conversation
Analysis: The collation is hard coded. Fix: The table initially had system character set hardcoded. However, a table field should derive character set and collation from table and table should derive it from database. Hence made necessary change to reflect that.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses an issue where a successful login under the PROXY protocol did not reset the connect-error counter for the proxied client and the proxy host. It introduces a new flag have_proxy_protocol_connect_errors to track connection errors and ensures both the socket peer and the real client have their connection error counters reset upon successful authentication. Additionally, test cases are added to verify this behavior. The review feedback correctly points out that the ip array in sql_connect.cc and the host_errors variable in net_serv.cc are declared but not initialized, which could lead to reading uninitialized memory.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| int auth_rc; | ||
| NET *net= &thd->net; | ||
| /* Socket peer IP address, the proxy host under PROXY protocol */ | ||
| char ip[NI_MAXHOST]; |
There was a problem hiding this comment.
The ip array is declared but not initialized. If there is any path where connect_errors is non-zero but ip has not been populated (or if vio_peer_addr fails/is skipped), passing ip to reset_host_connect_errors will result in reading uninitialized stack memory. Initializing it to zero prevents potential undefined behavior and compiler/static analyzer warnings.
char ip[NI_MAXHOST]= {0};| @@ -960,9 +962,13 @@ static handle_proxy_header_result handle_proxy_header(NET *net) | |||
| /* Change peer address in THD and ACL structures.*/ | |||
| uint host_errors; | |||
There was a problem hiding this comment.
The host_errors variable is declared but not initialized. If thd_set_peer_addr returns early or fails without writing to host_errors, it will contain garbage when checked in if (host_errors). Initializing it to 0 ensures safe and predictable behavior.
uint host_errors= 0;
…rors With proxy protocol thd_set_peer_addr() runs twice (proxy host, then the real client from the proxy header). Connect errors are accounted against the real client, but check_connection() incorrectly uses condition on the proxy host's count, rather than real client's address. Fix: reset both the proxy host, and real client's connect errors on successful connection. Added tests for incomplete handshake, and reset behavior, under proxy protocol, for both real client errors, and proxy host errors.
vaintroub
force-pushed
the
MDEV-25817-proxy-protocol-connect-errors-reset
branch
from
a2d3985 to
8744ed8
Compare
|
|
3 participants
With proxy protocol thd_set_peer_addr() runs twice (proxy host, then the real client from the proxy header). Connect errors are accounted against the real client, but check_connection() incorrectly uses condition on the proxy host's count, rather than real client's address.
Fix: reset both the proxy host, and real client's connect errors on successful connection.
Added tests for incomplete handshake, and reset behavior, under proxy protocol, for both real client errors, and proxy host errors.