Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTPS requests received on HTTP port show generic BadStatusLine exception #8065

Closed
1 task done
codyc1515 opened this issue Jan 27, 2024 · 5 comments · Fixed by #10055
Closed
1 task done

HTTPS requests received on HTTP port show generic BadStatusLine exception #8065

codyc1515 opened this issue Jan 27, 2024 · 5 comments · Fixed by #10055

Comments

@codyc1515
Copy link

codyc1515 commented Jan 27, 2024

Describe the bug

When an HTTPS request is sent to the HTTP server port, a generic exception is thrown aiohttp.http_exceptions.BadStatusLine: 400, message: Invalid method encountered.

This has caused confusion for some Home Assistant users with a misconfigured reverse-proxy set-up. With this message, it is not always clear where the issue is. Example.

To Reproduce

Send a HTTPS request on the HTTP port.

Expected behavior

b'\x16\x03' appears to be a known HTTPS connection message. Given that we know this, a more descriptive error message (in lieu of Invalid method encountered) could be provided, such as Received HTTPS traffic on HTTP port.

Logs/tracebacks

Logger: aiohttp.server
Source: /usr/local/lib/python3.11/site-packages/aiohttp/web_protocol.py:421
First occurred: 8:32:09 AM (12 occurrences)
Last logged: 8:32:10 AM

Error handling request
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/aiohttp/web_protocol.py", line 350, in data_received
    messages, upgraded, tail = self._request_parser.feed_data(data)
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "aiohttp/_http_parser.pyx", line 557, in aiohttp._http_parser.HttpParser.feed_data
aiohttp.http_exceptions.BadStatusLine: 400, message:
  Invalid method encountered:

    b'\x16\x03\x03\x01F\x01'
      ^

Python Version

3.11.6

aiohttp Version

3.9.1

multidict Version

6.0.4

yarl Version

1.9.4

OS

Home Assistant OS 11.4

Related component

Server

Additional context

No response

Code of Conduct

  • I agree to follow the aio-libs Code of Conduct
@codyc1515 codyc1515 added the bug label Jan 27, 2024
@andersk
Copy link

andersk commented Feb 22, 2024

This is becoming more frequent due to Chromium’s automatic HTTPS upgrades, where HTTPS requests are attempted on the same port if a port number is specified in the URL.

Related:

@Dreamsorcerer
Copy link
Member

Seems reasonable if someone want to volunteer to make the change. It will need a change for both Python and Cython parsers.

@bdraco
Copy link
Member

bdraco commented Nov 23, 2024

This error comes up quite often when there isn't anything actually wrong, and its quite expected that anything exposed to the public internet will get quite a bit of garbage traffic that will generate these errors. We should probably downgrade it to debug logging.

https://github.com/aio-libs/aiohttp/issues?q=sort%3Aupdated-desc+is%3Aissue+is%3Aopen+BadStatusLine

@bdraco
Copy link
Member

bdraco commented Nov 27, 2024

@Dreamsorcerer
Copy link
Member

I think it makes sense to make it a debug log at this point, though the noisiness in the past has been pretty helpful as most exceptions reported turned out to be parser bugs. I'm a lot more confident in the parser these days, so don't mind reducing the output.

But, even logging, we should improve the error message in this specific case by checking the first 2 bytes are \x16\x03 to make it easier to debug. Surely all those homeassistant issues will be more difficult for users to debug with the linked PR? i.e. They are now less likely to see the errors and those errors still don't explain the issue with their client clearly. I think HTTPS on HTTP port is such a common case that we can special case it. @bdraco Maybe something even simpler than #10067 and just change the error message (rather than the exception class)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants