Add new NotHttpProtocol exception to differentiate invalid methods and gibberish
#10067
Conversation
… methods and gibberish
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #10067 +/- ##
=======================================
Coverage 98.76% 98.76%
=======================================
Files 121 121
Lines 36860 36867 +7
Branches 4396 4398 +2
=======================================
+ Hits 36403 36410 +7
Misses 311 311
Partials 146 146
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
bdraco
added
backport-3.11
psf-chronographer
bot
added
the
bot:chronographer:provided
bdraco
changed the title
NotHttpProtocol exception to differentiate invalid methods and gibberish
CodSpeed Performance ReportMerging #10067 will not alter performanceComparing Summary
|
|
I want to look back at all the HA issues and be sure to add some more tests for other types of gibberish |
| if b" " not in data: | ||
| return NotHttpProtocol(err_msg) |
There was a problem hiding this comment.
This seems unreliable since a binary protocol can easily include b" " by accident.
A TLS handshake always begins with b"\x16" (RFC 8446 §5.1); we could use that to reliably detect HTTPS at least.
There was a problem hiding this comment.
Yeah its a bit messy to guess here... maybe we should just leave it as-is.
There was a problem hiding this comment.
I think its better to leave it as-is and not try to differentiate as llhttp is always going to be able to do a better job than a heuristic as otherwise we end up with a whole list of possible exceptions for TLS or other garbage
Followup to #10055 (comment) to better differentiate invalid methods and gibberish