This repository has been archived by the owner on Jan 13, 2021. It is now read-only.
Refactor Stream to ensure header blocks are decoded in the correct order #39
Conversation
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
…der: Previously, incoming frames were placed on per-Stream queues, which were read from lazily, i.e. whenever HTTP20Response.getheaders() or .read() were called. However, the HPACK algorithm depends on header blocks being encoded/decoded in the same order on both sides of the connection; otherwise, the codec states on each peer will get out of sync. This commit rearranges header decoding by moving it from Stream.getresponse() to Stream.receive_frame(), which is executed on the Connection's event loop. DATA frame receiving was refactored in a similar way for consistency.
Merged
| @@ -155,18 +181,14 @@ def listlen(list): | |||
| w = WindowUpdateFrame(self.stream_id) | |||
| w.window_increment = increment | |||
| self._data_cb(w) | |||
| else: # pragma: no cover | |||
There was a problem hiding this comment.
Don't pragma no cover this, test it. =)
|
This is a good idea, thanks! Some code review comments inline. =) |
|
Thanks for the review! Addressed comments, and build is green once more. |
|
Awesome! Aside from the comment I made in #40, I'm ready to merge this! =) |
|
Right, the comment was a good suggestion, but |
| @@ -12,6 +12,7 @@ | |||
| DataFrame, HeadersFrame, SettingsFrame, Frame, WindowUpdateFrame, | |||
| GoAwayFrame | |||
| ) | |||
| from .response import HTTP20Response | |||
Lukasa
added a commit
that referenced
this pull request
Apr 7, 2014
Refactor Stream to ensure header blocks are decoded in the correct order
|
Thanks! On Mon, Apr 7, 2014 at 11:35 AM, Cory Benfield [email protected]:
|
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Previously, incoming frames were placed on per-
Streamqueues, which were read from lazily, i.e. wheneverHTTP20Response.getheaders()or.read()were called. However, the HPACK algorithm depends on header blocks being encoded/decoded in the same order on both sides of the connection; otherwise, the codec states on each peer will get out of sync.This PR rearranges header decoding by moving it from
Stream.getresponse()toStream.receive_frame(), which is executed on theConnection's event loop.DATAframe receiving was refactored in a similar way for consistency.