[3.6] bpo-42967: only use '&' as a query string separator (GH-24297) by orsenthil · Pull Request #24532 · python/cpython (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation7 Commits3 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

orsenthil

@AdamGold @orsenthil

… urllib.parse.parse_qsl().

urllib.parse will only us "&" as query string separator by default instead of both ";" and "&" as allowed in earlier versions. An optional argument seperator with default value "&" is added to specify the separator.

Co-authored-by: Éric Araujo merwok@netwok.org Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Ken Jin 28750310+Fidget-Spinner@users.noreply.github.com Co-authored-by: Éric Araujo merwok@netwok.org. (cherry picked from commit fcbe0cb)

Co-authored-by: Adam Goldschmidt adamgold7@gmail.com

@orsenthil

The idle test case failing seems more like an unrelated flake to me.

@Fidget-Spinner

AdamGold

@@ -156,7 +160,7 @@ def parse(fp=None, environ=os.environ, keep_blank_values=0, strict_parsing=0):
if environ['REQUEST_METHOD'] == 'POST':
ctype, pdict = parse_header(environ['CONTENT_TYPE'])
if ctype == 'multipart/form-data':
return parse_multipart(fp, pdict)
return parse_multipart(fp, pdict, separator=separator)

Choose a reason for hiding this comment

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

I believe we should change the parse_multipart signature here

Choose a reason for hiding this comment

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

Thank you!. Yes, I had noticed in my local, but then lost it. :( - And unfortunately, the existing tests didn't cover to catch too.

@orsenthil

@AdamGold

We are adding a separator parameter to FieldStorage but I can not see any call to FieldStorage. In other versions, that call is within parse_multipart (therefore the signature changed). I think we can remove separator from FieldStorage altogether (but there are tests for it in this version - so that's odd).

@Fidget-Spinner

@orsenthil

We are adding a separator parameter to FieldStorage but I can not see any call to FieldStorage.

It is being called from test() function, which gets called if cgi.py is used as __main__. There is a path to usage, so leaving the changes and sending to parse_qsl in the FieldStorage seems okay to me. (instead of removing it).

@orsenthil

Hi Ned, the patch against 3.6 is complete. You could merge this when you get a chance and cut the release. Thank you.

gentoo-bot pushed a commit to gentoo/cpython that referenced this pull request

Mar 4, 2021

@orsenthil @mgorny

…4297) (pythonGH-24532)

bpo-42967: [security] Address a web cache-poisoning issue reported in urllib.parse.parse_qsl().

urllib.parse will only us "&" as query string separator by default instead of both ";" and "&" as allowed in earlier versions. An optional argument seperator with default value "&" is added to specify the separator.

Co-authored-by: Éric Araujo merwok@netwok.org Co-authored-by: Ken Jin 28750310+Fidget-Spinner@users.noreply.github.com Co-authored-by: Adam Goldschmidt adamgold7@gmail.com

Rebased for Python 2.7 by Michał Górny