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

Set upload limits consistently #43

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Feb 13, 2025

We previously checked that the content was below GitHub's 25M limit, but this was done in the request handler. aiohttp already checks the content size and has a limit of 1 MiB.

Instead, set the limit for aiohttp and for Caddy directly. Though the latter is redundant, it's possibly a bit more secure. Limiting upload to the regular site is also probably redundant since it goes to file_server which supports no uploads, but better to cut that off early. CloudFlare also has a limit set, but it's to its minimum allowed which is 100MB.

We previously checked that the content was below GitHub's 25M limit, but
this was done in the request handler. `aiohttp` _already_ checks the
content size and has a limit of 1 MiB.

Instead, set the limit for `aiohttp` and for Caddy directly. Though the
latter is redundant, it's possibly a bit more secure. Limiting upload to
the regular site is also probably redundant since it goes to
`file_server` which supports no uploads, but better to cut that off
early. CloudFlare also has a limit set, but it's to its minimum allowed
which is 100MB.
@QuLogic
Copy link
Member Author

QuLogic commented Feb 13, 2025

I noticed this with pushing the Plausible changes, where the list of files ended up being a 2.5MB JSON document, which failed before it even touched our code.

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