Fix malformed-payload handling on reconnect - #58
Draft
jkolo wants to merge 5 commits into
Draft
Conversation
MDRUnpackCommand indexed the unescaped frame without checking that it is long enough to hold a header, so a frame that carries fewer than seven bytes between its markers - or one whose escape sequences fail to unescape at all, which Unescape reports as an empty buffer - read past the end of the span. `3E 3C` on the wire is enough to reach it, and the packet debugger runs the same unpacker over recorded captures. Both that and a length field disagreeing with the markers now report MALFORMED rather than INCOMPLETE: the markers already bound the frame, so waiting for more bytes cannot repair it, and the receive buffer never drained. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RKt3Lk68pt5yr92YV1iTqn
A device that changed state while unattended pushes the matching notify as soon as the socket opens - PERI_NTFY_PARAM with the paired device list is the one seen in practice, right after another client released the link. That can arrive before CONNECT_RET_PROTOCOL_INFO answers, and the guards treated "too early" as "malformed", killing the session with MDR_RESULT_ERROR_MALFORMED_PAYLOAD. Being a race against our own handshake, it hit roughly one connection in three. There is still no table to parse such a frame against, so it is dropped rather than queued: the ACK has already gone out and init reads back every state we care about anyway. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RKt3Lk68pt5yr92YV1iTqn
mdr_framing_tests drives the real poll path through the mock transport with synthetic frames: data arriving before the handshake, frames too short to hold a header, a dangling escape sentry, a frame split across one and five byte reads, and leftovers preceding a good frame. Every case asserts the session survives and still accepts the handshake afterwards, which the capture replay cannot check because it always starts from protocol info. The mock moves to a shared header so both suites use it. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RKt3Lk68pt5yr92YV1iTqn
Send dumped its bytes with the same "<<" prefix Receive uses, so a debug log read as if the device had sent everything. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RKt3Lk68pt5yr92YV1iTqn
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qy2qqZHg5iaikSzDSNMT9o
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
1 participant
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.
Draft - opening early for visibility, still want to look over edge cases before this is ready.
Fixes malformed-payload handling around reconnect: a notify frame racing the protocol handshake, plus an out-of-bounds read while unpacking truncated frames.
mdr-framingCTest suite)One note:
libmdr/include/mdr/Generated/Command.hpppicked up a hand-addedMALFORMEDcase in the first commit instead of going throughcodegen- re-rancmake --build --target codegenlocally and it reproduces byte-identical output, so the hand-edit matches what codegen would have produced, but flagging since CONTRIBUTING asks for the generator to be run instead of hand-editing.