bpo-34226: fix cgi.parse_multipart without content_length #8530

Roger · 2018-07-28T15:07:10Z

In Python 3.7 the behavior of parse_multipart changed requiring CONTENT-LENGTH
header, this fix remove this header as required and fix FieldStorage
read_lines_to_outerboundary, by not using limit when it's negative,
since by default it's -1 if not content-length and keeps substracting what
was read from the file object.

Also added a test case for this problem.

https://bugs.python.org/issue34226

Automerge-Triggered-By: @orsenthil

pablogsal · 2018-07-28T15:13:08Z

Lib/cgi.py

@@ -717,7 +720,7 @@ def read_lines_to_outerboundary(self):
        last_line_lfend = True
        _read = 0
        while 1:
-            if _read >= self.limit:
+            if self.limit >= 0 and _read >= self.limit:


Please, simplify this to:

if 0 <= self.limit <= _read

done, thanks!

yan12125 · 2018-07-29T06:25:07Z

Thanks a lot for this PR! Here are my two cents:

Looks like a NEWS entry is necessary
It would be great to backport this fix to the 3.7 branch. I think that can be done by adding a needs backport to 3.7 label by a core developer?

zooba · 2018-07-29T09:44:34Z

This looks good - we just need a NEWS entry. Click on "Details" next to the failed check for info on how to install and use blurb. (The title of this PR - properly captialised - will be a fine entry, and it can go in the "Library" category.)

Roger · 2018-07-30T03:12:49Z

Should I backport to python 3.7?

yan12125 · 2018-07-30T03:19:24Z

Normally backporting will be carried out automatically by nice bots after the PR for master is merged :)

After the pull request has been merged, miss-islington (bot) will first try to do the backport automatically. In case that miss-islington is unable to do it, then the pull request author or the core developer who merged it should look into backporting it themselves, using the backport generated by cherry_picker.py as a starting point.

(From https://devguide.python.org/committing/#backporting-changes-to-an-older-version)

pablogsal · 2018-07-30T19:39:15Z

Misc/NEWS.d/next/Library/2018-07-29-12-14-54.bpo-34226.BE7zbu.rst

@@ -0,0 +1 @@
+Fix cgi.parse_multipart without content_length


Use formatting for cgi.parse_multipart:

cgi.parse_multipart -> cgi.parse_multipart.

Also, add "Patch by Roger Duran" at the end.

yan12125 · 2018-07-31T02:09:39Z

Lib/test/test_cgi.py

@@ -129,6 +129,20 @@ def test_parse_multipart(self):
                    'file': [b'Testing 123.\n'], 'title': ['']}
        self.assertEqual(result, expected)

+    def test_parse_multipart_without_content_legth(self):


Hi, here's a typo => test_parse_multipart_without_content_legth should be test_parse_multipart_without_content_length (missing "n")

Good catch!, fixed

yan12125 · 2018-08-06T07:30:26Z

@pablogsal @zooba Is there anything to do for this PR? As per PEP 537:

3.7.1: 2018-07-xx

I guess 3.7.1 is coming soon. I hope this PR can get merged before that.

yan12125 · 2018-09-11T03:30:15Z

The updated PEP 537 states that 3.7.1rc1 is coming the next week

3.7.1 candidate 1: 2018-09-18

Can this patch be reviewed and merged before that?

Roger · 2018-11-04T19:05:56Z

Hi, just notice that this patch it's not yet merged, is there any change to be done?

victorzhuk · 2018-11-12T19:19:32Z

If you are waiting this PR as me, temporary solution CONTENT_LENGTH=0 python app.py or define CONTENT_LENGTH=0 env inside python script

YtvwlD · 2019-08-09T18:30:00Z

Hey, this patch is still not merged. What is missing? (This is preventing me from upgrading an app to 3.7.)

Also, the workaround CONTENT_LENGTH=0 doesn't really work for me as it creates now a different exception.

Also add a reference to python/cpython#8530

srinivasreddy · 2020-06-12T11:05:10Z

@Roger Could you please resolve conflicts?

In Python 3.7 the behavior of parse_multipart changed requiring CONTENT-LENGTH header, this fix remove this header as required and fix FieldStorage read_lines_to_outerboundary, by not using limit when it's negative, since by default it's -1 if not content-length and keeps substracting what was read from the file object. Also added a test case for this problem.

Roger · 2020-06-12T13:57:31Z

@Roger Could you please resolve conflicts?

sure

orsenthil · 2020-06-15T14:57:43Z

I like this change. It looks like in #991 while concentrating on encoding, I missed to notice that the variable being looked for was CONTENT-LENGTH instead of CONTENT_LENGTH. Sad.

This change will address that regression introduced in 3.7. I will merge this and set the label to backport to 3.7

I verified the logic of this change.
Verified the test suite that we are good with this change.

miss-islington · 2020-06-15T14:58:58Z

Thanks @Roger for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8, 3.9.
🐍🍒⛏🤖

bedevere-bot · 2020-06-15T14:59:10Z

GH-20890 is a backport of this pull request to the 3.9 branch.

) In Python 3.7 the behavior of parse_multipart changed requiring CONTENT-LENGTH header, this fix remove this header as required and fix FieldStorage read_lines_to_outerboundary, by not using limit when it's negative, since by default it's -1 if not content-length and keeps substracting what was read from the file object. Also added a test case for this problem. (cherry picked from commit d8cf351) Co-authored-by: roger <rogerduran@gmail.com>

bedevere-bot · 2020-06-15T14:59:18Z

GH-20891 is a backport of this pull request to the 3.8 branch.

) In Python 3.7 the behavior of parse_multipart changed requiring CONTENT-LENGTH header, this fix remove this header as required and fix FieldStorage read_lines_to_outerboundary, by not using limit when it's negative, since by default it's -1 if not content-length and keeps substracting what was read from the file object. Also added a test case for this problem. (cherry picked from commit d8cf351) Co-authored-by: roger <rogerduran@gmail.com>

bedevere-bot · 2020-06-15T14:59:26Z

GH-20892 is a backport of this pull request to the 3.7 branch.

) In Python 3.7 the behavior of parse_multipart changed requiring CONTENT-LENGTH header, this fix remove this header as required and fix FieldStorage read_lines_to_outerboundary, by not using limit when it's negative, since by default it's -1 if not content-length and keeps substracting what was read from the file object. Also added a test case for this problem. (cherry picked from commit d8cf351) Co-authored-by: roger <rogerduran@gmail.com>

@ned-deily

) (GH-20892) In Python 3.7 the behavior of parse_multipart changed requiring CONTENT-LENGTH header, this fix remove this header as required and fix FieldStorage read_lines_to_outerboundary, by not using limit when it's negative, since by default it's -1 if not content-length and keeps substracting what was read from the file object. Also added a test case for this problem. (cherry picked from commit d8cf351) Co-authored-by: roger <rogerduran@gmail.com> Automerge-Triggered-By: @ned-deily

In Python 3.7 the behavior of parse_multipart changed requiring CONTENT-LENGTH header, this fix remove this header as required and fix FieldStorage read_lines_to_outerboundary, by not using limit when it's negative, since by default it's -1 if not content-length and keeps substracting what was read from the file object. Also added a test case for this problem. (cherry picked from commit d8cf351) Co-authored-by: roger <rogerduran@gmail.com>

) In Python 3.7 the behavior of parse_multipart changed requiring CONTENT-LENGTH header, this fix remove this header as required and fix FieldStorage read_lines_to_outerboundary, by not using limit when it's negative, since by default it's -1 if not content-length and keeps substracting what was read from the file object. Also added a test case for this problem.

bedevere-bot added the awaiting review label Jul 28, 2018

the-knights-who-say-ni added the CLA signed label Jul 28, 2018

pablogsal reviewed Jul 28, 2018

View reviewed changes

Roger force-pushed the fix_cgi_parse_multipart_without_content_length branch from 6275f1a to 1e085da Compare July 28, 2018 15:25

yan12125 mentioned this pull request Jul 29, 2018

Add Python 3.7 support and enable testing twisted/treq#228

Merged

zooba added needs backport to 3.7 type-bug An unexpected behavior, bug, or error sprint labels Jul 29, 2018

pablogsal reviewed Jul 30, 2018

View reviewed changes

Roger force-pushed the fix_cgi_parse_multipart_without_content_length branch from 7243923 to bd4f196 Compare July 30, 2018 21:04

yan12125 reviewed Jul 31, 2018

View reviewed changes

Roger force-pushed the fix_cgi_parse_multipart_without_content_length branch from bd4f196 to 68e9502 Compare July 31, 2018 07:21

skwashd mentioned this pull request Jul 29, 2019

multipart/form-data support in AWS Chalice aws/chalice#796

Open

twm added a commit to twisted/twisted that referenced this pull request Dec 16, 2019

Improve comment

f9cbd33

Also add a reference to python/cpython#8530

rodrigc pushed a commit to twisted/twisted that referenced this pull request Mar 28, 2020

Improve comment

9d2f80d

Also add a reference to python/cpython#8530

Roger added 2 commits June 12, 2020 15:44

bpo-34226: add news file

1330f8c

Roger force-pushed the fix_cgi_parse_multipart_without_content_length branch from 68e9502 to 1330f8c Compare June 12, 2020 14:53

Roger requested a review from ethanfurman as a code owner June 12, 2020 14:53

orsenthil approved these changes Jun 15, 2020

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting review labels Jun 15, 2020

orsenthil added needs backport to 3.8 needs backport to 3.9 only security fixes labels Jun 15, 2020

miss-islington merged commit d8cf351 into python:master Jun 15, 2020

bedevere-bot removed awaiting merge needs backport to 3.9 only security fixes labels Jun 15, 2020

bedevere-bot removed the needs backport to 3.8 label Jun 15, 2020

bedevere-bot removed the needs backport to 3.7 label Jun 15, 2020

sandhose mentioned this pull request Oct 26, 2022

Support OIDC backchannel logouts matrix-org/synapse#11414

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-34226: fix cgi.parse_multipart without content_length #8530

bpo-34226: fix cgi.parse_multipart without content_length #8530

Roger commented Jul 28, 2018 •

edited by bedevere-bot

Loading

pablogsal Jul 28, 2018

Roger Jul 28, 2018

yan12125 commented Jul 29, 2018

zooba commented Jul 29, 2018

Roger commented Jul 30, 2018

yan12125 commented Jul 30, 2018

pablogsal Jul 30, 2018

Roger Jul 30, 2018

yan12125 Jul 31, 2018

Roger Jul 31, 2018

yan12125 commented Aug 6, 2018

yan12125 commented Sep 11, 2018

Roger commented Nov 4, 2018

victorzhuk commented Nov 12, 2018

YtvwlD commented Aug 9, 2019

srinivasreddy commented Jun 12, 2020

Roger commented Jun 12, 2020

orsenthil commented Jun 15, 2020

miss-islington commented Jun 15, 2020

bedevere-bot commented Jun 15, 2020

bedevere-bot commented Jun 15, 2020

bedevere-bot commented Jun 15, 2020

bpo-34226: fix cgi.parse_multipart without content_length #8530

bpo-34226: fix cgi.parse_multipart without content_length #8530

Conversation

Roger commented Jul 28, 2018 • edited by bedevere-bot Loading

pablogsal Jul 28, 2018

Choose a reason for hiding this comment

Roger Jul 28, 2018

Choose a reason for hiding this comment

yan12125 commented Jul 29, 2018

zooba commented Jul 29, 2018

Roger commented Jul 30, 2018

yan12125 commented Jul 30, 2018

pablogsal Jul 30, 2018

Choose a reason for hiding this comment

Roger Jul 30, 2018

Choose a reason for hiding this comment

yan12125 Jul 31, 2018

Choose a reason for hiding this comment

Roger Jul 31, 2018

Choose a reason for hiding this comment

yan12125 commented Aug 6, 2018

yan12125 commented Sep 11, 2018

Roger commented Nov 4, 2018

victorzhuk commented Nov 12, 2018

YtvwlD commented Aug 9, 2019

srinivasreddy commented Jun 12, 2020

Roger commented Jun 12, 2020

orsenthil commented Jun 15, 2020

miss-islington commented Jun 15, 2020

bedevere-bot commented Jun 15, 2020

bedevere-bot commented Jun 15, 2020

bedevere-bot commented Jun 15, 2020

Roger commented Jul 28, 2018 •

edited by bedevere-bot

Loading