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

Webconfig: Remove dependency on cgi module #7600

Merged
merged 1 commit into from Jan 3, 2021

Conversation

faho
Copy link
Member

@faho faho commented Jan 2, 2021

This is slated for removal in python 3.10, see
https://www.python.org/dev/peps/pep-0594/#cgi.

We currently only use it for three things:

  • escape_html in old python versions that didn't have that in the html
    module
  • Parsing multipart/form-data
  • Figuring out the charset for json

We keep the first one - if loading escape_html from html fails we fall
back to cgi.

We remove the second - I can't find any case where we use
multipart/form-data. Any place we post data we either explicitly pass
application/x-www-form-urlencoded or implicitly use application/json.

The third is the tricky bit. This drops charset detection under the
assumption that we're never going to encounter anything other than
utf-8 (or ascii, which is a utf-8 subset). I'm not sure that holds,
but if it doesn't we can just add a regex to parse the charset.

TODOs:

  • [N/A] Changes to fish usage are reflected in user documentation/manpages.
  • [N/A] Tests have been added for regressions fixed
  • User-visible changes noted in CHANGELOG.rst

Note: Because python 3.10 is expected in October 2021 and we might not be making another release until then, I would like this to be included in 3.2.0, or we'd have to make a 3.2.1 with it later.

This is slated for removal in python 3.10, see
https://www.python.org/dev/peps/pep-0594/#cgi.

We currently only use it for three things:

- escape_html in old python versions that didn't have that in the html
  module
- Parsing multipart/form-data
- Figuring out the charset for json

We keep the first one - if loading escape_html from html fails we fall
back to cgi.

We remove the second - I can't find any case where we use
multipart/form-data. Any place we post data we either explicitly pass
application/x-www-form-urlencoded or implicitly use application/json.

The third is the tricky bit. This drops charset detection under the
assumption that we're never going to encounter anything other than
utf-8 (or ascii, which is a utf-8 subset). I'm not sure that holds,
but if it doesn't we can just add a regex to parse the charset.
@faho faho added this to the fish 3.2.0 milestone Jan 2, 2021
@faho faho merged commit e332555 into fish-shell:master Jan 3, 2021
Copy link
Member

@zanchey zanchey left a comment

Choose a reason for hiding this comment

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

This looks good, though I've made some small suggestions below

share/tools/web_config/webconfig.py Show resolved Hide resolved
share/tools/web_config/webconfig.py Show resolved Hide resolved
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 2, 2021
@faho faho deleted the python-no-cgi branch September 23, 2021 09:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants