[issue24764] cgi.FieldStorage can't parse multipart part headers with Content-Length and no filename in Content-Disposition

[Rhodri James]

Change by Rhodri James :


--
nosy:  -Rhodri James

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24764] cgi.FieldStorage can't parse multipart part headers with Content-Length and no filename in Content-Disposition

[Ethan Furman]

Change by Ethan Furman :


--
assignee:  -> ethan.furman
nosy: +Rhodri James, ethan.furman

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24764] cgi.FieldStorage can't parse multipart part headers with Content-Length and no filename in Content-Disposition

[Bert JW Regeer]

Bert JW Regeer added the comment:

I've uploaded a patchset to bug #2 that fixes this issue by fixing 
make_file, and doesn't cause Python to throw out the content-length information.

It also fixes FieldStorage for when you provide it a non-multipart form 
submission and there is no list in FS.

Please see http://bugs.python.org/issue2

--
nosy: +X-Istence

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24764] cgi.FieldStorage can't parse multipart part headers with Content-Length and no filename in Content-Disposition

[STINNER Victor]

Changes by STINNER Victor victor.stin...@gmail.com:


--
resolution:  - fixed
status: open - closed

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24764
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24764] cgi.FieldStorage can't parse multipart part headers with Content-Length and no filename in Content-Disposition

[STINNER Victor]

STINNER Victor added the comment:

I applied the fix, thanks Peter for the report and the fix, thanks Pierre for 
the review.

 https://bugs.launchpad.net/barbican/+bug/1485452, 
 is problem mentioned in the bug is same with mentioned bug?

I don't know, but you can try to apply the patch locally if you want, or 
download the Python 3.4 using Mercurial.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24764
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24764] cgi.FieldStorage can't parse multipart part headers with Content-Length and no filename in Content-Disposition

[Roundup Robot]

Roundup Robot added the comment:

New changeset 11e9f34169d1 by Victor Stinner in branch '3.4':
cgi.FieldStorage.read_multi ignores Content-Length
https://hg.python.org/cpython/rev/11e9f34169d1

New changeset 5b9209e4c3e4 by Victor Stinner in branch '3.5':
(Merge 3.4) cgi.FieldStorage.read_multi ignores Content-Length
https://hg.python.org/cpython/rev/5b9209e4c3e4

New changeset 0ff1acc89cf0 by Victor Stinner in branch 'default':
(Merge 3.5) cgi.FieldStorage.read_multi ignores Content-Length
https://hg.python.org/cpython/rev/0ff1acc89cf0

--
nosy: +python-dev

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24764
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24764] cgi.FieldStorage can't parse multipart part headers with Content-Length and no filename in Content-Disposition

[Peter Landry]

Peter Landry added the comment:

Pradeep, that error seems to be in Barbican. This bug and patch only addresses 
content-length headers in MIME multipart messages.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24764
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24764] cgi.FieldStorage can't parse multipart part headers with Content-Length and no filename in Content-Disposition

[Pradeep]

Pradeep added the comment:

Hi Victor, 
I found similar problem at 
https://bugs.launchpad.net/barbican/+bug/1485452, 
is problem mentioned in the bug is same with mentioned bug?

--
nosy: +pradeep

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24764
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24764] cgi.FieldStorage can't parse multipart part headers with Content-Length and no filename in Content-Disposition

[Berker Peksag]

Changes by Berker Peksag berker.pek...@gmail.com:


--
nosy: +berker.peksag
stage:  - patch review
type:  - behavior
versions:  -Python 3.2, Python 3.3

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24764
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24764] cgi.FieldStorage can't parse multipart part headers with Content-Length and no filename in Content-Disposition

[Pierre Quentel]

Pierre Quentel added the comment:

Victor, you can apply the patch and close the issue.
Le 7 août 2015 17:12, Peter Landry rep...@bugs.python.org a écrit :


 Peter Landry added the comment:

 A new patch that simply removes Content-Length from part headers when
 present.

 --
 Added file: http://bugs.python.org/file40145/cgi_multipart.patch

 ___
 Python tracker rep...@bugs.python.org
 http://bugs.python.org/issue24764
 ___


--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24764
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24764] cgi.FieldStorage can't parse multipart part headers with Content-Length and no filename in Content-Disposition

[STINNER Victor]

STINNER Victor added the comment:

Not today. I'm in holiday. Ping me in two weeks or find another core dev.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24764
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24764] cgi.FieldStorage can't parse multipart part headers with Content-Length and no filename in Content-Disposition

[Pierre Quentel]

Pierre Quentel added the comment:

I don't really see why there is a Content-Length in the headers of a
multipart form data. The specification at
http://www.w3.org/TR/html401/interact/forms.html#h-17.13.4.2 doesn't
mention it, and it is absent in the example that looks like the one tested
by Peter :

Content-Type: multipart/form-data; boundary=AaB03x
--AaB03x
Content-Disposition: form-data; name=submit-name

Larry
--AaB03x
Content-Disposition: form-data; name=files; filename=file1.txt
Content-Type: text/plain
... contents of file1.txt ...
--AaB03x--

In case a user agent would insert it, I think the best would be to
ignore it. That is, inside read_multi(), after

headers = parser.close()

add these lines :

if 'content-length' in headers:
del headers['content-length']

It's hard to see the potential side effects but I think it's cleaner
than the proposed patch, which is not correct anyway for another
reason : the attribute value is set to a bytes objects, instead of a
string.

Peter, does this make sense ? If so, can you submit another patch ?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24764
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24764] cgi.FieldStorage can't parse multipart part headers with Content-Length and no filename in Content-Disposition

[Peter Landry]

Peter Landry added the comment:

Yeah, I think that makes the most sense to me as well. I tried to make a 
minimum-impact patch, but this feels cleaner.

If we remove the Content-Length header, the `limit` kwarg might occur at an odd 
place in the part itself, but that feels unavoidable if someone submits an 
incorrect Content-Length for the request itself.

I'll re-work the patch and make sure the tests I added still add value. Thanks!

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24764
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24764] cgi.FieldStorage can't parse multipart part headers with Content-Length and no filename in Content-Disposition

[Peter Landry]

Peter Landry added the comment:

A new patch that simply removes Content-Length from part headers when present.

--
Added file: http://bugs.python.org/file40145/cgi_multipart.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24764
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24764] cgi.FieldStorage can't parse multipart part headers with Content-Length and no filename in Content-Disposition

[Pierre Quentel]

Pierre Quentel added the comment:

Yes, I will be able to review the patch next week

2015-07-31 18:13 GMT+02:00 STINNER Victor rep...@bugs.python.org:


 STINNER Victor added the comment:

 @Pierre Quentel: Hi! Are you still working on CGI? Can you please review
 this patch? Thanks.

 --

 Previous large change in the cgi module: issue #4953. Pierre helped a lot
 on this one.

 --
 nosy: +quentel

 ___
 Python tracker rep...@bugs.python.org
 http://bugs.python.org/issue24764
 ___


--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24764
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24764] cgi.FieldStorage can't parse multipart part headers with Content-Length and no filename in Content-Disposition

[STINNER Victor]

STINNER Victor added the comment:

@Pierre Quentel: Hi! Are you still working on CGI? Can you please review this 
patch? Thanks.

--

Previous large change in the cgi module: issue #4953. Pierre helped a lot on 
this one.

--
nosy: +quentel

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24764
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24764] cgi.FieldStorage can't parse multipart part headers with Content-Length and no filename in Content-Disposition

[Peter Landry]

Peter Landry added the comment:

I realized my formatting was poor, making it hard to quickly test the issue. 
Here's a cleaner version:

import cgi
from io import BytesIO

BOUNDARY = JfISa01
POSTDATA = --JfISa01
Content-Disposition: form-data; name=submit-name
Content-Length: 5

Larry
--JfISa01
env = {
'REQUEST_METHOD': 'POST',
'CONTENT_TYPE': 'multipart/form-data; boundary={}'.format(BOUNDARY),
'CONTENT_LENGTH': str(len(POSTDATA))}
fp = BytesIO(POSTDATA.encode('latin-1'))
fs = cgi.FieldStorage(fp, environ=env, encoding=latin-1)

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24764
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24764] cgi.FieldStorage can't parse multipart part headers with Content-Length and no filename in Content-Disposition

[Peter Landry]

New submission from Peter Landry:

`cgi.FieldStorage` can't parse a multipart with a `Content-Length` header set 
on a part:

```Python 3.4.3 (default, May 22 2015, 15:35:46)
[GCC 4.2.1 Compatible Apple LLVM 6.1.0 (clang-602.0.49)] on darwin
Type help, copyright, credits or license for more information.
 import cgi
 from io import BytesIO

 BOUNDARY = JfISa01
 POSTDATA = --JfISa01
... Content-Disposition: form-data; name=submit-name
... Content-Length: 5
...
... Larry
... --JfISa01
 env = {
... 'REQUEST_METHOD': 'POST',
... 'CONTENT_TYPE': 'multipart/form-data; boundary={}'.format(BOUNDARY),
... 'CONTENT_LENGTH': str(len(POSTDATA))}
 fp = BytesIO(POSTDATA.encode('latin-1'))
 fs = cgi.FieldStorage(fp, environ=env, encoding=latin-1)
Traceback (most recent call last):
  File stdin, line 1, in module
  File 
/usr/local/Cellar/python3/3.4.3/Frameworks/Python.framework/Versions/3.4/lib/python3.4/cgi.py,
 line 571, in __init__
self.read_multi(environ, keep_blank_values, strict_parsing)
  File 
/usr/local/Cellar/python3/3.4.3/Frameworks/Python.framework/Versions/3.4/lib/python3.4/cgi.py,
 line 726, in read_multi
self.encoding, self.errors)
  File 
/usr/local/Cellar/python3/3.4.3/Frameworks/Python.framework/Versions/3.4/lib/python3.4/cgi.py,
 line 573, in __init__
self.read_single()
  File 
/usr/local/Cellar/python3/3.4.3/Frameworks/Python.framework/Versions/3.4/lib/python3.4/cgi.py,
 line 736, in read_single
self.read_binary()
  File 
/usr/local/Cellar/python3/3.4.3/Frameworks/Python.framework/Versions/3.4/lib/python3.4/cgi.py,
 line 758, in read_binary
self.file.write(data)
TypeError: must be str, not bytes

```

This happens because of a mismatch between the code that creates a temp file to 
write to and the code that chooses to read in binary mode or not:

* the presence of `filename` in the `Content-Disposition` header triggers 
creation of a binary mode file
* the present of a `Content-Length` header for the part triggers a binary read

When `Content-Length` is present but `filename` is absent, `bytes` are written 
to the non-binary temp file, causing the error above.

I've reviewed the relevant RFCs, and I'm not really sure what the correct way 
to handle this is. I don't believe `Content-Length` is addressed for part 
bodies in the MIME spec[0], and HTTP has its own semantics[1].

At the very least, I think this behavior is confusing and unexpected. Some 
libraries, like Retrofit[2], will by default include `Content-Length`, and 
break when submitting POST data to a python server.

I've made an attempt to work in the way I'd expect, and attached a patch, but 
I'm really not sure if it's the proper decision. My patch kind of naively 
accepts the existing semantics of `Content-Length` that presume bytes, and 
treats the creation of a non-binary file as the bug.

[0]: http://www.ietf.org/rfc/rfc2045.txt
[1]: http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.4
[2]: http://square.github.io/retrofit/

--
components: Library (Lib)
files: cgi_multipart.patch
keywords: patch
messages: 247751
nosy: Peter Landry, haypo
priority: normal
severity: normal
status: open
title: cgi.FieldStorage can't parse multipart part headers with Content-Length 
and no filename in Content-Disposition
versions: Python 3.2, Python 3.3, Python 3.4, Python 3.5, Python 3.6
Added file: http://bugs.python.org/file40084/cgi_multipart.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24764
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com