Skip to content

fix: validate WebSocket CLOSE frames - #583

Merged
azkrishpy merged 2 commits into
mainfrom
todo/websocket-close
Oct 2, 2026
Merged

azkrishpy merged 2 commits into
mainfrom
todo/websocket-close

Conversation

@azkrishpy

Copy link
Copy Markdown
Contributor

Issue #, if available: N/A

Description of changes:

Validates received WebSocket control frames, which the decoder previously accepted without checking. Replaces the open validate payload of CLOSE frame TODO.

Three rules, all MUST requirements the decoder was not meeting:

  • control frames must carry 125 bytes or less (RFC-6455 5.5)
  • a CLOSE frame with a body must have at least the 2-byte status code, so a 1-byte payload is illegal (RFC-6455 5.5.1)
  • the CLOSE status code must be one the spec permits on the wire (RFC-6455 7.4)

Why this is a fix and not part of the TODO-cleanup chore: these frames used to decode and reach the caller, and now close the connection. The realistic case is a CLOSE frame carrying 1005 or 1006. Those codes are reserved for an endpoint's own internal use and must never be sent, but implementations do emit them. Previously the frame was delivered and the connection closed normally; now the decoder raises AWS_ERROR_HTTP_WEBSOCKET_PROTOCOL_ERROR, and there is no per-frame recovery, so the whole connection is dropped during what the peer intended as an orderly shutdown. That is what the spec asks for, but it turns working traffic into a failure and deserves its own review and release note.

The status code table accepts 1000-1014 except 1004, plus 3000-4999, and rejects 1005/1006/1015 along with everything else. Rejecting the reserved 1016-2999 range is the one judgment call here; the RFC leaves it undefined, so accepting and ignoring would also be defensible.

Still patch-level: no API changes, and the behavior is what the spec expects.

Two new decoder test cases covering the oversized control frame and the status codes.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@azkrishpy
azkrishpy requested a review from a team as a code owner September 8, 2026 21:58
@github-actions github-actions Bot added the patch ABI backward-compatible (patch release) label Sep 8, 2026
@azkrishpy
azkrishpy force-pushed the todo/websocket-close branch from 58a5c8f to eb8fdb7 Compare September 8, 2026 22:05
@codecov-commenter

codecov-commenter commented Sep 8, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.10%. Comparing base (050aa40) to head (eb2c87d).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #583      +/-   ##
==========================================
+ Coverage   79.95%   80.10%   +0.14%     
==========================================
  Files          28       28              
  Lines       12154    12182      +28     
==========================================
+ Hits         9718     9758      +40     
+ Misses       2436     2424      -12     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@azkrishpy
azkrishpy merged commit 802f6cd into main Oct 2, 2026
91 of 92 checks passed
@azkrishpy
azkrishpy deleted the todo/websocket-close branch October 2, 2026 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

patch ABI backward-compatible (patch release)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants