Repository navigation
[fix](fe) Enforce max_allowed_packet for incoming MySQL packets - #70420
Open
CalvinKirs wants to merge 2 commits into
Open
CalvinKirs wants to merge 2 commits into
CalvinKirs wants to merge 2 commits into
Conversation
### 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.
Member
Author
|
run buildall |
Member
Author
|
run buildall |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
FE reassembles MySQL physical frames into a logical packet before parsing handshake responses or commands. The existing
max_allowed_packetvariable 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 returnER_NET_PACKET_TOO_LARGE(1153) and close the connection. TLS record refills also stop after inbound closure while preserving any remaining plaintext.Details
MysqlChannelshares 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.close_notifywhile FE is reading a packet.max_allowed_packet. The TLS refill loop also treated a closed engine with no application bytes as an open empty record.CLOSEDrecords were not covered.GLOBAL max_allowed_packetbefore opening connections that require larger requests; session changes apply to subsequent packets on that session.Testing
All validation ran locally:
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 forfe-commonandfe-corealso passed.git diff --check: passed.Release note
Incoming MySQL logical packets now honor
max_allowed_packetover plaintext and TLS. Its default remains 16 MiB. Oversized packets receive error 1153 and the connection closes; increaseGLOBAL max_allowed_packetbefore opening new connections when larger requests are needed.Check List (For Author)
AI Disclosure