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.

Sign up for GitHub

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

Jump to bottom

HTTP/2 HEADERS stream dependency stream state #3572

Closed
Scottmitch opened this issue Apr 3, 2015 · 1 comment
Closed

HTTP/2 HEADERS stream dependency stream state #3572

Scottmitch opened this issue Apr 3, 2015 · 1 comment
Assignees
@Scottmitch
Labels
defect
Milestone
4.1.0.Beta5

Comments

@Scottmitch
Copy link
Member

The specification does not provide explicit direction as how to what state the Stream Dependency stream is expected to be in, and how implementations should react if it is not in the OPEN (or other "created" states). The decoder may have to be updated by:

  1. Move the setPriority above notification of the listener.
  2. Potentially check the stream state of the Stream Dependency stream before accepting the frame.

httpwg/http2-spec#739
@Scottmitch Scottmitch self-assigned this Apr 3, 2015
@Scottmitch Scottmitch added this to the 4.1.0.Beta5 milestone Apr 3, 2015
@Scottmitch
Copy link
Member Author

The behavior should be the same as the PRIORITY frame. We should create the stream. So we need to move the set priority above the notification.

Scottmitch added a commit to Scottmitch/netty that referenced this issue Apr 3, 2015
@Scottmitch
HTTP/2 HEADERS stream dependency fix
7e3ec8c 
Motivation:
The DefaultHttp2ConnectionDecoder has the setPriority call after the Http2FrameListener is notified of the change. The setPriority call has additional verification logic and may even create the dependency stream and so it must be before the Http2FrameListener is notified.

Modifications:
The DefaultHttp2ConnectionDecoder should treat the setPriority call in the same for the HEADERS and PRIORITY frame (call it before notifying the listener).

Result:
Http2FrameListener should see correct state when a HEADERS frame has a stream dependency that has not yet been created yet.  Fixes netty#3572.
@Scottmitch Scottmitch mentioned this issue Apr 3, 2015
Closed
Scottmitch added a commit that referenced this issue Apr 3, 2015
@Scottmitch
HTTP/2 HEADERS stream dependency fix
e5d01c4 
Motivation:
The DefaultHttp2ConnectionDecoder has the setPriority call after the Http2FrameListener is notified of the change. The setPriority call has additional verification logic and may even create the dependency stream and so it must be before the Http2FrameListener is notified.

Modifications:
The DefaultHttp2ConnectionDecoder should treat the setPriority call in the same for the HEADERS and PRIORITY frame (call it before notifying the listener).

Result:
Http2FrameListener should see correct state when a HEADERS frame has a stream dependency that has not yet been created yet.  Fixes #3572.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Scottmitch Scottmitch

Labels
defect
Projects
None yet
Milestone
4.1.0.Beta5
Development

Successfully merging a pull request may close this issue.

HTTP/2 HEADERS stream dependency fix Scottmitch/netty
1 participant
@Scottmitch