fix(sender): fail fast on corrupt packet lengths and skip unknown packet types#129
Open
jasontitus wants to merge 3 commits into
Open
fix(sender): fail fast on corrupt packet lengths and skip unknown packet types#129jasontitus wants to merge 3 commits into
jasontitus wants to merge 3 commits into
Conversation
…ion parsing Adds a TBDisplaySenderTests unit bundle (hosted by the app for @testable access) with 49 tests over the pure logic that has no hardware dependency: - TBMonitorProtocol: BE32 primitives, packet layout, drainPacket handling of fragmented/contiguous/split feeds, JSON payload round-trips, and parity of the hand-rolled input-event encoder with JSONDecoder — guarding the invariant documented on makeInputEventPacket (PR swellweb#123). - TBDiscoveredReceiver: per-transport ip(for:) selection (which IP the sender dials for Thunderbolt vs Network Link), id, shortHostName, displayText. - TBSenderAutomation: parseTransport/parseMode/parsePreset aliases, receiver matching, and the resolveSessionIndex tri-state. The automation parsing helpers move from private to internal so the test bundle can exercise them; behavior is unchanged. project.pbxproj and the shared TBDisplaySender scheme are regenerated via xcodegen so a fresh checkout can run `xcodebuild test` directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Executes the new TBDisplaySenderTests suite after the sender build. - Triggers CI on 3.1-maint and 3.2-dev in addition to main, so the branches that actually receive PRs get build/test coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ket types Two wire-protocol drain hardenings in TBMonitorProtocol.drainPacket, matching the receiver parser's behavior (net.c caps packets at 64 MiB and treats a bad length as fatal): - A corrupt length prefix (zero, or above the new 64 MiB maxPacketLength) now throws instead of returning "need more data". Previously a corrupted length such as 0xFFFFFFFF made the sender buffer inbound bytes forever, waiting for a packet that could never complete — unbounded memory growth on a corrupt stream. The drain loop in TBDisplaySenderService now closes the connection and surfaces the reason in the session status. - A packet with an unrecognized type byte (e.g. from a newer receiver) is now skipped and draining continues. Previously it consumed the packet but returned nil, which the caller treated as "buffer empty" — stalling every valid packet queued behind the unknown one until the next network read. Covered by 7 new unit tests (zero/oversized/all-ones lengths, cap boundary, corruption behind a valid packet, unknown-type skip and lone unknown-type). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Motivation
Two drain-loop weaknesses in
TBMonitorProtocol.drainPacket, both mismatches with the receiver's C parser (which caps packets at 64 MiB and treats a bad length as fatal):0xFFFFFFFF) made the sender buffer inbound bytes forever, waiting for a packet that can never complete — unbounded memory growth on a corrupt stream.What's in it
drainPacketnow throws on a zero or >64 MiB length (maxPacketLength, mirroringnet.c); the drain loop inTBDisplaySenderServicecloses the connection and surfaces the reason in the session status.How tested
xcodebuild test— 56/56 green (49 from #127 + 7 new).🤖 Generated with Claude Code