defnull / multipart

A fast multipart/form-data parser for python
https://multipart.readthedocs.io/
MIT License
135 stars 33 forks source link

multipart 1.0.0 breaks zope.publisher a little bit by being very strict about line endings #55

Closed mgedmin closed 2 months ago

mgedmin commented 2 months ago

The new parser in multipart 1.0.0 is stricter than the old one and requires \r\n line endings. If you use plain '\n', the parser doesn't recognize the boundaries and keeps scanning until it raises MultipartError("Unexpected end of multipart stream (parser closed)"), which is then silently caught and ignored (for users like zope.publisher that don't pass strict=True), making it look like the request body was empty.

This breaks the test suite of zope.publisher and probably other projects (I have one too). I don't mind fixing those, if this is an intentional behavior change in order to conform to the relevant RFCs or whatever, but I would suggest at least mentioning this backwards incompatibility in the changelog.

What do you think about this?

defnull commented 2 months ago

Thanks for the hint. Yes this should produce a more useful error message, fail quicker, and should be noted in the release notes. The new more strict behavior is intentional, though.

Input that contains LF instead of CRLF is clearly broken and should be rejected, even in non-strict mode. LF alone was never valid as far as I know, all RFCs I could find (including RFC 2046 from 1996) mandate CRLF. Maybe a decade ago there were broken e-mail clients producing such output, and early multipart parsers (for email) had to support it, but I do not know of any relevant browser or HTTP client that does this today. So, removing those unit-tests should be fine, there should be no real world impact.

I'll keep this issue open until I find the time to improve error reporting, detection and documentation on this. Should not be hard, but I'm quite packed this week.