Repository navigation
Conversation
WSFrameEncoder.decodeFrame read whatever payload length the frame header declared, up to Int.max, so a client could declare a 2^40 byte frame and feed bytes until the server ran out of memory. Any size check a handler makes on the assembled message comes after that. Check the declared length against a maximum right after the length and mask are decoded, before any payload is read. The limit is HTTPServer.Configuration.webSocketMaxFrameSize, 16 MiB by default, and HTTPConnection passes it to the client frame decoder. An oversized frame fails the stream with a WSFrameEncoder.Error carrying close code 1009; WSFrameEncoder.Error now names the close code a handler should send, so MessageFrameWSHandler answers with 1009 (Message Too Big) instead of a generic 1002. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T8cahEd4GfsHQ89sNrmP4p
WSFrameValidator checked only that a close frame's payload fits in 125 bytes. RFC 6455 §5.5.1 also requires a non-empty payload to begin with a 2 byte status code, §7.4 defines which codes an endpoint may send, and the reason that follows must be UTF-8. Reject a 1 byte payload, a status code outside 1000-1003, 1007-1014 and 3000-4999 (1005, 1006 and 1015 are reserved for local use; 1004 and 1016-2999 for future revisions), and a reason that is not valid UTF-8. Each closes the connection with 1002 like the validator's other errors. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T8cahEd4GfsHQ89sNrmP4p
An actor that passes elements from one task to another one at a time. send(_:) suspends until next() takes the element, so a sender gets no further ahead than its receiver and nothing accumulates between them. finish(with:) ends the handoff with a final element or error for the receiver and cancels any sender still waiting; cancelling a waiting sender or receiver removes its continuation. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T8cahEd4GfsHQ89sNrmP4p
MessageFrameWSHandler moved frames and messages between the connection and the handler through two AsyncStream continuations with unbounded buffers. A client that floods a server whose handler is slow, or that stops reading while the handler keeps sending, grows those buffers until the process runs out of memory; and because yielding never waits, a handler could not tell when a message had reached the connection. Replace the continuations with AsyncHandoffs. A client frame is now read only when the handler has taken the previous message, and a message is taken from the handler's stream only when the connection is about to write the previous frame, so back-pressure reaches the client's socket in one direction and the handler's stream in the other. A handler that produces messages on demand, with AsyncStream(unfolding:) for instance, is paced by the client. The frames stream is now pull-based, so it has no continuation whose termination cancels the handler's tasks. protocolFrames gains an onTermination closure that runs when the stream ends or is released, and the handler cancels its tasks from there. On a client's close frame, the echo is sent and the frames finished before the .close message is handed to the handler as the final message of its stream, so the closing handshake does not wait on the handler. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T8cahEd4GfsHQ89sNrmP4p
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #249 +/- ##
==========================================
+ Coverage 93.75% 93.83% +0.08%
==========================================
Files 72 73 +1
Lines 3793 3878 +85
==========================================
+ Hits 3556 3639 +83
- Misses 237 239 +2 ☔ View full report in Codecov by Harness. |
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.
Based on #248; the first two commits are that PR, so this diff shows them until it merges. The change here is the last two commits.
MessageFrameWSHandler.makeFramesmoves frames and messages between the connection and the handler through twoAsyncStreamcontinuations with unbounded buffers: incoming messages are yielded intomessagesIn, outgoing frames intoframesOut, and a task pumps between them. A client that floods a server whose handler is slow grows the first buffer; a client that stops reading while the handler keeps sending grows the second. On a small ARM board that ends with the process killed. And sinceyieldnever waits, a handler has no way to tell when a message has reached the connection.Change
AsyncHandoff<Element>(new, internal): an actor that passes elements one at a time.sendsuspends untilnexttakes the element, so a sender gets no further ahead than its receiver.finish(with:)ends it with a final element or error; a waiting sender or receiver that is cancelled is removed. It is built onwithIdentifiableThrowingContinuation, likeSocketPool.MessageFrameWSHandleruses two handoffs in place of the continuations. The handler's input isAsyncStream { try? await messagesIn.next() }, so a client frame is read only when the handler has taken the previous message; the connection's frames come fromAsyncThrowingStream { try await framesOut.next() }, so a message is taken from the handler's stream only when the connection is about to write the previous frame. Back-pressure reaches the client's socket in one direction and the handler's stream in the other. A handler that produces messages on demand, for example withAsyncStream(unfolding:), is paced by the client; one that yields into a continuation keeps its own buffer, as before.WSMessageHandlerandWSHandlerare unchanged.AsyncStream.protocolFrames(from:onTermination:): the frames stream is now pull-based, so there is no continuation whoseonTerminationcancels the handler's tasks. The closure runs when the stream ends or is released without being read to the end, and the handler cancels its tasks from there. This is the same mechanism as before (the stream's storage going away), one level up..closemessage is delivered as the final message of the handler's stream, so the closing handshake no longer waits on the handler.One existing test changed:
responseFramesread both messages before collecting the frames, which relied on the pong being buffered; it now collects the frames concurrently.Testing
New tests:
AsyncHandoffTests(8) cover suspension on both sides, final element and error delivery, and cancellation of a sender or receiver.WSHandlerTestscheck that a client frame stream is read at most one frame ahead of the handler taking messages, that the handler's stream is pulled at most one message ahead of frames being taken, and that a close frame is echoed and then delivered as the final message.protocolFramesrunsonTerminationboth when read to the end and when released early. The WebSocket suites were run 8 times in a row without a failure.swift teston Linux (Swift 6.4): 323 tests, the same 3 issues as onmain(HTTPDecoderTests.standardizedPathandnaughtyPath_IsParsed). Not run on Darwin locally.🤖 Generated with Claude Code
https://claude.ai/code/session_01T8cahEd4GfsHQ89sNrmP4p