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

Do not apply max_form_parts to non-multipart data #2694

Merged
merged 1 commit into from
May 5, 2023

Conversation

ThiefMaster
Copy link
Member

@ThiefMaster ThiefMaster commented May 5, 2023

Another side effect of inlining some helper functions is that parsing application/x-www-form-urlencoded form data now uses max_form_parts like multipart/form-data. Not as important in this case, but may as well be consistent.

I feel like change from #2608 is actually more of a bug/regression that should possibly be reverted, which is exactly what this PR does.

  • Unlike parsing multipart data, the overhead of parsing url-encoded form data is tiny (less than 2s for a million fields vs 25s for the same amount of multipart fields)
  • It's not really that uncommon to have POST data with many values, especially compared to equally huge multipart requests
  • Parsing urlencoded form data is more similar to parsing JSON data than to parsing multipart data - and AFAIK the default JSON parser has no such limit either.
  • "parts" is a term that's usually just used when it comes to MIME/multipart data, not random form fields
  • the docstring of Request.max_form_parts also only mentions "multipart parts"

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

I believe no versionchanged is needed since the change this fixes was silent in this regard as well.

ThiefMaster added a commit to indico/indico that referenced this pull request May 5, 2023
See pallets/werkzeug#2694 (and revert this if/when the PR gets merged
and released and werkzeug is updated to a version including the change)
@davidism davidism added this to the 2.3.4 milestone May 5, 2023
@davidism davidism merged commit 3072610 into pallets:2.3.x May 5, 2023
@ThiefMaster ThiefMaster deleted the formencoded-max-parts branch May 5, 2023 17:29
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants