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

feat: Optional max_file_size for parsing form files #2718

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

khadrawy
Copy link

@khadrawy khadrawy commented Oct 6, 2024

Summary

Limiting the size of file being uploaded in case the storage is limited or some business logic can't handle certain size.

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

@khadrawy khadrawy force-pushed the feat/upload-file-max-size branch from 2d62595 to bc3b9c3 Compare October 6, 2024 04:24
@khadrawy khadrawy force-pushed the feat/upload-file-max-size branch from bc3b9c3 to d44aa98 Compare October 6, 2024 04:46
@khadrawy khadrawy marked this pull request as ready for review November 15, 2024 06:33
@Kludex
Copy link
Member

Kludex commented Dec 3, 2024

If we instead make the max_part_size configurable, would that already be good enough here?

@khadrawy
Copy link
Author

khadrawy commented Dec 3, 2024

If we instead make the max_part_size configurable, would that already be good enough here?

But from what I see here is that this is only applied to non file parts.

@Kludex
Copy link
Member

Kludex commented Dec 3, 2024

If we instead make the max_part_size configurable, would that already be good enough here?

But from what I see here is that this is only applied to non file parts.

Hmmm... Does it make sense to be only on non file parts tho? 🤔

@khadrawy
Copy link
Author

khadrawy commented Dec 3, 2024

@Kludex I think we need different limits for parts vs files. As files are spooled, we could allow larger size but for parts we want to have a stricter limit as it is stored in memory.
I can re-work the PR to make it configurable for both.

@khadrawy
Copy link
Author

@Kludex what do you think about making all of limits configurable? https://github.com/encode/starlette/pull/2798/files
I see that you are going to rename the max_file_size here #2780 I can adjust naming if we can go forward with making all of the size limits configurable.

@khadrawy khadrawy mentioned this pull request Dec 15, 2024
3 tasks
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.

2 participants