New issue
Advanced search Search tips

Issue 626375 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment

HTTP/2 AltSvc is not parsed correctly.

Project Member Reported by b...@chromium.org, Jul 7 2016

Issue description

The current string identifier for HTTP/2 in "npn-h2", which is incorrect: RFC 7540 Sections 3.1. and 11.1. clearly defines "h2" for HTTP/2.  As a result of this, spec-conformant HTTP/2 AltSvc headers are ignored by Chrome.

The same utility functions (HttpServerProperties::AlternateProtocolFromString() and  AlternateProtocolToString()) are used to parse/serialize settings persisted to disk.  In order to be able to read old settings, "npn-h2" should still be parsed.
 

Comment 1 by b...@chromium.org, Jul 7 2016

Labels: -OS-Linux OS-All
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 7 2016

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

commit 08352775a31f45a7ad5f280ee1a7b98315338844
Author: bnc <bnc@chromium.org>
Date: Thu Jul 07 22:34:24 2016

Serialize NPN_HTTP_2 as "h2" instead of "npn-h2".

This will allow properly parsing spec-compliant Alt-Svc headers.

At the same time, still parse "npn-h2" so that old persisted settings can be
read from disk.

BUG= 626375 

Review-Url: https://codereview.chromium.org/2129973002
Cr-Commit-Position: refs/heads/master@{#404264}

[modify] https://crrev.com/08352775a31f45a7ad5f280ee1a7b98315338844/net/http/http_server_properties.cc
[modify] https://crrev.com/08352775a31f45a7ad5f280ee1a7b98315338844/net/http/http_server_properties_manager_unittest.cc

Project Member

Comment 3 by sheriffbot@chromium.org, Jul 8 2016

Labels: -M-53 M-54 MovedFrom-53
Moving this nonessential bug to the next milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 4 by b...@chromium.org, Jul 8 2016

Labels: -M-54 -MovedFrom-53 Merge-Request-53 M-53
Requesting merge of https://crrev.com/2129973002 to M53 (branch 2785).  This CL has made it to Canary 54.0.2791.0.  Thank you.

Comment 5 by dimu@google.com, Jul 8 2016

Labels: -Merge-Request-53 Merge-Approved-53 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M53 (branch: 2785)
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 9 2016

Labels: -merge-approved-53 merge-merged-2785
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d4eaa8b08fcfea8e277aaee04e8c3a9d84d5f889

commit d4eaa8b08fcfea8e277aaee04e8c3a9d84d5f889
Author: Bence Béky <bnc@chromium.org>
Date: Sat Jul 09 14:38:26 2016

Serialize NPN_HTTP_2 as "h2" instead of "npn-h2".

This will allow properly parsing spec-compliant Alt-Svc headers.

At the same time, still parse "npn-h2" so that old persisted settings can be
read from disk.

TBR=rch@chromium.org

BUG= 626375 

Review-Url: https://codereview.chromium.org/2129973002
Cr-Commit-Position: refs/heads/master@{#404264}
(cherry picked from commit 08352775a31f45a7ad5f280ee1a7b98315338844)

Review URL: https://codereview.chromium.org/2133393002 .

Cr-Commit-Position: refs/branch-heads/2785@{#65}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/d4eaa8b08fcfea8e277aaee04e8c3a9d84d5f889/net/http/http_server_properties.cc
[modify] https://crrev.com/d4eaa8b08fcfea8e277aaee04e8c3a9d84d5f889/net/http/http_server_properties_manager_unittest.cc

Comment 7 by b...@chromium.org, Jul 9 2016

Status: Fixed (was: Started)
Merged https://crrev.com/2129973002 manually.  I compiled net_unittests locally on my Linux box and made sure they passed before landing.

Sign in to add a comment