Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MultipartReader does not support multipart data with a preamble #880

Closed
obmarg opened this issue May 19, 2016 · 4 comments · Fixed by #881
Closed

MultipartReader does not support multipart data with a preamble #880

obmarg opened this issue May 19, 2016 · 4 comments · Fixed by #881
Labels

Comments

@obmarg
Copy link
Contributor

obmarg commented May 19, 2016

Long story short

I'm using the MultipartReader to read multi-part data from a 3rd party API. It's response has an extra CRLF pair before the first boundary marker.

It appears from reading the RFC that it's valid to include a preamble before the first boundary, and it should just be ignored

I'm passing a response from this server through aiohttp.MultipartReader.from_response and then calling .next()

Expected behaviour

aiohttp ignores the preamble and returns the first part.

Actual behaviour

I get the exception:

ValueError: Invalid boundary b'', expected b'------=_Part_125656_1263945784.1463662001461'

Steps to reproduce

I've added a failing unit test here: obmarg@8724035

Your environment

Not sure any of it's relevant, but: MacOS, python 3.5.1, aiohttp 0.19.0 (though reproduced with master).

obmarg added a commit to obmarg/aiohttp that referenced this issue May 19, 2016
This updates the MultipartReader to ignore the preamble of a multipart
message.  To quote the RFC:

> There appears to be room for additional information prior to the first
> boundary delimiter line and following the final boundary delimiter line.
> These areas should generally be left blank, and implementations must
> ignore anything that appears before the first boundary delimiter line or
> after the last one.

To do this, the MultipartReader now acts slightly differently at the
beginning of the file.  Instead of looking for a boundary and excepting
if it doesn't find one, it will skip over the initial data looking for
the first boundary and start reading from there.

If it doesn't find any boundary, it will except similar to how it did
before.

Fixes aio-libs#880
obmarg added a commit to obmarg/aiohttp that referenced this issue May 19, 2016
This updates the MultipartReader to ignore the preamble of a multipart
message.  To quote the RFC:

> There appears to be room for additional information prior to the first
> boundary delimiter line and following the final boundary delimiter line.
> These areas should generally be left blank, and implementations must
> ignore anything that appears before the first boundary delimiter line or
> after the last one.

To do this, the MultipartReader now acts slightly differently at the
beginning of the file.  Instead of looking for a boundary and excepting
if it doesn't find one, it will skip over the initial data looking for
the first boundary and start reading from there.

If it doesn't find any boundary, it will except similar to how it did
before.

Fixes aio-libs#880
obmarg added a commit to obmarg/aiohttp that referenced this issue May 19, 2016
This updates the MultipartReader to ignore the preamble of a multipart
message.  To quote the RFC:

> There appears to be room for additional information prior to the first
> boundary delimiter line and following the final boundary delimiter line.
> These areas should generally be left blank, and implementations must
> ignore anything that appears before the first boundary delimiter line or
> after the last one.

To do this, the MultipartReader now acts slightly differently at the
beginning of the file.  Instead of looking for a boundary and excepting
if it doesn't find one, it will skip over the initial data looking for
the first boundary and start reading from there.

If it doesn't find any boundary, it will except similar to how it did
before.

Fixes aio-libs#880
@fafhrd91
Copy link
Member

@kxepal ?

@kxepal
Copy link
Member

kxepal commented May 23, 2016

@fafhrd91 hm? I guess that issue is linked with the PR you pinged a little bit before now, no?

@fafhrd91
Copy link
Member

ok

@lock
Copy link

lock bot commented Oct 29, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants