reject websocket close codes above the valid range#12895
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #12895 +/- ##
=======================================
Coverage 98.95% 98.95%
=======================================
Files 131 131
Lines 47824 47830 +6
Branches 2480 2480
=======================================
+ Hits 47324 47330 +6
Misses 376 376
Partials 124 124
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Merging this PR will not alter performance
Comparing Footnotes
|
|
Hi maintainers, Just a friendly follow-up on this PR. When you have a chance, could you please provide an update on its review status? Happy to make any additional changes if needed. Thank you! |
What do these changes do?
The websocket reader already rejects invalid close codes when it parses a CLOSE frame, but only at the low end of the range: a code under 3000 that isn't one of the registered ones raises a protocol error. There's no matching check at the top, so anything from 5000 up to 65535 is treated as a perfectly normal close code. :rfc:
6455#section-7.4.1only defines status codes up to 4999, so a peer can send a CLOSE frame carrying e.g. 65535 and we hand that straight through to the application as the close code rather than failing the connection. I noticed it while reading the close handling next to the lower-bound guard and the upper half just wasn't there. This adds the missing> 4999check alongside the existing one so out-of-range codes are rejected the same way the low ones are.Are there changes in behavior for the user?
A CLOSE frame whose status code is above 4999 now raises a protocol error instead of being delivered as a valid close. Codes in the permitted 1000-4999 range behave exactly as before.
Is it a substantial burden for the maintainers to support this?
No, it's a one-line addition to a check that already exists.
Related issue number
None. Spotted while reading through the close-frame handling.
Checklist
CONTRIBUTORS.txtCHANGES/folder