Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES/10600.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fixed http parser not rejecting HTTP/1.1 requests that do not have valid Host header.
-- by :user:`Cycloctane`.
7 changes: 5 additions & 2 deletions aiohttp/_http_parser.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,7 @@ cdef class HttpParser:
cdef _on_headers_complete(self):
self._process_header()

http_version = self.http_version()
should_close = not cparser.llhttp_should_keep_alive(self._cparser)
upgrade = self._cparser.upgrade
chunked = self._cparser.flags & cparser.F_CHUNKED
Expand All @@ -453,6 +454,8 @@ cdef class HttpParser:
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:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should be restricted to 1.1?

https://www.rfc-editor.org/rfc/rfc9110#section-7.2-4

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

https://www.rfc-editor.org/rfc/rfc9112#appendix-C.2.1-1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if not, it should still be a lower bound, right? Not an exact match.

raise BadHttpMessage("Missing 'Host' header in request.")
h_upg = headers.get("upgrade", "")
allowed = upgrade and h_upg.isascii() and h_upg.lower() in ALLOWED_UPGRADES
if allowed or self._cparser.method == cparser.HTTP_CONNECT:
Expand All @@ -476,11 +479,11 @@ cdef class HttpParser:
method = http_method_str(self._cparser.method)
msg = _new_request_message(
method, self._path,
self.http_version(), headers, raw_headers,
http_version, headers, raw_headers,
should_close, encoding, upgrade, chunked, self._url)
else:
msg = _new_response_message(
self.http_version(), self._cparser.status_code, self._reason,
http_version, self._cparser.status_code, self._reason,
headers, raw_headers, should_close, encoding,
upgrade, chunked)

Expand Down
5 changes: 4 additions & 1 deletion aiohttp/http_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
LineTooLong,
TransferEncodingError,
)
from .http_writer import HttpVersion, HttpVersion10
from .http_writer import HttpVersion, HttpVersion10, HttpVersion11
from .streams import EMPTY_PAYLOAD, StreamReader
from .typedefs import RawHeaders

Expand Down Expand Up @@ -635,6 +635,9 @@ def parse_message(self, lines: list[bytes]) -> RawRequestMessage:
chunked,
) = self.parse_headers(lines[1:])

if version_o == HttpVersion11 and hdrs.HOST not in headers:
raise BadHttpMessage("Missing 'Host' header in request.")

if close is None: # then the headers weren't set in the request
if version_o <= HttpVersion10: # HTTP 1.0 must asks to not close
close = True
Expand Down
Loading
Loading