Dealing with large requests: possibility to set max_incomplete_event_size for h11

[alexiri]

Checklist

• There are no similar issues or pull requests for this yet.
• I discussed this idea on the community chat and feedback is positive.
  (Asked in the chat, but no response received)

Is your feature related to a problem? Please describe.

I'm working on an application that needs to deal with large http headers, but I'm getting the following exception:

WARNING:  Invalid HTTP request received.
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/uvicorn/protocols/http/h11_impl.py", line 136, in handle_events
    event = self.conn.next_event()
  File "/usr/local/lib/python3.9/site-packages/h11/_connection.py", line 432, in next_event
    raise RemoteProtocolError(
h11._util.RemoteProtocolError: Receive buffer too long

Describe the solution you would like.

This error could be fixed if I was able to configure the max_incomplete_event_size of the h11 connection:

https://github.com/python-hyper/h11/blob/master/h11/_connection.py#L119

Describe alternatives you considered

For the time being, I've moved to Hypercorn which does allow me to change that parameter.

Additional context

[Kludex]

Yeah, some time ago I was reviewing the h11 implementation, and I've noticed that we just use the default.

The default value is explained on h11, but I guess it does make sense to allow the user to change its value if needed.

PR is welcome, but let's use the same strategy that we use with other CLI arguments: if the argument is not used, just ignore it.

Also, the "describe alternatives you considered" is meant to be an alternative solution that we can use to solve the problem in hands. Have you also tried the httptools implementation?

[alexiri]

I confess I haven't tried the httptools implementation, I was in a bit of a rush with the deployment of my application and didn't think of testing it. I'll see if I find some time this week to do so.

[johnraz]

Doesn't this also applies to large query string?
If so I think the issue should be renamed to avoid that people discard reading the issue while the explanation is actually there ;-)

[tomchristie]

I'm working on an application that needs to deal with large http headers

So, there is a trade-off with #1240 - which is that although it's a very nicely worked on PR. It also adds more configuration controls, which really we ought to try to avoid if at all possible.

More configurability means more different things for folks to read and understand when they're looking for whatever they do actually need.

It might not be a bad idea for us to just talk through your actual use-case here. Can you provide an example of why you need super large headers, and give an indication of what kind of size-range you're expecting?

A useful thing for us to then do would be just to have a bit of a look into what other clients or servers would (or wouldn't) support your use case. So we can make a more informed call on if we want to add configurability here or not.

[alexiri]

My application was sitting behind an SSO proxy which passed the user's information in headers. This information includes the groups that a user is a member of, which my application uses for some sharing features. Some users can be members of a huge number of groups, which results in a very large header that uvicorn is currently unable to deal with.

Since h11 already supports configuring this, I thought it would be worthwhile to actually expose this through uvicorn to make it useful. It seems I'm in the minority on this, though, and I've since moved on and no longer need this feature.

[gauravojha]

@alexiri @Kludex we have the same usecase as yours (behind a proxy with user info in header). Would love to ask with reopening the evaluation of #1240

In case contribution or changes to the original PR are required, we would be more than happy to help contribute back if it helps.

[Kludex]

I can review it if you continue the previous implementation. I've written some comments over there.

[gauravojha]

thanks.. will do..

[Kludex]

Be aware that I'd need to study this issue again, so it's not a promise of merging it. But if you work on the PR, I'll try my best to follow up.

[Kludex]

This was solved by #1514, and it's available since 0.18.0.