Repository navigation
Decode frames that arrive together before applying the buffer cap - #3084
Merged
janiversen merged 2 commits intoNov 8, 2026
Merged
Conversation
datagram_received dropped the whole receive buffer when the bytes just read pushed it past max_buffer_size, before any frame was decoded. Two complete requests that arrive in one read (more than 264 bytes together, for instance two write multiple registers requests of 60 registers) were discarded without a response. Decode first and apply the cap to what is left, which can only be the start of a frame.
janiversen
requested changes
Nov 8, 2026
| self.recv_buffer = self.recv_buffer[cut:] | ||
| if len(self.recv_buffer) > self.max_buffer_size: | ||
| # Several frames can arrive in one read, but what is left after | ||
| # decoding is at most the start of a frame, which cannot be this long. |
Collaborator
There was a problem hiding this comment.
please remove the comment, inline comments are only used when it is not clear what the code does.
In this case the comment is actually wrong, the reason for limitint the buffer is to ensure against a malicious client that tries to break the server by inserting a big stream.
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.
Since #3026,
ModbusProtocol.datagram_receivedemptiesrecv_bufferas soon as it grows pastmax_buffer_size, before any frame is decoded. Modbus TCP allows several outstanding transactions and TCP does not keep message boundaries, so two complete requests can arrive in one read. When they are larger than 264 bytes together, the server answers neither and logs nothing (#3083).datagram_receivednow decodes first and applies the cap to what is left. What is left after decoding can only be the start of one frame, so the cap still bounds the buffer.Validation
test_data_received_frames_above_buffer_size(new): withmax_buffer_sizeat 10, 25 bytes that decode as frames reachcallback_dataand the buffer is empty afterwards. Ondev(0ccfdb4) it fails:callback_datais not called.ModbusTcpServeron 127.0.0.1: two write multiple registers requests in one send of 266, 426 and 518 bytes now get 2 of 2 responses (0 of 2 ondev). Separate sends, and coalesced sends of up to 262 bytes, still get 2 of 2.pytest test/transporton Python 3.14.7 (macOS), withouttest_serial.pybecause pyserial is not installed here: 105 passed. The 6 failures are the SERIAL cases oftest_comm.py, which fail the same way on unmodifieddevhere.ruff check,ruff format --check,codespellandpylint(10.00/10) on the touched files are clean.Fixes #3083
AI disclosure: written with an AI coding agent and tested as described above.