zopefoundation / zope.publisher

Map requests from HTTP/WebDAV clients, web browsers, XML-RPC and FTP clients onto Python objects
Other
3 stars 13 forks source link

Fix POST with large values on Python 3 #16

Closed jinty closed 7 years ago

jinty commented 7 years ago

Results in a traceback looking like this:

Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/zope/publisher/publish.py", line 127, in publish
    request.processInputs()
  File "/usr/lib/python3/dist-packages/zope/publisher/browser.py", line 318, in processInputs
    keep_blank_values=1, **args)
  File "/usr/lib/python3.5/cgi.py", line 559, in __init__
    self.read_multi(environ, keep_blank_values, strict_parsing)
  File "/usr/lib/python3.5/cgi.py", line 730, in read_multi
    self.encoding, self.errors)
  File "/usr/lib/python3.5/cgi.py", line 561, in __init__
    self.read_single()
  File "/usr/lib/python3.5/cgi.py", line 743, in read_single
    self.read_lines()
  File "/usr/lib/python3.5/cgi.py", line 772, in read_lines
    self.read_lines_to_outerboundary()
  File "/usr/lib/python3.5/cgi.py", line 848, in read_lines_to_outerboundary
    self.__write(odelim + line)
  File "/usr/lib/python3.5/cgi.py", line 782, in __write
    self.file.write(data)
  File "/usr/lib/python3.5/tempfile.py", line 622, in func_wrapper
    return func(*args, **kwargs)
TypeError: a bytes-like object is required, not 'str'
jinty commented 7 years ago

On Thu, Mar 02, 2017 at 02:23:33AM -0800, Marius Gedminas wrote:

mgedmin approved this pull request.

LGTM, with only minor stylistic comments.

:)

Ok Ok, I'll stop being lazy

I'd love to see a traceback of the failure this is fixing somewhere in the pull request description.

@@ -613,7 +613,11 @@ def get(self, key, default=None): class ZopeFieldStorage(FieldStorage):

 def make_file(self, binary=None):
  • return tempfile.NamedTemporaryFile('w+b')
  • if PYTHON2 or self._binary_file:
  • return tempfile.NamedTemporaryFile("w+b")
  • else:
  • return tempfile.NamedTemporaryFile("w+",
  • encoding=self.encoding, newline = '\n')

Drop the spaces around = here?

Ok, though I'll note those spaces are in the stdlib code I cribbed from. Not that that's any excuse

@@ -220,6 +227,38 @@ def testFileUploadPost(self):

Test that we can actually read the file data

self.assertEqual(request.form['upload'].read(), b'Some data')

  • def testFileUploadPost(self):
  • """Produce a Fieldstorage with a file handle that exposes
  • its filename."""

Remove the docstring? It's not accurate, and some test runners like to use __doc__ instead of __name__, obscuring which test it is running. This is especially bad when you have multiple tests with the same docstring, which is the case here.

Ok

  • boundary=---------------------------1'}
  • request = self._createRequest(extra, body=LARGE_FILE_BODY)
  • request.processInputs()
  • self.assertTrue(request.form['upload'].name)
  • request = self._createRequest(extra, body=IE_FILE_BODY)
  • request.processInputs()
  • self.assertEqual(request.form['upload'].filename, 'notepad.exe')
  • Test that we can actually read the file data

  • self.assertEqual(request.form['upload'].read(), b'Some data')
  • def testLargePost(self):
  • """Produce a Fieldstorage with a file handle that exposes
  • its filename."""

Same docstring as previous test.

Ok, I fixed this up. It's a much better pull request now. Thanks.

-- Brian Sutherland

mgedmin commented 7 years ago

LGTM!