reject reserved close code 1006 in websocket reader - #13536
Conversation
4d003f2 to
5a050bf
Compare
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Reviews (4): Last reviewed commit: "Update test_autobahn.py" | Re-trigger Greptile |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #13536 +/- ##
=======================================
Coverage 99.02% 99.02%
=======================================
Files 135 135
Lines 50486 50500 +14
Branches 2650 2652 +2
=======================================
+ Hits 49993 50007 +14
Misses 370 370
Partials 123 123
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
|
Co-authored-by: Sam Bull <aa6bs0@sambull.org>
for more information, see https://pre-commit.ci
Co-authored-by: Sam Bull <aa6bs0@sambull.org>
for more information, see https://pre-commit.ci
Co-authored-by: Sam Bull <aa6bs0@sambull.org>
Backport to 3.15: 💚 backport PR created✅ Backport PR branch: Backported as #13578 🤖 @patchback |
A
Closeframe's status code is checked inWebSocketReader._handle_frameagainstALLOWED_CLOSE_CODES, a set built from theWSCloseCodeenum. The enum carriesABNORMAL_CLOSURE(1006), which aiohttp only sets locally when a connection drops without a Close frame, so a frame that arrives with1006on the wire passed the check and was delivered as a valid close code (and could be echoed back in aiohttp's own close frame). RFC 6455 7.4.1 reserves1006, along with1005and1015, and forbids it as a status code on the wire, so1006is now rejected as a protocol error the same way1005and1015already are.What do these changes do?
Exclude
ABNORMAL_CLOSUREfrom the incoming-frame acceptance set so a Close frame carrying1006fails with a protocol error.1005and1015were already rejected because they are not enum members; only1006slipped through.Are there changes in behavior for the user?
Only for the malformed case. A peer that sends
1006in a Close frame now trips a protocol error instead of surfacing as a close message. The locally-generated1006for an abnormal closure (no Close frame received) is unchanged.Is it a substantial burden for the maintainers to support this?
No. It drops one reserved code from the acceptance set and adds a regression test beside the existing close-code tests.
Related issue number
None.
Checklist
CONTRIBUTORS.txt(already listed)CHANGES/folder