CalvinKirs opened a new pull request, #70420: URL: https://github.com/apache/doris/pull/70420
### Background FE reassembles MySQL physical frames into a logical packet before parsing handshake responses or commands. The existing `max_allowed_packet` variable defaults to 16 MiB and previously applied only to prepared-statement long data. ### What problem does this PR solve? Issue Number: None Related PR: None PR Type: Bugfix Problem Summary: Incoming logical packets did not honor `max_allowed_packet`: continuation frames were appended without checking their accumulated payload size. Check the total before reading each payload or growing its buffer, using the same logical-packet reader for plaintext and decrypted TLS traffic. Oversized packets return `ER_NET_PACKET_TOO_LARGE` (1153) and close the connection. TLS record refills also stop after inbound closure while preserving any remaining plaintext. ### Details - **Change map:** Three production files (+90/-128) and one test file (+227); no generated files or docs. `MysqlChannel` shares plaintext/TLS framing, checks cumulative payload size and buffer capacity, and handles rejection and TLS EOF. The other production changes add the standard MySQL error code and update the existing variable's comment. - **Frequency:** Whenever an incoming logical packet exceeds the configured limit; the TLS EOF case occurs when a peer sends `close_notify` while FE is reading a packet. - **Root cause:** The receive path enforced physical framing but did not compare the reassembled size with `max_allowed_packet`. The TLS refill loop also treated a closed engine with no application bytes as an open empty record. - **Why existing tests missed it:** Receive-side continuation limits and consumed TLS `CLOSED` records were not covered. - **Fix:** Use long arithmetic to check cumulative size before payload reads and allocation, bound buffer growth, and close even if writing the ERR response fails. Preserve buffered plaintext and distinguish established TLS EOF from an open empty record. - **Other paths and branches:** Handshake responses, commands and LOCAL INFILE share the receive check. Raw TLS handshake records retain separate framing; prepared-statement long-data handling keeps its existing limit. - **Behavior change:** The existing 16 MiB default now applies to incoming logical packets. Raise `GLOBAL max_allowed_packet` before opening connections that require larger requests; session changes apply to subsequent packets on that session. ### Testing All validation ran locally: - The oversized-header regression fails on the original implementation and passes with this fix. - `bash run-fe-ut.sh --run org.apache.doris.mysql.MysqlChannelTest,org.apache.doris.mysql.MysqlProtoTest,org.apache.doris.mysql.protocol.MysqlProtocolAdapterTest`: 32 tests passed, with no failures, errors or skips. Coverage includes continuation boundaries, rejection before body reads, failed ERR writes, TLS fragmentation/coalescing and TLS inbound closure. - `DISABLE_BUILD_UI=ON FE_MAVEN_THREADS=4 bash build.sh --fe --exclude-obs-dependencies --exclude-cos-dependencies`: passed, including Checkstyle. Separate Checkstyle checks for `fe-common` and `fe-core` also passed. - Connector/J 8.0.27, 8.0.33, 8.4.0 and 9.7.0: 104 compatibility cases passed, with no failures or skips, including real TLS and cross-FE forwarding. Both FEs used this patch; BE used an existing 4.1.3 binary. Test users/databases were cleaned up and authentication configuration was restored. - Real socket checks over plaintext and TLS: oversized handshake continuations are rejected after the header without sending the continuation body; exactly 16 MiB succeeds and the connection remains usable; one byte over the limit returns 1153. - `git diff --check`: passed. ### Release note Incoming MySQL logical packets now honor `max_allowed_packet` over plaintext and TLS. Its default remains 16 MiB. Oversized packets receive error 1153 and the connection closes; increase `GLOBAL max_allowed_packet` before opening new connections when larger requests are needed. ### Check List (For Author) - Behavior changed: Yes (see Details). - Does this need documentation: No; the existing variable and default are unchanged, and the newly enforced behavior is described in the release note. - Pre-submission status: Ready. ### AI Disclosure - Tool: Codex (GPT-6). - Generated: Code, tests and this description. - Human review: Pending maintainer review. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
