TABs should be ignored while parsing certain headers
Reported by
cva...@akamai.com,
Jan 2 2018
|
|||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.108 Safari/537.36 Steps to reproduce the problem: 1. With Chrome 65 (Canary at present), go to https://cvazac.netlify.com/tabs.html What is the expected behavior? The asserts should succeed, that is, the duration of the server timing entry should be 123. What went wrong? We get back 0 as a duration because TABs aren't treated as legal whitespace here: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/network/HeaderFieldTokenizer.cpp?q=CONSUMEtokenor&sq=package:chromium&l=130 Did this work before? No Does this work in other browsers? N/A Chrome version: 63.0.3239.108 Channel: stable OS Version: OS X 10.12.6 Flash Version: This also affects parsing of the Content-Type header.
,
Jan 2 2018
,
Jan 3 2018
,
Jan 3 2018
,
Feb 21 2018
Tested the issue on chrome version 65.0.3325.73 on Ubuntu 14.04 with steps mentioned below: 1) Launched chrome and navigated to URL: https://cvazac.netlify.com/tabs.html 2) Opened Devtools> Console and Devtools> Network and checked for the time field 3) In Devtools> Network tab, under time section observed 27ms @Reporter: Please find the attached screenshot for your reference and let us know your inputs on it which helps us in further triaging it in better way. Thanks!
,
Feb 21 2018
Switch to the console and you will see the failing assert.
If you evaluate this in the console, the value will be 0. It should be 123.
```
performance.getEntriesByType('resource')[0].serverTiming[0].duration
```
If you switch over to the network tab, you'll see that the response header is this:
```
server-timing: metric ; dur = 123 ;
```
,
Feb 21 2018
Thank you for providing more feedback. Adding the requester to the cc list. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Feb 26 2018
This looks extremely straight forward. Deep, can you take a look?
,
Apr 24 2018
http://jsonbin.org was a useful test example, but their header format is deprecated (as the console message mentions). I have an internal test that adds the Server-Timing header (regrettably I can't expose it), but I get the output in dev tools (in screenshot illustrated as "pure_delay" = 506.5ms), but nothing in the resourceTiming entry (blank entry in serverTiming). Since the previous release (macOS chrome 65), the values would appear in devTools, but no duration was included in the resourceTiming entry. These current examples are from "66.0.3359.117 (Official Build)"
,
Jul 6
,
Jul 16
,
Jul 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5ec3a7338a79dc825bb2bde3efeeb056ff117f06 commit 5ec3a7338a79dc825bb2bde3efeeb056ff117f06 Author: Tom McKee <tommckee@chromium.org> Date: Mon Jul 23 23:38:45 2018 Adding tests against and fixes for tabs in Server-Timing headers According to the spec (https://w3c.github.io/server-timing/#the-server-timing-header-field), optional whitespace is allowed amongst parameter names and values. We should be discarding this whitespace during parsing but, when there are tabs, we were treating it like an error and discarding the whole value. Changed the code that tokenizes HTTP headers to skip tabs and spaces when consuming optional whitespace. BUG= 798446 Change-Id: If776761e5ea199e662ec7b2b5aa245e4581131fd Reviewed-on: https://chromium-review.googlesource.com/1147258 Reviewed-by: Emil A Eklund <eae@chromium.org> Commit-Queue: Tom McKee <tommckee@chromium.org> Cr-Commit-Position: refs/heads/master@{#577325} [add] https://crrev.com/5ec3a7338a79dc825bb2bde3efeeb056ff117f06/third_party/WebKit/LayoutTests/external/wpt/server-timing/resources/parsing/84.js [add] https://crrev.com/5ec3a7338a79dc825bb2bde3efeeb056ff117f06/third_party/WebKit/LayoutTests/external/wpt/server-timing/resources/parsing/84.js.sub.headers [modify] https://crrev.com/5ec3a7338a79dc825bb2bde3efeeb056ff117f06/third_party/WebKit/LayoutTests/external/wpt/server-timing/server_timing_header-parsing-expected.txt [modify] https://crrev.com/5ec3a7338a79dc825bb2bde3efeeb056ff117f06/third_party/WebKit/LayoutTests/external/wpt/server-timing/server_timing_header-parsing.html [modify] https://crrev.com/5ec3a7338a79dc825bb2bde3efeeb056ff117f06/third_party/WebKit/LayoutTests/external/wpt/server-timing/server_timing_header-parsing.https-expected.txt [modify] https://crrev.com/5ec3a7338a79dc825bb2bde3efeeb056ff117f06/third_party/WebKit/LayoutTests/external/wpt/server-timing/server_timing_header-parsing.https.html [modify] https://crrev.com/5ec3a7338a79dc825bb2bde3efeeb056ff117f06/third_party/blink/renderer/platform/network/header_field_tokenizer.cc [modify] https://crrev.com/5ec3a7338a79dc825bb2bde3efeeb056ff117f06/third_party/blink/renderer/platform/network/header_field_tokenizer.h [modify] https://crrev.com/5ec3a7338a79dc825bb2bde3efeeb056ff117f06/third_party/blink/renderer/platform/network/parsed_content_header_field_parameters_test.cc
,
Jul 24
|
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by krajshree@chromium.org
, Jan 2 2018