New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 785411 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

HTTP/2 pushed stream is not reset if resource is found in cache.

Project Member Reported by b...@chromium.org, Nov 15 2017

Issue description

When pushed resource is found in cache, SpdySession::CancelPush() is supposed to send a RST_STREAM to the server.  However, the condition |active_streams_.find(stream_id) == active_streams_.end()| never holds true (I assume it was meant to be != instead of ==), therefore ResetStream is never called [1].  A fix is on the way [2].

[1] https://cs.chromium.org/chromium/src/net/spdy/chromium/spdy_session.cc?q=SpdySession::CancelPush&l=869
[2] https://crrev.com/c/772395
 
*facepalm* Thanks for catching this Bence! I wonder why the unittests didn't catch this...

This is a bug for Spdy, not shared by QUIC as we use the stream id to close promise stream.
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/62a209c44d63af015bfd7b2bccb3ec37b08686e3

commit 62a209c44d63af015bfd7b2bccb3ec37b08686e3
Author: Bence Béky <bnc@chromium.org>
Date: Wed Nov 15 23:26:31 2017

Reset pushed stream when SpdySession::CancelPush() is called.

A condition was inverted by accident in SpdySession::CancelPush(), this
CL fixes that.

Bug:  785411 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ib2d8be55f94c79b9f6e91dea7e0097beeb87ec1c
Reviewed-on: https://chromium-review.googlesource.com/772395
Reviewed-by: Zhongyi Shi <zhongyi@chromium.org>
Commit-Queue: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516886}
[modify] https://crrev.com/62a209c44d63af015bfd7b2bccb3ec37b08686e3/net/spdy/chromium/spdy_session.cc
[modify] https://crrev.com/62a209c44d63af015bfd7b2bccb3ec37b08686e3/net/spdy/chromium/spdy_session_unittest.cc

Comment 3 by b...@chromium.org, Nov 16 2017

Status: Fixed (was: Started)

Sign in to add a comment