-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Feature/bugfix: make the HTTP client able to return HTTP chunks when chunked transfer encoding is used #2150
Merged
Merged
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
7a30dc5
implement http chunk parsing in http parser (C version)
jlacoline ad942af
http chunk decoding: implement chunk signals in Python parser
jlacoline 09b1da5
StreamReader: add tests for [begin|end]_chunk_receiving methods
jlacoline 511a680
update documentation to clarify the difference between iter_any() and…
jlacoline 6d0211f
add tests for http chunks parsing
jlacoline 32e4273
add changelog file for PR 2150
jlacoline fae40a7
http chunk parsing: readchunk() now returns tuples of (data, end_of_h…
jlacoline f56b069
Merge remote-tracking branch 'upstream/master' into add_http_chunk_fe…
jlacoline 4563338
http chunk parsing: adapt iterchunks() generator to new return format
jlacoline 513f0ad
streams.py: use parenthesis for line wrapping instead of backslash
jlacoline 85ea0eb
add unit tests for ChunkTupleAsyncStreamIterator
jlacoline 09848db
do not catch EofStream in ChunkTupleAsyncStreamIterator
jlacoline d69d890
change the behaviour of stream.readchunk when searching for the next …
jlacoline 5e8e25f
add tests to the stream.readchunk() method
jlacoline 939d386
http_parser.py: remove useless blank line
jlacoline bf31405
update documentation in streams.rst
jlacoline 19b9d14
update documentation in docs/client_reference.rst
jlacoline 5ed76d6
minor change to test_streams.py
jlacoline e172a23
change formatting in streams.rst
jlacoline c99e06c
fix spelling errors in documentation
jlacoline 1136c56
stream.rs: replace 'boolean' with :class:
jlacoline 39381ba
Merge branch 'master' into add_http_chunk_feature
asvetlov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,6 +40,14 @@ def __anext__(self): | |
raise StopAsyncIteration # NOQA | ||
return rv | ||
|
||
class ChunkTupleAsyncStreamIterator(AsyncStreamIterator): | ||
@asyncio.coroutine | ||
def __anext__(self): | ||
rv = yield from self.read_func() | ||
if rv == (b'', False): | ||
raise StopAsyncIteration # NOQA | ||
return rv | ||
|
||
|
||
class AsyncStreamReaderMixin: | ||
|
||
|
@@ -58,20 +66,21 @@ def iter_chunked(self, n): | |
return AsyncStreamIterator(lambda: self.read(n)) | ||
|
||
def iter_any(self): | ||
"""Returns an asynchronous iterator that yields slices of data | ||
as they come. | ||
"""Returns an asynchronous iterator that yields all the available | ||
data as soon as it is received | ||
|
||
Python-3.5 available for Python 3.5+ only | ||
""" | ||
return AsyncStreamIterator(self.readany) | ||
|
||
def iter_chunks(self): | ||
"""Returns an asynchronous iterator that yields chunks of the | ||
size as received by the server. | ||
"""Returns an asynchronous iterator that yields chunks of data | ||
as they are received by the server. The yielded objects are tuples | ||
of (bytes, bool) as returned by the StreamReader.readchunk method. | ||
|
||
Python-3.5 available for Python 3.5+ only | ||
""" | ||
return AsyncStreamIterator(self.readchunk) | ||
return ChunkTupleAsyncStreamIterator(self.readchunk) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a test for the call. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
|
||
|
||
class StreamReader(AsyncStreamReaderMixin): | ||
|
@@ -96,6 +105,8 @@ def __init__(self, limit=DEFAULT_LIMIT, timer=None, loop=None): | |
loop = asyncio.get_event_loop() | ||
self._loop = loop | ||
self._size = 0 | ||
self._cursor = 0 | ||
self._http_chunk_splits = None | ||
self._buffer = collections.deque() | ||
self._buffer_offset = 0 | ||
self._eof = False | ||
|
@@ -200,6 +211,7 @@ def unread_data(self, data): | |
self._buffer[0] = self._buffer[0][self._buffer_offset:] | ||
self._buffer_offset = 0 | ||
self._size += len(data) | ||
self._cursor -= len(data) | ||
self._buffer.appendleft(data) | ||
|
||
def feed_data(self, data): | ||
|
@@ -218,6 +230,18 @@ def feed_data(self, data): | |
if not waiter.done(): | ||
waiter.set_result(False) | ||
|
||
def begin_http_chunk_receiving(self): | ||
if self._http_chunk_splits is None: | ||
self._http_chunk_splits = [] | ||
|
||
def end_http_chunk_receiving(self): | ||
if self._http_chunk_splits is None: | ||
raise RuntimeError("Called end_chunk_receiving without calling " | ||
"begin_chunk_receiving first") | ||
if not self._http_chunk_splits or \ | ||
self._http_chunk_splits[-1] != self.total_bytes: | ||
self._http_chunk_splits.append(self.total_bytes) | ||
|
||
@asyncio.coroutine | ||
def _wait(self, func_name): | ||
# StreamReader uses a future to link the protocol feed_data() method | ||
|
@@ -320,16 +344,34 @@ def readany(self): | |
|
||
@asyncio.coroutine | ||
def readchunk(self): | ||
"""Returns a tuple of (data, end_of_http_chunk). When chunked transfer | ||
encoding is used, end_of_http_chunk is a boolean indicating if the end | ||
of the data corresponds to the end of a HTTP chunk , otherwise it is | ||
always False. | ||
""" | ||
if self._exception is not None: | ||
raise self._exception | ||
|
||
if not self._buffer and not self._eof: | ||
if (self._http_chunk_splits and | ||
self._cursor == self._http_chunk_splits[0]): | ||
# end of http chunk without available data | ||
self._http_chunk_splits = self._http_chunk_splits[1:] | ||
return (b"", True) | ||
yield from self._wait('readchunk') | ||
|
||
if self._buffer: | ||
return self._read_nowait_chunk(-1) | ||
if not self._buffer: | ||
# end of file | ||
return (b"", False) | ||
elif self._http_chunk_splits is not None: | ||
while self._http_chunk_splits: | ||
pos = self._http_chunk_splits[0] | ||
self._http_chunk_splits = self._http_chunk_splits[1:] | ||
if pos > self._cursor: | ||
return (self._read_nowait(pos-self._cursor), True) | ||
return (self._read_nowait(-1), False) | ||
else: | ||
return b"" | ||
return (self._read_nowait_chunk(-1), False) | ||
|
||
@asyncio.coroutine | ||
def readexactly(self, n): | ||
|
@@ -378,6 +420,7 @@ def _read_nowait_chunk(self, n): | |
data = self._buffer.popleft() | ||
|
||
self._size -= len(data) | ||
self._cursor += len(data) | ||
return data | ||
|
||
def _read_nowait(self, n): | ||
|
@@ -438,7 +481,7 @@ def readany(self): | |
|
||
@asyncio.coroutine | ||
def readchunk(self): | ||
return b'' | ||
return (b'', False) | ||
|
||
@asyncio.coroutine | ||
def readexactly(self, n): | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Make the HTTP client able to return HTTP chunks when chunked transfer encoding is used. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please explain why you replaced
>=
with>
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
required == chunk_len
, thenself._chunk_size == 0
and the "else" part does what is needed.So lines 552 and 553 looked like duplicated code, and deduplicating this allowed me to put
self.payload.end_http_chunk_receiving()
at only one place in the code.But the code will continue after the 'else' as opposed to the previous version where (False, None) was returned immediately. So the behaviour is not exactly the same even if the same result will be eventually returned.
So imo it looks better but if you want me to revert to the previous version tell me.
Also I'm going to remove the useless blank line(554)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, fine with the change