multipart Gracefully handle CONTENT

fixes #34

Jun 04 '21 08:06 agroszer

The test failures here will be fixed by #33, I think.

Jun 04 '21 17:06 cjwatson

looks like nginx is adding that, because https://www.python.org/dev/peps/pep-0333/#environ-variables says The contents of any Content-Length fields in the HTTP request. May be empty or absent.

zope.publisher used until

https://github.com/zopefoundation/zope.publisher/commit/382157293c72349f3c8742291f6aceba084bfabd#diff-524caa733a305f3082fecb38afb53289e232cf956d36fafe845832db6ac43b4c

stdlib cgi FieldStorage, which was also ignoring this:

        clen = -1
        if 'content-length' in self.headers:
            try:
                clen = int(self.headers['content-length'])
            except ValueError:
                pass

we just recently switched from python 3.7 to 3.9 and bumped zope.publisher to the latest

also fixing the zope part:

https://github.com/zopefoundation/zope.publisher/pull/64

Jun 07 '21 15:06 agroszer

@agroszer Could you rebase your branch on master so that we get passing tests before I merge this? Thanks!

Feb 05 '22 13:02 cjwatson

@cjwatson done

Feb 07 '22 08:02 agroszer

Hm. Not wanting to trigger 500 is understandable, but I struggle to decide which behavior is correct in case of a bad Content-Type value. HTTP spec does not allow it, so a 400 error would be correct. nginx behaves that way, too. The WSGI spec is very unhelpful (as usual) by allowing clearly invalid values for that header. The wording sounds like empty and absent values should be handled equally. So, the patch would be correct by defaulting to -1 as if the header was missing.

Can this be dangerous? The presence of the header indicates that we have a HTTP/1.1 connection that is able to send more than one request over a single connection. So, in theory, a carefully crafted HTTP 1.1 request could be both, one request or multiple requests, depending on the way a bad content-length is interpreted. Not sure which interpretation is best here, but following HTTP spec is probably less risky than following the weaker WSGI spec. A WSGI servers that turns invalid HTTP requests into valid WSGI requests would be incorrect, IMHO.

So, I lean towards throwing MultipartError in strict mode and assuming -1 otherwise. This is what strict-mode is made for, in the end.

Feb 07 '22 10:02 defnull

Gracefully handle CONTENT_LENGTH == ""