Reject HTTP/1.1 requests without Host header#12264
Reject HTTP/1.1 requests without Host header#12264Cycloctane wants to merge 6 commits intoaio-libs:masterfrom
Conversation
add host header to request test payloads
for more information, see https://pre-commit.ci
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #12264 +/- ##
=======================================
Coverage 99.10% 99.11%
=======================================
Files 130 130
Lines 45432 45442 +10
Branches 2400 2401 +1
=======================================
+ Hits 45025 45038 +13
+ Misses 276 273 -3
Partials 131 131
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
psf-chronographer
bot
added
the
bot:chronographer:provided
| raise BadHttpMessage(f"Duplicate '{bad_hdr}' header found.") | ||
|
|
||
| if self._cparser.type == cparser.HTTP_REQUEST: | ||
| if http_version == HttpVersion11 and hdrs.HOST not in headers: |
There was a problem hiding this comment.
I don't think this should be restricted to 1.1?
There was a problem hiding this comment.
When 2+ support is added, it'll need to additionally check for this :authority header. But, until then, it should probably require Host on all versions.
There was a problem hiding this comment.
I don't think this should be restricted to 1.1?
It seems that this requirement is new in 1.1. It is not included in the original HTTP/1.0 RFC.
There was a problem hiding this comment.
Hmm, my link above is for RFC 9110 which covers HTTP generically, not a specific version. RFC 9112 says that Host header support was rolled out pretty quickly in HTTP/1.0, so it seems reasonable to require it today?
There was a problem hiding this comment.
Even if not, it should still be a lower bound, right? Not an exact match.
3 participants
What do these changes do?
BadHttpMessagewhen a HTTP/1.1 request does not set host headerAre there changes in behavior for the user?
Is it a substantial burden for the maintainers to support this?
Related issue number
Fixes #10600
Checklist
CONTRIBUTORS.txtCHANGES/folder