Skip to content
New issue

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

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

Already on GitHub? # to your account

Multipart read_chunk without content-length #750

Merged
merged 1 commit into from
Jan 28, 2016

Conversation

tumb1er
Copy link
Contributor

@tumb1er tumb1er commented Jan 27, 2016

Allows to read body parts of multipart/form-data request without Content-Length header for parts.

Current BodyPartReader.read_chunk requires Content-Length header to be set in body part. In fact, simple html form with file field doesn't send file length (at least in Chrome), so read_chunk is unusable for large binary file uploads.

Proposed implementation does two things:

  • It reads body part chunk-by-chunk until boundary is found in two subsequent chunks.
  • If boundary is found, it is pushed back to aiohttp.streams.StreamReader via unread_data method.

Since next body part content may occure in last read chunk, BodyPartReader._unread deque is not enough, and data should be returned to StreamReader instance.


@asyncio.coroutine
def _read_chunk_from_stream(self, size):
""" Reads content chunk of body part with unknown length.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to start docstring from space.

@kxepal
Copy link
Member

kxepal commented Jan 27, 2016

Surprise to read about Chrome, but it's good to have Content-Length optional, though iirc that's not right.

@tumb1er
Copy link
Contributor Author

tumb1er commented Jan 28, 2016

@kxepal if no more comments, let's consider PR as ready-to-merge.

@kxepal
Copy link
Member

kxepal commented Jan 28, 2016

@tumb1er I think need to rebase and squash it first. There are strange and unrelated xor commits here.

@asvetlov Are you ok with?

@asvetlov
Copy link
Member

Yes, looks good to me.
@kxepal please go forward and merge when you will be ready.

@tumb1er tumb1er force-pushed the multipart_read_chunk branch from 7dda8eb to 8c7950f Compare January 28, 2016 13:54
pep8

fixes for PR

fixes for PR

fix typo

don't search boundary twice
@tumb1er tumb1er force-pushed the multipart_read_chunk branch from 8c7950f to f1351a3 Compare January 28, 2016 13:58
@tumb1er
Copy link
Contributor Author

tumb1er commented Jan 28, 2016

@kxepal squashed and rebased.

kxepal added a commit that referenced this pull request Jan 28, 2016
Multipart read_chunk without content-length
@kxepal kxepal merged commit fb2829b into aio-libs:master Jan 28, 2016
@kxepal
Copy link
Member

kxepal commented Jan 28, 2016

@tumb1er thanks!

@tumb1er tumb1er deleted the multipart_read_chunk branch January 28, 2016 14:33
This was referenced Jan 31, 2016
@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
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants