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

Limit request bodies size (default 250MB) #4

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

peb-adr
Copy link
Member

@peb-adr peb-adr commented Aug 31, 2023

No description provided.

@peb-adr
Copy link
Member Author

peb-adr commented Aug 31, 2023

This also includes a version update of caddy.
2.3.0 ->2.7

As a nice side effect of this, it seems the odd

docker-proxy-1  | {"level":"error","ts":1693489378.8311799,"logger":"http.handlers.reverse_proxy","msg":"aborting with incomplete response","error":"context canceled"}

logs no longer occur \o/

@peb-adr
Copy link
Member Author

peb-adr commented Aug 31, 2023

As proposed in OpenSlides/openslides-client#2732 (comment), 8859538 sends the limit with response.

I called the header Request-Body-Max-Size. There may be better suited names, however I could not find a standard one for this use case. Content-Length came to mind but does not seem appropriate, as the body of the request is empty.

@peb-adr
Copy link
Member Author

peb-adr commented Sep 6, 2023

I will prepend the custom header with 'X-' to make it clear as such.

@peb-adr
Copy link
Member Author

peb-adr commented Oct 25, 2023

This is a bit annoying.
Until caddy 2.7.0 (where caddyserver/caddy#5652 is fixed) using the request_body caddy directive will return status 502 (instead of 413), which is not good since the client cannot easily differentiate.
However caddy 2.6.0 was a major update which also introduced a problem for us where connections from the autoupdate service are closed correctly. So in order to get to the needed version, that has to be investigated first...

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

Successfully merging this pull request may close these issues.

1 participant