Wrong status code used when rejecting push streams |
||||
Issue descriptionChrome rejects pushed streams if the item is already in the push cache. However, it rejects with the PROTOCOL_ERROR code, whereas it should use CANCEL or REFUSED_STREAM. I used https://github.com/jakearchibald/http2-push-test to test, where the root URL pushes /data. If you refresh the page without requesting /data you'll see the PROTOCOL_ERROR.
,
May 30 2017
jakearchibald@: Thanks for providing the feedback. Could you help provide a net-internals for us to investigate? And which version of chrome are you testing against?
,
May 31 2017
jakearchibald@: when you say "you'll see the PROTOCOL_ERROR", presumably you are saying the PROTOCOL_ERROR in net logs, right? It's not user visisble? This is cancellation from push cache rather than the HTTP cache. The error code is generated in SpdySession::TryCreatePushStream, where the resource has been pushed in an earlier promise, but not claimed. So next time, the resource is pushed again, the push cache will notice that. We currently generated a PROTOCL_ERROR, which is a minor bug as we are using the wrong error code. According to the sepc https://tools.ietf.org/html/rfc7540#section-8.2.2: iff the client determines, for any reason, that it does not wish to receive the pushed response from the server or if the server takes... the client can send a RST_STREAM frame, using either the CANCEL or REFUSED_STREAM code and referencing the pushed stream's identifier. https://cs.chromium.org/chromium/src/net/spdy/chromium/spdy_session.cc?type=cs&q=Received+duplicate+pushed&l=1664 This bug applied to QUIC as well.
,
Jun 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4f304415e461ebcb905767d603cc580e691b81a9 commit 4f304415e461ebcb905767d603cc580e691b81a9 Author: zhongyi <zhongyi@chromium.org> Date: Thu Jun 01 02:00:03 2017 Use correct error code: REFUSED_STREAM to reject server push from the server in SPDY. BUG= 726725 Review-Url: https://codereview.chromium.org/2914943002 Cr-Commit-Position: refs/heads/master@{#476147} [modify] https://crrev.com/4f304415e461ebcb905767d603cc580e691b81a9/net/spdy/chromium/spdy_network_transaction_unittest.cc [modify] https://crrev.com/4f304415e461ebcb905767d603cc580e691b81a9/net/spdy/chromium/spdy_session.cc
,
Jun 2 2017
Since there might be ongoing changes to supersede the RFC 7540 guidance, this bug currently only applies to HTTP/2 and is fixed in commit 4f304415e461ebcb905767d603cc580e691b81a9. https://quicwg.github.io/base-drafts/draft-ietf-quic-http.html#rfc.section.6.1 jakearchibald@: the fix has been landed in Chrome Canary: 61.0.3118.0. Could you take a look and verify the fix? Thanks! |
||||
►
Sign in to add a comment |
||||
Comment 1 by jakearchibald@chromium.org
, May 26 2017