From 15674315fbcd4e225191bd0fa4cdfc133c4f004a Mon Sep 17 00:00:00 2001 From: Aron Podrigal Date: Tue, 5 Feb 2019 16:19:03 -0600 Subject: [PATCH 1/3] 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 b96bd1f0fe39ac..d6e66c026cca5c 100755 --- a/Lib/cgi.py +++ b/Lib/cgi.py @@ -662,7 +662,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() @@ -670,31 +670,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() @@ -716,7 +715,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 @@ -824,6 +823,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 530d87bdd22b23a47d3a731d5a3165e037ab2f85 Mon Sep 17 00:00:00 2001 From: Aron Podrigal Date: Sun, 2 Jun 2019 15:48:38 -0700 Subject: [PATCH 2/3] 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 b86638e1c283af..12235617e472bb 100644 --- a/Lib/test/test_cgi.py +++ b/Lib/test/test_cgi.py @@ -202,22 +202,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'] @@ -226,14 +226,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 9d94cb975c9d388180d6860c98ba33c14088520e Mon Sep 17 00:00:00 2001 From: Aron Podrigal Date: Mon, 3 Jun 2019 10:06:15 -0500 Subject: [PATCH 3/3] 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.