Created on 2014-02-03 21:33 by srittau, last changed 2019-09-29 13:12 by ned.deily. This issue is now closed.
Consider the attached test case. This test will run fine with Python 2.7, but will fail with Python 3.3. If cgi.FieldStorage() tries to parse a multipart request without a Content-Length header in the main section, segments will have a length of 0.
During the parse process, two instances of FieldStorage are involved. The outer one reads the whole request and creates and delegates reading of the fragment to inner instances.
The main problem is that FieldStorage.read_lines_to_outerboundary() of the inner FieldStorage will read nothing, since self.limit is lower than zero.
def read_lines_to_outerboundary(self):
...
while 1:
if _read >= self.limit:
break
...
This happens, since limit is passed when creating the inner instance in FieldStorage.read_multi():
def read_multi(self, environ, keep_blank_values, strict_parsing):
...
part = klass(self.fp, headers, ib, environ, keep_blank_values,
strict_parsing,self.limit-self.bytes_read,
self.encoding, self.errors)
...
Now, if the total request did not have a Content-Length header, self.limit will be -1.
The naive fix works for the test case, at least, but I don't know if there are other repercussions:
--- /usr/lib/python3.3/cgi.py 2014-02-03 22:31:16.649431125 +0100
+++ cgi.py 2014-02-03 22:32:14.849704379 +0100
@@ -788,7 +788,7 @@
last_line_lfend = True
_read = 0
while 1:
- if _read >= self.limit:
+ if self.limit >= 0 and _read >= self.limit:
break
line = self.fp.readline(1<<16) # bytes
self.bytes_read += len(line)
Actually, the problem is cgi.py around line 550:
clen = -1
if 'content-length' in self.headers:
try:
clen = int(self.headers['content-length'])
except ValueError:
pass
if maxlen and clen > maxlen:
raise ValueError('Maximum content length exceeded')
self.length = clen
if self.limit is None and clen:
self.limit = clen
… so self.limit ends up being -1 instead of None. :-/
Somebody please change this test to
if self.limit is None and clen >= 0:
Patch attached.
This also applies to 3.4 and 3.5.
Thanks for the report, the test case, and the patch. It would be helpful if the informal test could be turned into a patch as a formal test in Lib/test_cgi.py and if, Matthias, you would be willing to sign the PSF contributor agreement if you haven't already (https://www.python.org/psf/contrib/contrib-form/).
Is there any progress on this? The fix seems trivial.
The attached patch(cgi.patch) doesn't fix the problem for me: cgi-bug.py still fails with a TypeError. Here is a patch with a test to fix the problem. With issue20504.diff applied: $ ./python t.py 5 (Only changed the "assert len(fields["my-arg"].file.read()) == 5" line with "print(len(fields["my-arg"].file.read()))")
See also issue #24764: "cgi.FieldStorage.read_multi ignores Content-Length" (changeset 11e9f34169d1).
I reviewed issue20504.diff on Rietveld.
I'm just going to ping on this issue. It looks like this has just slipped off the radar. I've seen the last diff and the code review, but it seems that this just needs some final follow-up on the code review comments, no? I could easily do the final cleanup myself, but would prefer to let someone already on this thread take it across the finish line since I didn't do any of the real work for this patch. If really necessary, I may be able to clean this up if no one else can spare the time.
Owch, yeah, this fell off the radar. Anyway, I've signed the CLA, so if somebody could finish and apply this I'd be grateful. Myself, I unfortunately don't have the time.
I have submitted PR #10638 to fix this issue.
New changeset 2d7cacacc310b65b43e7e2de89e7722291dea6a4 by Benjamin Peterson (Pierre Quentel) in branch 'master': bpo-20504 : in cgi.py, fix bug when a multipart/form-data request has… (#10638) https://github.com/python/cpython/commit/2d7cacacc310b65b43e7e2de89e7722291dea6a4
New changeset e3bd941e4e6f4465f17a0e5a4a6bdf4ea0afdd0d by Miss Islington (bot) in branch '3.8': bpo-20504 : in cgi.py, fix bug when a multipart/form-data request has… (GH-10638) https://github.com/python/cpython/commit/e3bd941e4e6f4465f17a0e5a4a6bdf4ea0afdd0d
New changeset 99f0e81f43f64b83e18e8cb2a0b66c53a81a74ab by Miss Islington (bot) in branch '3.7': bpo-20504 : in cgi.py, fix bug when a multipart/form-data request has… (GH-10638) https://github.com/python/cpython/commit/99f0e81f43f64b83e18e8cb2a0b66c53a81a74ab
Now that the PR has been merged, can someone close the issue ?
messages: + msg253303
stage: patch review