[ python-Bugs-1112856 ] patch 1079734 broke cgi.FieldStorage w/ multipart post req.

[SourceForge.net]

Bugs item #1112856, was opened at 2005-01-31 00:58
Message generated for change (Comment added) made by arigo
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1112856&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Python Library
Group: Python 2.5
>Status: Closed
>Resolution: Fixed
Priority: 5
Submitted By: Irmen de Jong (irmen)
Assigned to: Nobody/Anonymous (nobody)
Summary: patch 1079734 broke cgi.FieldStorage w/ multipart post req.

Initial Comment:
Patch 1079734 "Make cgi.py use email instead of rfc822
or mimetools" seems to have broken the cgi.FieldStorage
in cases where the request is a multipart post (for
instance, when a file upload form field is used).
See the attached test program.

With cgi.py revision <1.83 (python 2.4 for instance) I
get the expected results;

374
FieldStorage(None, None, [FieldStorage('param1', None,
'Value of param1'), FieldStorage('param2', None, 'Value
of param2'), FieldStorage('file', '', ''),
FieldStorage(None, None, '')])

but with cgi.py rev 1.83 (current) I get this:

374
FieldStorage(None, None, [FieldStorage('param1', None,
'')])


Another thing that I observed (which isn't reproduced
by this test program) is that cgi.FieldStorage.__init__
never completes when the fp is a socket-file (and the
request is again a multipart post). It worked fine with
the old cgi.py.


----------------------------------------------------------------------

>Comment By: Armin Rigo (arigo)
Date: 2005-09-19 09:12

Message:
Logged In: YES 
user_id=4771

Reverted (Lib/cgi.py r1.85).

----------------------------------------------------------------------

Comment By: Josh Hoyt (joshhoyt)
Date: 2005-09-16 15:58

Message:
Logged In: YES 
user_id=693077

Yes, please revert the patch.

----------------------------------------------------------------------

Comment By: Irmen de Jong (irmen)
Date: 2005-09-16 13:40

Message:
Logged In: YES 
user_id=129426

I vote for revert, so that my code may run again on Python 2.5. 
Also see Josh's comment ,  with which I agree.

----------------------------------------------------------------------

Comment By: Armin Rigo (arigo)
Date: 2005-09-16 11:52

Message:
Logged In: YES 
user_id=4771

The problem is still there now (5 months later), so should I go ahead and revert the cgi.py changes of r1.83?  I does break user code out there.

----------------------------------------------------------------------

Comment By: Josh Hoyt (joshhoyt)
Date: 2005-04-17 23:39

Message:
Logged In: YES 
user_id=693077

I've been playing with using the email parser to do the
whole job of parsing the data, and I think it's too big of a
change. It'd be very hard to ensure API compatibility and
the same behaviour. I think that in the long run, it's the
right thing to do, but I think that the API should change
when that change is made. My recommendation for the time
being is to revert the original patch that I made.

----------------------------------------------------------------------

Comment By: Irmen de Jong (irmen)
Date: 2005-02-08 21:40

Message:
Logged In: YES 
user_id=129426

I've added a test that shows the 'freezing' problem I talked
about.
Start the server.py, it will listen on port 9000. Open the
post.html in your web browser, enter some form data, and
submit the form.
It will POST to the server.py and if you started that one
with cvs-python (2.5a0) it will freeze on the marked line.
If you start server.py with Python 2.4, it will work fine.

----------------------------------------------------------------------

Comment By: Irmen de Jong (irmen)
Date: 2005-02-06 22:58

Message:
Logged In: YES 
user_id=129426

Yes, I'll try to make a test case for that within the next
few days. 

----------------------------------------------------------------------

Comment By: Josh Hoyt (joshhoyt)
Date: 2005-02-04 23:02

Message:
Logged In: YES 
user_id=693077

Irmen, can you try to create a test case where the
cgi.FieldStorage never completes, so I can make sure that
any fix I come up with resolves it?

I will try to put together an implementation where the email
parser parses the whole multipart message.

----------------------------------------------------------------------

Comment By: Irmen de Jong (irmen)
Date: 2005-02-04 22:06

Message:
Logged In: YES 
user_id=129426

Johannes: while your patch makes my cgibug.py test case run
fine, it has 2 problems:
1- it runs much slower than the python2.4 code (probably
because of the reading back thing Josh is talking about);
2- it still doesn't fix the second problem that I observed:
cgi.FieldStorage never completes when fp is a socket. I
don't have a separate test case for this yet, sorry.

So Josh: perhaps your idea doesn't have these 2 problems?

----------------------------------------------------------------------

Comment By: Josh Hoyt (joshhoyt)
Date: 2005-02-04 15:45

Message:
Logged In: YES 
user_id=693077

Johannes, your patch looks fine to me. It would be nice if
we didn't have to keep reading back each part from the
parsed message, though.

I had an idea for another approach. Use email to parse the
MIME message fully, then convert it to FieldStorage fields.
Parsing could go something like:

== CODE ==

from email.FeedParser import FeedParser
parser = FeedParser()
# Create bogus content-type header...
parser.feed('Content-type: %s ; boundary=%s \r\n\r\n' %
(self.type, self.innerboundary))
parser.feed(self.fp.read())
message = parser.close()

# Then take parsed message and convert to FieldStorage fields

== END CODE ==

This lets the email parser handle all of the complexities of
MIME, but it does mean that we have to accurately re-create
all of the necessary headers. I can cook up a full patch if
anyone thinks this would fly.

----------------------------------------------------------------------

Comment By: Johannes Gijsbers (jlgijsbers)
Date: 2005-02-04 10:15

Message:
Logged In: YES 
user_id=469548

Here's a patch. We're interested in two things in the
patched loop: 

* the rest of the multipart/form-data, including the headers
for the current part, for consumption by HeaderParser (this
is the tail variable)
* the rest of the multipart/form-data without the headers
for the current part, for consumption by FieldStorage (this
is the message.get_payload() call)

Josh, Irmen, do you see any problems with this patch?

(BTW, this fix should be ported to the parse_multipart
function as well, when I check it in (and I should make
cgibug.py into a test))

----------------------------------------------------------------------

Comment By: Josh Hoyt (joshhoyt)
Date: 2005-02-04 07:21

Message:
Logged In: YES 
user_id=693077

The email.HeaderParser module that is now used for parsing
headers consumes the rest of the body of the message when it
is used. If there was some way of telling the parser to stop
reading when it got to the end of the headers, I think this
problem would be fixed. Unfortunately, there is no external
interface to the email module's parser that lets us do this.
I hope we can come up with a reasonable solution that does
not involve reverting to using deprecated modules.

Thanks for the test case.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1112856&group_id=5470