From 8e491e7ba5442e8e33b8b1f77fe0aa59db19fa10 Mon Sep 17 00:00:00 2001 From: Aron Podrigal Date: Tue, 5 Feb 2019 16:19:03 -0600 Subject: [PATCH 1/4] Fixes issue 27777: cgi.FieldStorage can't parse simple body with Content-Length and no Content-Disposition - If content length is present, read at most len bytes. - When reading `read_lines_to_eof` use `read` instead of `readline`. - Use `self.__write` even when content length is present and > 1000. --- Lib/cgi.py | 31 +++++++++++-------- .../2019-02-05-16-57-27.bpo-27777.AKUxDR.rst | 2 ++ 2 files changed, 20 insertions(+), 13 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2019-02-05-16-57-27.bpo-27777.AKUxDR.rst diff --git a/Lib/cgi.py b/Lib/cgi.py index 77ab703cc03600..f1aca48f23d5e0 100755 --- a/Lib/cgi.py +++ b/Lib/cgi.py @@ -667,7 +667,7 @@ def read_multi(self, environ, keep_blank_values, strict_parsing): def read_single(self): """Internal: read an atomic part.""" if self.length >= 0: - self.read_binary() + self.read_content() self.skip_lines() else: self.read_lines() @@ -675,31 +675,30 @@ def read_single(self): bufsize = 8*1024 # I/O buffering size for copy to file - def read_binary(self): - """Internal: read binary data.""" - self.file = self.make_file() + def read_content(self): + """Internal: read data up until content-length len.""" + if self.length > 1000: + self.file = self.make_file() + else: + self.file = self.io_object() todo = self.length if todo >= 0: while todo > 0: data = self.fp.read(min(todo, self.bufsize)) # bytes - if not isinstance(data, bytes): - raise ValueError("%s should return bytes, got %s" - % (self.fp, type(data).__name__)) self.bytes_read += len(data) if not data: self.done = -1 break - self.file.write(data) + self.__write(data) todo = todo - len(data) def read_lines(self): """Internal: read lines until EOF or outerboundary.""" - if self._binary_file: - self.file = self.__file = BytesIO() # store data as bytes for files - else: - self.file = self.__file = StringIO() # as strings for other fields + self.file = self.__file = self.io_object() if self.outerboundary: self.read_lines_to_outerboundary() + elif self.length >= 0: + self.read_content() else: self.read_lines_to_eof() @@ -721,7 +720,7 @@ def __write(self, line): def read_lines_to_eof(self): """Internal: read lines until EOF.""" while 1: - line = self.fp.readline(1<<16) # bytes + line = self.fp.read(1<<16) # bytes self.bytes_read += len(line) if not line: self.done = -1 @@ -830,6 +829,12 @@ def make_file(self): return tempfile.TemporaryFile("w+", encoding=self.encoding, newline = '\n') + def io_object(self): + if self._binary_file: + return BytesIO() # store data as bytes for files + else: + return StringIO() # as strings for other fields + # Test/debug code # =============== diff --git a/Misc/NEWS.d/next/Library/2019-02-05-16-57-27.bpo-27777.AKUxDR.rst b/Misc/NEWS.d/next/Library/2019-02-05-16-57-27.bpo-27777.AKUxDR.rst new file mode 100644 index 00000000000000..c967372ae7a9c8 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-02-05-16-57-27.bpo-27777.AKUxDR.rst @@ -0,0 +1,2 @@ +Fixes a bug in :mod:`cgi` when a request has a "Content-Length" header but +without "Content-Disposition" headers. From 74afddda794b1c2b0f29057f3b613edbf8a11469 Mon Sep 17 00:00:00 2001 From: Aron Podrigal Date: Sun, 2 Jun 2019 15:48:38 -0700 Subject: [PATCH 2/4] Updated test_cgi.py Renamed `test_fieldstorage_readline` to `test_fieldstorage_read` and test `read` instead of `readline` since we use `read`. --- Lib/test/test_cgi.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Lib/test/test_cgi.py b/Lib/test/test_cgi.py index 101942de947fb4..bc9c495d9c6dc3 100644 --- a/Lib/test/test_cgi.py +++ b/Lib/test/test_cgi.py @@ -215,22 +215,22 @@ def test_log(self): self.addCleanup(cgi.closelog) cgi.log("Testing log 4") - def test_fieldstorage_readline(self): - # FieldStorage uses readline, which has the capacity to read all - # contents of the input file into memory; we use readline's size argument - # to prevent that for files that do not contain any newlines in + def test_fieldstorage_read(self): + # FieldStorage uses read, which has the capacity to read all contents + # of the input file into memory; we use read's size argument to + # prevent that for files that do not contain any newlines in # non-GET/HEAD requests - class TestReadlineFile: + class TestReadFile: def __init__(self, file): self.file = file self.numcalls = 0 - def readline(self, size=None): + def read(self, size=None): self.numcalls += 1 if size: - return self.file.readline(size) + return self.file.read(size) else: - return self.file.readline() + return self.file.read() def __getattr__(self, name): file = self.__dict__['file'] @@ -239,14 +239,14 @@ def __getattr__(self, name): setattr(self, name, a) return a - f = TestReadlineFile(tempfile.TemporaryFile("wb+")) + f = TestReadFile(tempfile.TemporaryFile("wb+")) self.addCleanup(f.close) f.write(b'x' * 256 * 1024) f.seek(0) env = {'REQUEST_METHOD':'PUT'} fs = cgi.FieldStorage(fp=f, environ=env) self.addCleanup(fs.file.close) - # if we're not chunking properly, readline is only called twice + # if we're not chunking properly, read is only called twice # (by read_binary); if we are chunking properly, it will be called 5 times # as long as the chunksize is 1 << 16. self.assertGreater(f.numcalls, 2) From 1dd58c9333e391e6b3bf5bb6ecee8270dccc05b8 Mon Sep 17 00:00:00 2001 From: Aron Podrigal Date: Mon, 3 Jun 2019 10:06:15 -0500 Subject: [PATCH 3/4] Update 2019-02-05-16-57-27.bpo-27777.AKUxDR.rst Added mention of FieldStorage --- .../next/Library/2019-02-05-16-57-27.bpo-27777.AKUxDR.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2019-02-05-16-57-27.bpo-27777.AKUxDR.rst b/Misc/NEWS.d/next/Library/2019-02-05-16-57-27.bpo-27777.AKUxDR.rst index c967372ae7a9c8..9e8513dd208a30 100644 --- a/Misc/NEWS.d/next/Library/2019-02-05-16-57-27.bpo-27777.AKUxDR.rst +++ b/Misc/NEWS.d/next/Library/2019-02-05-16-57-27.bpo-27777.AKUxDR.rst @@ -1,2 +1,2 @@ -Fixes a bug in :mod:`cgi` when a request has a "Content-Length" header but +Fixes a bug in :mod:`cgi` :class:`cgi.FieldStorage` when a request has a "Content-Length" header but without "Content-Disposition" headers. From 8d1a64b7eafd8d966d827ee72ac9435d35a5bd32 Mon Sep 17 00:00:00 2001 From: Aron Podrigal Date: Mon, 13 Jul 2020 00:52:36 -0500 Subject: [PATCH 4/4] Added test test_content_length_no_content_disposition from #10771 --- Lib/cgi.py | 1 + Lib/test/test_cgi.py | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/Lib/cgi.py b/Lib/cgi.py index f1aca48f23d5e0..8aa176774220c0 100755 --- a/Lib/cgi.py +++ b/Lib/cgi.py @@ -681,6 +681,7 @@ def read_content(self): self.file = self.make_file() else: self.file = self.io_object() + self.__file = self.file todo = self.length if todo >= 0: while todo > 0: diff --git a/Lib/test/test_cgi.py b/Lib/test/test_cgi.py index bc9c495d9c6dc3..8afdd6ea05db6a 100644 --- a/Lib/test/test_cgi.py +++ b/Lib/test/test_cgi.py @@ -366,6 +366,18 @@ def test_fieldstorage_part_content_length(self): self.assertEqual(fs.list[0].name, 'submit-name') self.assertEqual(fs.list[0].value, 'Larry') + def test_content_length_no_content_disposition(self): + body = b'{"test":123}' + env = { + 'CONTENT_LENGTH': len(body), + 'REQUEST_METHOD': 'POST', + 'CONTENT_TYPE': 'application/json', + 'wsgi.input': BytesIO(body), + } + + form = cgi.FieldStorage(fp=env['wsgi.input'], environ=env) + self.assertEqual(form.file.read(), body.decode(form.encoding)) + def test_field_storage_multipart_no_content_length(self): fp = BytesIO(b"""--MyBoundary Content-Disposition: form-data; name="my-arg"; filename="foo"