Add support for custom handling of requests that throw in `aiohttp.http_exceptions.*` #3287

bazbar · 2018-09-24T14:43:33Z

Long story short

Not sure if this is a user error, or a bug.

On a bad request, the server raise aiohttp.http_exceptions.BadStatusLine: invalid HTTP method, and custom headers are ignored. I can't find any way to catch this exception.

My application works as it should, and custom headers are sent on both GET and POST, and also HEAD - using the on_prepare signal.

I want to use custom headers and hide the Server: Python/3.6 aiohttp/3.4.4. But a simple request containing only "/" bypass all handlers, and display the default headers. And also raise a seemingly non-handable exception.

Expected behaviour

I'd expect being able to handle all requests, no matter how erranous.

Actual behaviour

In console, AioHttp drops

HTTP/1.0 400 Bad Request
Content-Type: text/html; charset=utf-8
Content-Length: 11
Date: Mon, 24 Sep 2018 14:13:37 GMT
Server: Python/3.6 aiohttp/3.4.4

Bad Request

and in logs:

Traceback (most recent call last):
  File "site-packages\aiohttp\web_protocol.py", line 242, in data_received
  File "aiohttp\_http_parser.pyx", line 523, in aiohttp._http_parser.HttpParser.feed_data
aiohttp.http_exceptions.BadStatusLine: invalid HTTP method

Steps to reproduce

Start a barebone server (like in Getting started)
telnet 127.0.0.1 8080, curl should work as well
type "/"

Your environment

Windows
Python/3.6 aiohttp/3.4.4
aiohttp server

The text was updated successfully, but these errors were encountered:

asvetlov · 2018-09-24T14:45:38Z

GitMate.io thinks possibly related issues are #2632 (aiohttp.http_exceptions.BadStatusLine: invalid HTTP method), #1619 (aiohttp.Client can't parse Trailer headers and throws exception), #1792 (aiohttp.http_exceptions.BadStatusLine caused by extra CRLF after POST data), #58 (aiohttp.HttpClient), and #1390 (Build chat server with aiohttp).

asvetlov · 2018-09-26T21:12:50Z

The best we can do here is dropping connection without any output (but logging the fact).
BadStatusLine is very exceptional.
In general, it is a kind of parser error. Nothing to do here, no user code to call.

asvetlov · 2018-09-26T21:13:22Z

Would you make a pull request?

webknjaz · 2018-09-27T08:40:33Z

No, server should reply with 400 Bad Request.

Yes, different servers act differently:

➜ telnet google.com 80
Trying 2a00:1450:4014:801::200e...
telnet: connect to address 2a00:1450:4014:801::200e: No route to host
Trying 172.217.23.206...
Connected to google.com.
Escape character is '^]'.
/
HTTP/1.0 400 Bad Request
Content-Type: text/html; charset=UTF-8
Referrer-Policy: no-referrer
Content-Length: 1555
Date: Thu, 27 Sep 2018 08:27:56 GMT

<!DOCTYPE html>
<html lang=en>
  <meta charset=utf-8>
  <meta name=viewport content="initial-scale=1, minimum-scale=1, width=device-width">
  <title>Error 400 (Bad Request)!!1</title>
  <style>
    *{margin:0;padding:0}html,code{font:15px/22px arial,sans-serif}html{background:#fff;color:#222;padding:15px}body{margin:7% auto 0;max-width:390px;min-height:180px;padding:30px 0 15px}* > body{background:url(//www.google.com/images/errors/robot.png) 100% 5px no-repeat;padding-right:205px}p{margin:11px 0 22px;overflow:hidden}ins{color:#777;text-decoration:none}a img{border:0}@media screen and (max-width:772px){body{background:none;margin-top:0;max-width:none;padding-right:0}}#logo{background:url(//www.google.com/images/branding/googlelogo/1x/googlelogo_color_150x54dp.png) no-repeat;margin-left:-5px}@media only screen and (min-resolution:192dpi){#logo{background:url(//www.google.com/images/branding/googlelogo/2x/googlelogo_color_150x54dp.png) no-repeat 0% 0%/100% 100%;-moz-border-image:url(//www.google.com/images/branding/googlelogo/2x/googlelogo_color_150x54dp.png) 0}}@media only screen and (-webkit-min-device-pixel-ratio:2){#logo{background:url(//www.google.com/images/branding/googlelogo/2x/googlelogo_color_150x54dp.png) no-repeat;-webkit-background-size:100% 100%}}#logo{display:inline-block;height:54px;width:150px}
  </style>
  <a href=//www.google.com/><span id=logo aria-label=Google></span></a>
  <p><b>400.</b> <ins>That’s an error.</ins>
  <p>Your client has issued a malformed or illegal request.  <ins>That’s all we know.</ins>
Connection closed by foreign host.

➜ telnet twitter.com 80
Trying 104.244.42.193...
Connected to twitter.com.
Escape character is '^]'.
/
Connection closed by foreign host.

However, let's clarify terms. The first line is called Request Line in HTTP request and Status Line in HTTP response. So BadStatusLine is a bit misleading name for the exception.

Now, if you look at RFC 7230, section 3.1.2, it clearly recommended to send a reply, not drop the connection immediately:

Recipients of an invalid request-line SHOULD respond with either a
400 (Bad Request) error or a 301 (Moved Permanently) redirect with
the request-target properly encoded. A recipient SHOULD NOT attempt
to autocorrect and then process the request without a redirect, since
the invalid request-line might be deliberately crafted to bypass
security filters along the request chain.

So if we want to follow RFCs, we have to do this and not reset a TCP connection. For some users, though, who need this connection resetting may be provided via feature flag or recommendations of how to do this manually.

webknjaz · 2018-09-27T08:42:38Z

@asvetlov the topic starter wants to define some HTTP handlers probably for consistency with valid requests (I'd not want to expose things to attacker who tries to find out which web server I use with just sending a malicious request).

asvetlov · 2018-09-27T08:52:55Z

Got you

webknjaz · 2018-09-27T09:12:24Z

Oh, and just as an idea: in CherryPy we have a setting/hook for error codes, which specifies what to send in payload. It can be either a string or a callable.

Maybe you could think of some similar concept within aiohttp architecture or add a recommendation to docs on how to process such things.

asvetlov · 2018-09-27T11:54:48Z

In aiohttp, we use middlewares for error handling.
The fix should follow this way I guess (but still support low-level HTTP server)

webknjaz · 2018-09-27T14:09:28Z

So the problem is that currently you can't catch this with middleware, right?

asvetlov · 2018-09-27T14:14:45Z

The thread starts from asking for handling the error by on_prepare signal.
Middleware is another way to handle it.

bazbar · 2018-09-30T15:24:34Z

Hi. Sorry for late reply and thanks for the discussion. For my application, this bug has the effect of allowing a client to bypass any attempts to serve custom headers passed in a WebResponse or by web.Application.on_response_prepare.append(). Else, the bug is more cosmetic, or principal, depending on how it's considered.

I'd prefer AioHttp to be RFC compliant, and not being able to handle any exception that's raised, seems weird. Perhaps catching it in system code/low-level code, and raise aiohttp.web.HTTPBadRequest is a viable way.

More seasoned coders than me should fix this. I would't know the wider effects of any change I'd make.

A bit confused - is this the same exception as in Standard lib's http.client.BadStatusLine, derived from HTTPException?

Thanks for all the work, and a very nice product!

asvetlov · 2018-09-30T17:07:37Z

No, aiohttp.web_exceptions are not derived from http.client ones.
There are many reasons for it but the first is: server-side exceptions for 400+ status codes have no common meaning with 400+ statuses received by client.

bazbar · 2018-10-04T20:22:27Z

Update - This behaviour disappeared when adding a ssl_context to the web.TCPSite().

asvetlov · 2018-10-04T20:30:56Z

Heh, encrypted SSL channel doesn't look like a regular plain HTTP status line, isn't it?

webknjaz · 2018-10-07T14:49:14Z

Wow, that looks oddly unrelated..

sersorrel · 2019-08-07T15:27:37Z

Can aiohttp include any more details of the request in the exception? I'm encountering this issue in my tests after upgrading from aiohttp 3.3.2 to 3.5.4, and I have no idea what's causing it.

syrkuit · 2019-12-21T22:43:03Z

So the problem is that currently you can't catch this with middleware, right?

That's certainly my problem, signals are no help either.
Not only can't I add extra headers to the response, but I also can't suppress these errors which aren't useful to me since I cannot do anything about them.

[Problem certainly is there with SSL as well (unsurprisingly), so long as you actually establish the SSL connection (i.e. openssl s_client -connect :)]

asvetlov · 2019-12-24T10:56:01Z

There is no well-formed request exists, no middleware is affected.
The error happens on the HTTP protocol level, even before aiohttp routing starts to work.
In other words, middlewares/signals are not called if the HTTP protocol is broken. There is no peer to send a response at all etc.

The only thing we can do is the conversion of exception with the attached stacktrace into a logged warning.
A volunteer is welcome!

syrkuit · 2019-12-24T15:00:41Z

You're correct that there's no well-formed request, and I don't think calling middlewares makes any sense either, however something catches the error and responds to the client with a well-formed 400 response.

A volunteer is welcome!

What do you think is the right thing to do though?

Removing the stacktrace and adding the peer's IP address to the error message (logged to aiohttp.server) would be nice (and trivial, I imagine), but that doesn't help with what the OP wanted (and what brought me here): customizing headers. For this, it's tempting to reuse signals but calling them with either "None" or a minimalist request seems likely to cause problems.
Another option could be to allow customizing the default error pages, beyond what https://docs.aiohttp.org/en/stable/web_advanced.html#handling-error-pages suggests (which doesn't apply here).

syrkuit · 2020-04-10T12:23:32Z

Any advice?

derlih · 2020-12-20T09:41:26Z

The only thing we can do is the conversion of exception with the attached stacktrace into a logged warning.

@asvetlov can you add more details what exactly should happen in this case?

rodusluo · 2022-06-09T06:23:20Z

you can try this to block the log:
app = web.Application()
app.logger.manager.disable = 100

FionnD · 2022-07-29T12:13:50Z

@rodusluo where did you find this?

If anyone reading this was able to provide explanation on how to validate a HTTP request before it hits AIOHTTP that would probably go someway to solving the issue here.

webknjaz · 2022-08-03T22:09:38Z

before it hits AIOHTTP that would probably go someway to solving the issue here.

Put a reverse proxy in front?

Dreamsorcerer · 2024-08-09T00:24:35Z

I think changing this behaviour so the BadRequest goes through middlewares seems reasonable and doable.

Looking through the code, we'd need to change:

aiohttp/aiohttp/web_protocol.py

Lines 537 to 542 in 4d604ea

    
           if isinstance(message, _ErrInfo): 
        
               # make request_factory work 
        
               request_handler = self._make_error_handler(message) 
        
               message = ERROR 
        
           else: 
        
               request_handler = self._request_handler

So that we always use self._request_handler.
Then we'd need to adapt Application._handle() so that it can receive an error object or similar. The main handler, before being wrapped by middlewares, would then be a system one that throws the BadRequest, similar to the NotFound ones we'd get from the router.

hilarex · 2024-11-28T11:37:33Z

I see that the issue is closed, but I also want to catch exception and answer with a custom HTTP response (while still keeping my custom headers).

How would I do that ?

bdraco · 2024-11-28T14:41:55Z

Let's leave this open since we haven't implemented being able to do that yet. I accidentally marked this one as it was similar to a group of others since there are multiple requests in this issue.

I renamed the issue to better clarify what the OP wants

bdraco · 2024-12-01T17:39:50Z

Ideally we have a way to customize the logging level as well #10076 (comment)

Dreamsorcerer · 2024-12-01T17:51:43Z

Note to self (or anyone who chooses to implement this): Remove the conditional debug logging from #10055 and add an example of using middlewares to customise the logging behaviour. (To change the logging level as @bdraco mentions can probably be done by logging in the middleware and not raising the exception?)

asvetlov added bug enhancement server labels Sep 24, 2018

poljar mentioned this issue May 10, 2019

Error: invalid http method matrix-org/pantalaimon#12

Closed

derlih mentioned this issue Dec 22, 2020

Handle exceptions? #5358

Closed

lordlabuckdas mentioned this issue Aug 5, 2021

error templating fixes mushorg/snare#308

Merged

Badiboy mentioned this issue Jan 7, 2023

aiohttp invalid method encountered eternnoir/pyTelegramBotAPI#1871

Closed

Dreamsorcerer removed the bug label Aug 9, 2024

Dreamsorcerer added this to the 4.0 milestone Aug 9, 2024

Dreamsorcerer mentioned this issue Aug 27, 2024

Cannot remove "Server" header in all cases for security hardening #6463

Open

1 task

Dreamsorcerer mentioned this issue Nov 15, 2024

Cathing internal exceptions #9882

Closed

1 task

bdraco mentioned this issue Nov 27, 2024

Downgrade logging of invalid HTTP methods on first request to debug level #10055

Merged

bdraco closed this as completed in #10055 Nov 27, 2024

bdraco reopened this Nov 28, 2024

bdraco changed the title ~~Server throws 'aiohttp.http_exceptions.BadStatusLine'~~ add supprot for custom handling of requests that result in 'aiohttp.http_exceptions.BadStatusLine' Nov 28, 2024

bdraco changed the title ~~add supprot for custom handling of requests that result in 'aiohttp.http_exceptions.BadStatusLine'~~ add support for custom handling of requests that result in 'aiohttp.http_exceptions.BadStatusLine' Nov 28, 2024

bdraco changed the title ~~add support for custom handling of requests that result in 'aiohttp.http_exceptions.BadStatusLine'~~ add support for custom handling of requests thatly throw in 'aiohttp.http_exceptions.* Nov 28, 2024

bdraco changed the title add support for custom handling of requests thatly throw in 'aiohttp.http_exceptions.* add support for custom handling of requests thatly throw in aiohttp.http_exceptions.* Nov 28, 2024

bdraco changed the title ~~add support for custom handling of requests thatly throw in aiohttp.http_exceptions.*~~ Add support for custom handling of requests that throw in aiohttp.http_exceptions.* Nov 28, 2024

bdraco mentioned this issue Dec 1, 2024

Fix request count check for BadHttpMethod #10076

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for custom handling of requests that throw in `aiohttp.http_exceptions.*` #3287

Add support for custom handling of requests that throw in `aiohttp.http_exceptions.*` #3287

bazbar commented Sep 24, 2018

asvetlov commented Sep 24, 2018

asvetlov commented Sep 26, 2018

asvetlov commented Sep 26, 2018

webknjaz commented Sep 27, 2018

webknjaz commented Sep 27, 2018 •

edited

Loading

asvetlov commented Sep 27, 2018

webknjaz commented Sep 27, 2018

asvetlov commented Sep 27, 2018

webknjaz commented Sep 27, 2018

asvetlov commented Sep 27, 2018

bazbar commented Sep 30, 2018

asvetlov commented Sep 30, 2018

bazbar commented Oct 4, 2018

asvetlov commented Oct 4, 2018

webknjaz commented Oct 7, 2018

sersorrel commented Aug 7, 2019

syrkuit commented Dec 21, 2019

asvetlov commented Dec 24, 2019

syrkuit commented Dec 24, 2019

syrkuit commented Apr 10, 2020

derlih commented Dec 20, 2020

rodusluo commented Jun 9, 2022

FionnD commented Jul 29, 2022

webknjaz commented Aug 3, 2022

Dreamsorcerer commented Aug 9, 2024 •

edited

Loading

hilarex commented Nov 28, 2024

bdraco commented Nov 28, 2024 •

edited

Loading

bdraco commented Dec 1, 2024

Dreamsorcerer commented Dec 1, 2024

Add support for custom handling of requests that throw in aiohttp.http_exceptions.* #3287

Add support for custom handling of requests that throw in aiohttp.http_exceptions.* #3287

Comments

bazbar commented Sep 24, 2018

Long story short

Expected behaviour

Actual behaviour

Steps to reproduce

Your environment

asvetlov commented Sep 24, 2018

asvetlov commented Sep 26, 2018

asvetlov commented Sep 26, 2018

webknjaz commented Sep 27, 2018

webknjaz commented Sep 27, 2018 • edited Loading

asvetlov commented Sep 27, 2018

webknjaz commented Sep 27, 2018

asvetlov commented Sep 27, 2018

webknjaz commented Sep 27, 2018

asvetlov commented Sep 27, 2018

bazbar commented Sep 30, 2018

asvetlov commented Sep 30, 2018

bazbar commented Oct 4, 2018

asvetlov commented Oct 4, 2018

webknjaz commented Oct 7, 2018

sersorrel commented Aug 7, 2019

syrkuit commented Dec 21, 2019

asvetlov commented Dec 24, 2019

syrkuit commented Dec 24, 2019

syrkuit commented Apr 10, 2020

derlih commented Dec 20, 2020

rodusluo commented Jun 9, 2022

FionnD commented Jul 29, 2022

webknjaz commented Aug 3, 2022

Dreamsorcerer commented Aug 9, 2024 • edited Loading

hilarex commented Nov 28, 2024

bdraco commented Nov 28, 2024 • edited Loading

bdraco commented Dec 1, 2024

Dreamsorcerer commented Dec 1, 2024

Add support for custom handling of requests that throw in `aiohttp.http_exceptions.*` #3287

Add support for custom handling of requests that throw in `aiohttp.http_exceptions.*` #3287

webknjaz commented Sep 27, 2018 •

edited

Loading

Dreamsorcerer commented Aug 9, 2024 •

edited

Loading

bdraco commented Nov 28, 2024 •

edited

Loading