bpo-27777: fix cgi.FieldStorage parsing for body with Content-Length and no Content-Disposition

[ar45]

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.

https://bugs.python.org/issue27777

[merwok]

Another PR is open: #10771

Long discussion in the bug report.

[ar45]

Another PR is open: #10771

Long discussion in the bug report.

See comment on that PR for why I opted to do it differently
#10771 (comment)

[ar45]

@ethanfurman Do you mind reviewing this?

[kulikjak]

We did some internal testing with this patch and found a bug with the following trace:

Traceback (most recent call last):
  File "/var/ai/image-server/cgi-bin/telemetry.py", line 834, in <module>
    sys.exit(main())
  File "/var/ai/image-server/cgi-bin/telemetry.py", line 815, in main
    form = cgi.FieldStorage()
  File "/usr/lib/python3.7/cgi.py", line 494, in __init__
    self.read_single()
  File "/usr/lib/python3.7/cgi.py", line 687, in read_single
    self.read_content()
  File "/usr/lib/python3.7/cgi.py", line 710, in read_content
    self.__write(data)
  File "/usr/lib/python3.7/cgi.py", line 729, in __write
    data = self.__file.getvalue()
AttributeError: _io.TextIOWrapper object has no attribute getvalue 

This happens when self.length > 1000:

    if self.length > 1000:
        self.file = self.make_file()

make_file() internally calls tempfile.TemporaryFile(), which returns TextIOWrapper.

Then, in the __write(), self._write(data) is called, which fails with the aforementioned error:

def __write(self, line):
    """line is always bytes, not string"""
    if self.__file is not None:
        if self.__file.tell() + len(line) > 1000:
            self.file = self.make_file()
            data = self.__file.getvalue()    <- here

You can reproduce this simply by sending much bigger body in added test_content_length_no_content_disposition()

[AlexWaygood]

@ar45, thanks for the PR. Unfortunately, the cgi module is now deprecated following the acceptance of PEP 594, so bugfixes and improvements for this module will no longer be accepted. I am therefore closing this PR.

I hope that this does not dissuade you from contributing to CPython in the future 🙂