Skip to content
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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python 2.7 FieldStorage is broken... #293

Closed
digitalresistor opened this issue Nov 18, 2016 · 3 comments · Fixed by #390
Closed

Python 2.7 FieldStorage is broken... #293

digitalresistor opened this issue Nov 18, 2016 · 3 comments · Fixed by #390

Comments

@digitalresistor
Copy link
Member

digitalresistor commented Nov 18, 2016

We have a work-around for Python issues:

https://bugs.python.org/issue23801
https://bugs.python.org/issue27777 (and as a side effect a workaround for: https://bugs.python.org/issue24764)

Now the issue is that as soon as I grabbed the test for FieldStorage from Python 3.6 (including my patchset for fixing 27777) things started failing on Python 2.7:

__________________________________________ Test_cgi_FieldStorage_Py3_tests.test_fieldstorage_part_content_length __________________________________________

self = <tests.test_compat.Test_cgi_FieldStorage_Py3_tests object at 0x1057fb990>

    def test_fieldstorage_part_content_length(self):
        from webob.compat import cgi_FieldStorage
        BOUNDARY = "JfISa01"
        POSTDATA = """--JfISa01
    Content-Disposition: form-data; name="submit-name"
    Content-Length: 5
    --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)
>       assert len(fs.list) == 1
E       assert 2 == 1
E        +  where 2 = len([FieldStorage('submit-name', None), FieldStorage(None, None)])
E        +    where [FieldStorage('submit-name', None), FieldStorage(None, None)] = FieldStorage(None, None, [FieldStorage('submit-name', None), FieldStorage(None, None)]).list

tests/test_compat.py:83: AssertionError
========================================================== 1 failed, 1106 passed in 5.80 seconds ==========================================================
    def test_fieldstorage_part_content_length(self):
        from webob.compat import cgi_FieldStorage
        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)
        assert len(fs.list) == 1
        assert fs.list[0].name == 'submit-name'
        assert fs.list[0].value == b'Larry'

Instead of being a single value, it is two. I wonder if we should just grab FieldStorage wholesale from Python 3.6...

This test is from https://bugs.python.org/issue24764!

@mmerickel got any advice here?

@digitalresistor digitalresistor changed the title Python 2.7 FieldStorage is really broken... Python 2.7 FieldStorage is broken... Nov 18, 2016
@asottile
Copy link
Contributor

asottile commented Mar 2, 2017

I think there's a typo in the example? the termination of multipart data should end in a --

This does what I expect:

        from webob.compat import cgi_FieldStorage
        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)
        assert len(fs.list) == 1
        assert fs.list[0].name == 'submit-name'
        assert fs.list[0].value == b'Larry'

@digitalresistor
Copy link
Member Author

@asottile Does that fix the issue so that the test passes on both Py2.7 and Py3.x?

That being said, the original example was taken from the Python bug tracker...

@asottile
Copy link
Contributor

asottile commented Mar 3, 2017

Based on https://tools.ietf.org/html/rfc2046#section-5.1, a closing boundary delimiter ends with -- -- I believe the example in the python bug report is not compliant and really shouldn't have caused a change in the stdlib.

Though to be honest, I'm not well versed in multipart form data and perhaps I'm completely off base :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants