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.

Sign up for GitHub

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

Jump to bottom

Raise a more obvious exception when form data is missing "boundary". #1349

Merged
merged 6 commits into from
Feb 3, 2022
Merged

Raise a more obvious exception when form data is missing "boundary". #1349

merged 6 commits into from
Feb 3, 2022

Conversation

toxadx
Copy link
Contributor

@toxadx toxadx commented Nov 29, 2021 •
edited by tomchristie
Loading

The best strategy is to let KeyError raise when parsing request with missing boundary parameter, so end-user can process KeyError exception in their code.

Closes #1125
Related to #1303

@adriangb adriangb added feature New feature or request refactor Refactor code labels Feb 2, 2022
tomchristie
tomchristie approved these changes Feb 3, 2022
View reviewed changes
Copy link
Member

@tomchristie tomchristie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raising a KeyError here isn't perfect, but I'd agree that it's better.
So yup, let's go with this. Thanks!

@tomchristie tomchristie merged commit a6d3d8f into encode:master Feb 3, 2022
@tomchristie tomchristie changed the title Raise KeyError at early stage for missing boundary Raise a more obvious exception when form data is missing "boundary". Feb 3, 2022
@Kludex
Copy link
Member

Kludex commented Feb 3, 2022

We should probably catch that and raise something else from Starlette itself 🤔

@tomchristie
Copy link
Member

@Kludex - I'd be okay with that if someone's sufficiently motivated, yup.

(Rolled with this for now, since as @toxadx mentions, this is at least an improvement for now.)

@Kludex Kludex mentioned this pull request Mar 9, 2022
Closed
@opeco17 opeco17 mentioned this pull request Mar 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomchristie tomchristie tomchristie approved these changes

Assignees
No one assigned
Labels
feature New feature or request refactor Refactor code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Insufficient input validation of content-type 'multipart/form-data'
4 participants
@toxadx @Kludex @tomchristie @adriangb