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

Issue 834791 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Network Error Logging: Rename field to include-subdomains per spec

Project Member Reported by dcreager@google.com, Apr 19 2018

Issue description

The NEL spec changed the spelling of the `includeSubdomains` header field to `include-subdomains`, to better match all of the other header fields.  (The old special was a carryover from the CSP spec, I think; but we've decided to go with internal consistency instead of historical consistency for Reporting and NEL.)

Anyhoo, the Chrome NEL implementation still looks for `includeSubdomains`; we should update the code to match the spec.
 

Comment 1 by dcreager@google.com, Apr 19 2018

Cc: juliatut...@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 24 2018

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

commit c57c12cc95656a712fc41bf5f391c4cec1bc18b0
Author: Douglas Creager <dcreager@google.com>
Date: Tue Apr 24 15:34:28 2018

Network Error Logging: Use new spelling of include-subdomains

The NEL spec recently changed the spelling of the `includeSubdomains`
header field to `include-subdomains`, to better match all of the other
header fields.  This patch updates Chrome's NEL reporter implementation
to look for the new spelling in response headers.  No one had started
including NEL headers yet (using the previous spelling), so there's no
need for any backwards-compatibility logic to look for both spellings.

Bug:  834791 
Change-Id: I13449aa324eb80f0978f2649b44721022617383f
Reviewed-on: https://chromium-review.googlesource.com/1019842
Reviewed-by: Julia Tuttle <juliatuttle@chromium.org>
Commit-Queue: Douglas Creager <dcreager@google.com>
Cr-Commit-Position: refs/heads/master@{#553135}
[modify] https://crrev.com/c57c12cc95656a712fc41bf5f391c4cec1bc18b0/net/network_error_logging/network_error_logging_service.cc
[modify] https://crrev.com/c57c12cc95656a712fc41bf5f391c4cec1bc18b0/net/network_error_logging/network_error_logging_service_unittest.cc

Comment 3 by dcreager@google.com, Apr 25 2018

Labels: Merge-Request-67
We'd like to merge this patch into M67 so that we're using the correct spelling of this W3C spec field from the beginning.

Comment 4 by gov...@chromium.org, Apr 25 2018

Pls apply appropriate OSs label.

Comment 5 by dcreager@google.com, Apr 25 2018

Labels: OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
Project Member

Comment 6 by sheriffbot@chromium.org, Apr 26 2018

Labels: -Merge-Request-67 Merge-Approved-67 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

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

Comment 7 by gov...@chromium.org, Apr 26 2018

Pls merge your change to M67 branch 3396 ASAP so we can pick it up for next M67 Beta release. Thank you.

Comment 8 by dcreager@google.com, Apr 26 2018

crrev.com/c/1029510 is ready to commit the cherry-pick.  I don't have committers access, govind@, could you approve/commit?  Thanks!
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 26 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3e35667dd6e8d7bc71da07b3c3477983b7a4095b

commit 3e35667dd6e8d7bc71da07b3c3477983b7a4095b
Author: Douglas Creager <dcreager@google.com>
Date: Thu Apr 26 14:09:23 2018

Network Error Logging: Use new spelling of include-subdomains

The NEL spec recently changed the spelling of the `includeSubdomains`
header field to `include-subdomains`, to better match all of the other
header fields.  This patch updates Chrome's NEL reporter implementation
to look for the new spelling in response headers.  No one had started
including NEL headers yet (using the previous spelling), so there's no
need for any backwards-compatibility logic to look for both spellings.

Bug:  834791 
Change-Id: I13449aa324eb80f0978f2649b44721022617383f
Reviewed-on: https://chromium-review.googlesource.com/1019842
Reviewed-by: Julia Tuttle <juliatuttle@chromium.org>
Commit-Queue: Douglas Creager <dcreager@google.com>
Cr-Original-Commit-Position: refs/heads/master@{#553135}(cherry picked from commit c57c12cc95656a712fc41bf5f391c4cec1bc18b0)
Reviewed-on: https://chromium-review.googlesource.com/1029510
Reviewed-by: Misha Efimov <mef@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#322}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/3e35667dd6e8d7bc71da07b3c3477983b7a4095b/net/network_error_logging/network_error_logging_service.cc
[modify] https://crrev.com/3e35667dd6e8d7bc71da07b3c3477983b7a4095b/net/network_error_logging/network_error_logging_service_unittest.cc

Owner: dcreager@chromium.org
Status: Fixed (was: Assigned)

Sign in to add a comment