Re: [Python-Dev] cgi.FieldStorage with multipart/form-data tries to decode binary file as UTF-8 if "filename=" not specified

15 Feb 2017

      That's reasonable. This isn't an issue on Python 2.x because everything is
handled as bytes (str on 2.x). It looks like cgi.FieldStorage() only got
Unicode support in Python 3.x at all fairly "late" in the game, for 3.2 or
3.3. It was in that commit (https://github.com/python/cpy
thon/commit/5c23b8e6ea7cdb0002842d16dbce4b4d716dd35a by Victor Stinner)
where the check for "it's a binary file if it has the 'filename' parameter"
was added. It wasn't an issue before that because Unicode wasn't really
handled.

It seems to me this could be added to FieldStorage pretty easily in a
backwards-compatible way, by one of the following:

1) Adding a file_fields keyword argument to FieldStorage.__init__ which
would allow you to specify that "these fields are definitely files, please
treat them as binary". So in the case of my example on the SO question,
you'd do FieldStorage(..., file_fields=('payload',))

2) Add a .binary_file (or .file_binary) property to FieldStorage which
would be like .file but for sure give you a binary file.

3) Allowing you to specify encoding=None, which would mean don't decode
fields to str, return everything as bytes. This might not be so nice, as
then other non-file fields would be returned as bytes too, and the caller
would have to decode those manually.

Any thoughts on which of #1-3 might be best?

I also realized that to work around this, instead of my fairly complicated
content-disposition headers fix, I can subclass FieldStorage and just
override "filename" with a property, so that FieldStorage.__init__ thinks
that field does have a filename:

    class MyFieldStorage(cgi.FieldStorage):
        @property
        def filename(self):
            return 'file_name' if self.name == 'payload' else self._filename

        @filename.setter
        def filename(self, value):
            self._filename = value

Also, on StackOverflow someone responded that you can also work around this
by passing errors='surrogateescape' and then
file_input.read().encode('utf-8', 'surrogateescape') ... pretty hacky, but
at least you can get the bytes back.

-Ben

On Wed, Feb 15, 2017 at 12:35 PM, Brett Cannon  wrote:
...
On Wed, 15 Feb 2017 at 08:14 Ben Hoyt  wrote:
...
I posted this on StackOverflow [1], but I'm posting it here as well, as I
believe this is a bug (or at least quirk) in cgi.FieldStorage where you
can't access a file upload properly if "filename=" is not present in the
MIME part's Content-Disposition header. There are a couple of related bugs
open (and closed) on bugs.python.ord, but not quite this issue.
Is it legitimate for cgi.FieldStorage to use the presence of "filename="
to determine "this is a binary file" (in which case this is not a bug and
my client is just buggy), or is this a bug? I lean towards the latter as
the spec indicates that the filename is optional [2].
Assuming this isn't a recent change in semantics I would say this is now a
quick considering how old the module is and people probably rely on its
current semantics.
-Brett
...
Copying from my StackOverflow question, including a test/repro case:
When I use `cgi.FieldStorage` to parse a `multipart/form-data` request
(or any web framework like Pyramid which uses `cgi.FieldStorage`) I have
trouble processing file uploads from certain clients which don't provide a
`filename=file.ext` in the part's `Content-Disposition` header.
If the `filename=` option is missing, `FieldStorage()` tries to decode
the contents of the file as UTF-8 and return a string. And obviously many
files are binary and not UTF-8 and as such give bogus results.
For example:
>>> import cgi
    >>> import io
    >>> body = (b'--KQNTvuH-itP09uVKjjZiegh7\r\n' +
    ...         b'Content-Disposition: form-data; name=payload\r\n\r\n' +
    ...         b'\xff\xd8\xff\xe0\x00\x10JFIF')
    >>> env = {
    ...     'REQUEST_METHOD': 'POST',
    ...     'CONTENT_TYPE': 'multipart/form-data; boundary=KQNTvuH-
itP09uVKjjZiegh7',
    ...     'CONTENT_LENGTH': len(body),
    ... }
    >>> fs = cgi.FieldStorage(fp=io.BytesIO(body), environ=env)
    >>> (fs['payload'].filename, fs['payload'].file.read())
    (None, '����\x00\x10JFIF')
Browsers, and *most* HTTP libraries do include the `filename=` option for
file uploads, but I'm currently dealing with a client that doesn't (and
omitting the `filename` does seem to be valid according to the spec).
Currently I'm using a pretty hacky workaround by subclassing
`FieldStorage` and replacing the relevant `Content-Disposition` header with
one that does have the filename:
import cgi
    import os
class FileFieldStorage(cgi.FieldStorage):
        """To use, subclass FileFieldStorage and override _file_fields
with a tuple
        of the names of the file field(s). You can also override
_file_name with
        the filename to add.
        """
_file_fields = ()
        _file_name = 'file_name'
def __init__(self, fp=None, headers=None, outerboundary=b'',
                     environ=os.environ, keep_blank_values=0,
strict_parsing=0,
                     limit=None, encoding='utf-8', errors='replace'):
if self._file_fields and headers and headers.get('content-
disposition'):
                content_disposition = headers['content-disposition']
                key, pdict = cgi.parse_header(content_disposition)
                if (key == 'form-data' and pdict.get('name') in
self._file_fields and
                        'filename' not in pdict):
                    del headers['content-disposition']
                    quoted_file_name = self._file_name.replace('"', '\\"')
                    headers['content-disposition'] = '{};
filename="{}"'.format(
                            content_disposition, quoted_file_name)
super().__init__(fp=fp, headers=headers,
outerboundary=outerboundary,
                             environ=environ,
keep_blank_values=keep_blank_values,
                             strict_parsing=strict_parsing, limit=limit,
                             encoding=encoding, errors=errors)
Using the `body` and `env` in my first test, this works now:
>>> class TestFieldStorage(FileFieldStorage):
    ...     _file_fields = ('payload',)
    >>> fs = TestFieldStorage(fp=io.BytesIO(body), environ=env)
    >>> (fs['payload'].filename, fs['payload'].file.read())
    ('file_name', b'\xff\xd8\xff\xe0\x00\x10JFIF')
Is there some way to avoid this hack and tell `FieldStorage` not to
decode as UTF-8? It would be nice if you could provide `encoding=None` or
something, but it doesn't look like it supports that.
Thanks,
Ben.
[1] https://stackoverflow.com/questions/42213318/cgi-
fieldstorage-with-multipart-form-data-tries-to-decode-
binary-file-as-utf-8-e
[2] https://www.w3.org/Protocols/rfc2616/rfc2616-sec19.html#sec19.5.1
_______________________________________________
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: https://mail.python.org/mailman/options/python-dev/
brett%40python.org