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

fix: cgi.FieldStorage not available in Python 3.13 #1438

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

defnull
Copy link
Member

@defnull defnull commented Nov 29, 2023

cgi.FieldStorage (used for multipart parsing) was deprecated in Python 3.11 and removed in 3.13. We now have to ship our own implementation (mostly a copy&paste from the multipart module)

cgi.FieldStorage (used for multipart parsing) was deprecated
in Python 3.11 and removed in 3.13. We now have to ship our
own implementation (mostly a copy&paste from the `multipart` module)
@defnull defnull added this to the Release 0.13 milestone Nov 29, 2023
@defnull defnull self-assigned this Nov 29, 2023
@oz123
Copy link
Contributor

oz123 commented Jan 2, 2024

@defnull is there anyway one can help you to get this merged?

The new parser is used for all python versions, which may break backwards compatibility for certain edge cases and error scenarios. For a 'good' request there should be no difference, though.

What changed:
* Lax newline parsing (accepting \r or \n in place of \r\n) is a security risk and no longer implemented. Most modern browsers and client libraries follow the spec and should not have any issues producing valid multipart data.
* Parsing errors are no longer silently ignored, but trigger an appropiate 400 error.
@defnull
Copy link
Member Author

defnull commented Jan 3, 2024

Tests do not actually pass yet and test coverage was lacking, so this should be considered a WIP for now and not ready to merge yet.

I had a local version that has better test coverage and kinda works, but was not pushed yet because I'm still not confident enough that this is a proper replacement. I'll push a working version now and would appreciate some testing (correctness and performance) and feedback.

@defnull
Copy link
Member Author

defnull commented Jan 3, 2024

Github actions are kinda useless for testing legacy python versions. I'll have to somehow get those tests working locally. We still want to support 2.7 and 3.6+ with the master branch.

@oz123
Copy link
Contributor

oz123 commented Jan 3, 2024

May out of scope here, but why do you still keen on supporting legacy python versions on the master branch?
Also, I can help with fixing GH workflows on those python versions, if this helps brings this forward (hint, I'll use asdf to install python2.7 - python3.7 on a plain ubuntu container).

@defnull
Copy link
Member Author

defnull commented Jan 3, 2024

May out of scope here, but why do you still keen on supporting legacy python versions on the master branch?

Yes you are right, that should not be a blocking issue for the next release.

The 0.12 branch supports versions down to 2.5 and 3.2. An impressive feat and really nice for development in legacy environments, but no longer feasible with modern versions in mind. The master/main branch which should have been released years ago as 0.13 already dropped 2.5 and 2.6, which leaves 2.7 and 3.2+. But supporting 3.2 in a new release makes no sense anymore.

Maybe we should just move on, prepare Bottle 1.0 with a Python 3.8+ requirement and call it a day. People that need support for EOL Python versions can still rely on Bottle 0.12

@oz123
Copy link
Contributor

oz123 commented Jan 26, 2024

Ping. Do you need help with finishing this PR?
This is almost done...

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.

None yet

2 participants