New issue
Advanced search Search tips

Issue 766928 link

Starred by 2 users
Status: Fixed
Owner:
rch@chromium.org
Closed: Sep 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

wrong QUIC v40 STREAM frame

Reported by martense...@gmail.com, Sep 20 2017 
UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.91 Safari/537.36

Example URL:

Steps to reproduce the problem:
Use QUIC v40.

What is the expected behavior?

What went wrong?
QUIC v40 is supposed to implement the IETF format for RST_STREAM, ACK and STREAM frames.

Chrome currently writes the Data Length as first in the STREAM_FRAME in https://cs.chromium.org/chromium/src/net/quic/core/quic_framer.cc?q=quic_framer&sq=package:chromium&l=1995. According to the IETF draft (https://quicwg.github.io/base-drafts/draft-ietf-quic-transport.html#rfc.section.8.14), it should be written after the Offset.

Did this work before? No 

Chrome version: 61.0.3163.91  Channel: canary
OS Version: OS X 10.12.6
Flash Version:

Comment 1 by mmenke@chromium.org, Sep 20 2017

Components: -Internals>Network Internals>Network>QUIC

Comment 2 by rch@chromium.org, Sep 20 2017

Owner: rch@chromium.org

Comment 3 by krajshree@chromium.org, Sep 21 2017

Labels: Needs-Triage-M61 TE-NeedsTriageHelp
The issue seems to be out of TE-scope as it is related to QUIC. Hence, adding label TE-NeedsTriageHelp for further investigation from dev team.

Thanks...!!

Comment 4 by rch@chromium.org, Sep 21 2017

Labels: -Needs-Triage-M61
Status: Started (was: Unconfirmed)
I'm working on this and should have a fix relatively soon.
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 22 2017 

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

commit 2a40d33829f7faabaee72f128194cb2cb3ebf5b7
Author: Ryan Hamilton <rch@chromium.org>
Date: Fri Sep 22 17:32:24 2017

Rename (currently unused) QUIC v41 to v42 in preparation for fixing v40 in a new v41.

Merge internal change: 169577711

Bug:  766928 
Change-Id: Iecb17bdfb604b7e730faa679f29a5b259dba8530
Reviewed-on: https://chromium-review.googlesource.com/676759
Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
Reviewed-by: Randy Smith <rdsmith@chromium.org>
Reviewed-by: Jana Iyengar <jri@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Commit-Queue: Ryan Hamilton <rch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503783}
[modify] https://crrev.com/2a40d33829f7faabaee72f128194cb2cb3ebf5b7/chrome/browser/page_load_metrics/observers/protocol_page_load_metrics_observer.cc
[modify] https://crrev.com/2a40d33829f7faabaee72f128194cb2cb3ebf5b7/components/domain_reliability/util.cc
[modify] https://crrev.com/2a40d33829f7faabaee72f128194cb2cb3ebf5b7/net/http/http_response_info.cc
[modify] https://crrev.com/2a40d33829f7faabaee72f128194cb2cb3ebf5b7/net/http/http_response_info.h
[modify] https://crrev.com/2a40d33829f7faabaee72f128194cb2cb3ebf5b7/net/quic/chromium/quic_http_stream.cc
[modify] https://crrev.com/2a40d33829f7faabaee72f128194cb2cb3ebf5b7/net/quic/core/quic_flags_list.h
[modify] https://crrev.com/2a40d33829f7faabaee72f128194cb2cb3ebf5b7/net/quic/core/quic_version_manager.cc
[modify] https://crrev.com/2a40d33829f7faabaee72f128194cb2cb3ebf5b7/net/quic/core/quic_version_manager.h
[modify] https://crrev.com/2a40d33829f7faabaee72f128194cb2cb3ebf5b7/net/quic/core/quic_version_manager_test.cc
[modify] https://crrev.com/2a40d33829f7faabaee72f128194cb2cb3ebf5b7/net/quic/core/quic_versions.cc
[modify] https://crrev.com/2a40d33829f7faabaee72f128194cb2cb3ebf5b7/net/quic/core/quic_versions.h
[modify] https://crrev.com/2a40d33829f7faabaee72f128194cb2cb3ebf5b7/net/tools/quic/quic_dispatcher_test.cc
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 23 2017 

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

commit 6a151e60dad836ffa604f07449238de43a774433
Author: Ryan Hamilton <rch@chromium.org>
Date: Fri Sep 22 23:59:33 2017

Rename QUIC v40 to v41 in preparation for fixing this new v41.

Merge internal change: 169589373

Bug:  766928 
Change-Id: I86543184aad5056e95e04a6e60e7c3c020f4001d
Reviewed-on: https://chromium-review.googlesource.com/677476
Commit-Queue: Ryan Hamilton <rch@chromium.org>
Reviewed-by: Jana Iyengar <jri@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503911}
[modify] https://crrev.com/6a151e60dad836ffa604f07449238de43a774433/net/quic/chromium/quic_http_stream.cc
[modify] https://crrev.com/6a151e60dad836ffa604f07449238de43a774433/net/quic/core/quic_flags_list.h
[modify] https://crrev.com/6a151e60dad836ffa604f07449238de43a774433/net/quic/core/quic_framer.cc
[modify] https://crrev.com/6a151e60dad836ffa604f07449238de43a774433/net/quic/core/quic_framer_test.cc
[modify] https://crrev.com/6a151e60dad836ffa604f07449238de43a774433/net/quic/core/quic_version_manager.cc
[modify] https://crrev.com/6a151e60dad836ffa604f07449238de43a774433/net/quic/core/quic_version_manager.h
[modify] https://crrev.com/6a151e60dad836ffa604f07449238de43a774433/net/quic/core/quic_version_manager_test.cc
[modify] https://crrev.com/6a151e60dad836ffa604f07449238de43a774433/net/quic/core/quic_versions.cc
[modify] https://crrev.com/6a151e60dad836ffa604f07449238de43a774433/net/quic/core/quic_versions.h
[modify] https://crrev.com/6a151e60dad836ffa604f07449238de43a774433/net/tools/quic/quic_dispatcher_test.cc
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 24 2017 

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

commit 569732c0a96ec8d0c4c5df7f7012188696c7d37c
Author: Ryan Hamilton <rch@chromium.org>
Date: Sun Sep 24 21:38:00 2017

Fix v41 STREAM frame layout.

Merge internal change: 169600926

Bug:  766928 
Change-Id: Ib6bc0750559a3c05b679aa262701235a2df5a22a
Reviewed-on: https://chromium-review.googlesource.com/677736
Commit-Queue: Ryan Hamilton <rch@chromium.org>
Reviewed-by: Jana Iyengar <jri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503964}
[modify] https://crrev.com/569732c0a96ec8d0c4c5df7f7012188696c7d37c/net/quic/core/quic_framer.cc
[modify] https://crrev.com/569732c0a96ec8d0c4c5df7f7012188696c7d37c/net/quic/core/quic_framer_test.cc

Comment 8 by rch@chromium.org, Sep 25 2017

Status: Fixed (was: Started)
►

Sign in to add a comment