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

HTTP/2 engine must tolerate priority frames received in any state and better handle flow control problems #5126

Closed
wtlucy opened this issue Sep 18, 2018 · 1 comment · Fixed by #5127
Assignees
Labels
in:Transport release bug This bug is present in a released version of Open Liberty release-18.0.0.4 team:Sirius

Comments

@wtlucy
Copy link
Contributor

wtlucy commented Sep 18, 2018

RFC 7540 states that it's valid to receive PRIORITY frames on any stream in any state:

PRIORITY frames can be sent on closed streams to prioritize
streams that are dependent on the closed stream. Endpoints SHOULD
process PRIORITY frames, though they can be ignored if the stream
has been removed from the dependency tree (see Section 5.3.4).

and

The PRIORITY frame can be sent on a stream in any state

The HTTP/2 engine currently sends a connection error back to the client if a PRIORITY frame is received on a stream that is even-numbered and has been closed and removed from the dependency tree. We should ignore such PRIORITY frames instead of killing the connection.

This issue was reported and reproduced with Chrome 69.

Additionally, improvements need to be made to the flow control logic. We should update the code to stop waiting for a window update if an RST_STREAM or GOAWAY is recieved, and we should take out FFDC generation for these types of flow control errors.

@wtlucy wtlucy added in:Transport release bug This bug is present in a released version of Open Liberty team:Sirius labels Sep 18, 2018
@wtlucy wtlucy self-assigned this Sep 18, 2018
@wtlucy wtlucy changed the title HTTP/2 engine must tolerate priority frames received on closed streams HTTP/2 engine must tolerate priority frames received in any state Sep 19, 2018
@wtlucy wtlucy changed the title HTTP/2 engine must tolerate priority frames received in any state HTTP/2 engine must tolerate priority frames received in any state and better handle flow control problems Sep 24, 2018
@wtlucy
Copy link
Contributor Author

wtlucy commented Sep 24, 2018

I've spent some time investigating problems with http/2 push on Chrome for this issue.

For some reason, Chrome doesn't correctly/quickly update its own flow control window for push()ed resources unless those resources are "adopted" by the browser. So, if large resources are pushed, we (the server) can very quickly send over so much data that the flow control window is completely eroded, and we cannot send any more data frames until a window update is received. But because Chrome may not have "adopted" those resources, it will not have counted the received data against its flow control window, so it won't know to send a window update. At that point, the connection essentially times out, since Chrome can be waiting for more data on a non-push stream, and we're waiting for a window update to send that remaining data.

Chromium has a bug open for this, but it hasn't been updated since 2015.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in:Transport release bug This bug is present in a released version of Open Liberty release-18.0.0.4 team:Sirius
Projects
None yet
2 participants