Chrome's header parser skips empty values |
||||
Issue descriptionThis results in some weirdness: Content-Length: ,10, is considered valid, for instance. Also, it means the code to walk through the original "Header lines" ignores leading and trailing commas in header values, instead of actually returning the full original header line. Changes here do have the potential to break sites, unfortunately, but we should work towards a more standardized header parser.
,
Oct 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7913a74b0e22d99c3425dcea22e68783919a9667 commit 7913a74b0e22d99c3425dcea22e68783919a9667 Author: Matt Menke <mmenke@chromium.org> Date: Thu Oct 25 17:14:13 2018 Header parser: Don't allow single quotes. No HTTP header specs allow single quotes to be used instead of double quotes, so this CL better aligns Chrome with the spec. Testing current behavior, using http://test.greenbytes.de/tech/tc2231/attwithfntokensq.asis: FireFox doesn't support single quotes, but Chrome and Edge do. Bug: 896233 , 179825 Change-Id: I29f034180d3dec06d767e6dff0ce938f48e47147 Reviewed-on: https://chromium-review.googlesource.com/c/1286733 Reviewed-by: Min Qin <qinmin@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Reviewed-by: Asanka Herath <asanka@chromium.org> Commit-Queue: Matt Menke <mmenke@chromium.org> Cr-Commit-Position: refs/heads/master@{#602768} [modify] https://crrev.com/7913a74b0e22d99c3425dcea22e68783919a9667/components/download/internal/common/download_stats.cc [modify] https://crrev.com/7913a74b0e22d99c3425dcea22e68783919a9667/components/gcm_driver/crypto/encryption_header_parsers_unittest.cc [modify] https://crrev.com/7913a74b0e22d99c3425dcea22e68783919a9667/net/http/http_content_disposition.cc [modify] https://crrev.com/7913a74b0e22d99c3425dcea22e68783919a9667/net/http/http_content_disposition.h [modify] https://crrev.com/7913a74b0e22d99c3425dcea22e68783919a9667/net/http/http_content_disposition_unittest.cc [modify] https://crrev.com/7913a74b0e22d99c3425dcea22e68783919a9667/net/http/http_util.cc [modify] https://crrev.com/7913a74b0e22d99c3425dcea22e68783919a9667/net/http/http_util.h [modify] https://crrev.com/7913a74b0e22d99c3425dcea22e68783919a9667/net/http/http_util_unittest.cc [modify] https://crrev.com/7913a74b0e22d99c3425dcea22e68783919a9667/tools/metrics/histograms/enums.xml
,
Oct 25
Forgot to mention on the commit message that Safari also does not support single quotes.
,
Oct 26
Unless I'm missing something this fixed bug 896232 , not this bug?
,
Oct 26
Thanks!
,
Oct 31
For Access-Control-Allow-Origin it seems Chrome is alone with this kind of handling, FWIW: https://github.com/web-platform-tests/wpt/pull/13815.
,
Oct 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/56f794cd1a0f6f5dbd3b28c60b030ffdd0106b90 commit 56f794cd1a0f6f5dbd3b28c60b030ffdd0106b90 Author: Matt Menke <mmenke@chromium.org> Date: Wed Oct 31 23:48:27 2018 Fix handling of LWS separated by commas in HTTP/1.x headers. Previously, each the following was treated the same: Content-Length: 7 Content-Length: ,7 Content-Length: 7, Content-Length: , ,7, , And all condensed into a single header with a value of 7. There's nothing in any spec that instructs us to do this. Moreover, the spec explicitly states that the two lines: Content-Length: 7 Content-Length: Can be safely merged into: Content-Length: 7, Without changing meaning, but in the former case, Chrome treats two headers lines as if they were separate header lines, while in the latter, as mentioned, it ignores the existence of the comma. Also, the original header line could never be retrieved in this case, since the extra information was just silently purged. This CL treats both responses as having two separate Content-Length headers (And this applies to all other non-coalescing headers as well). This behavior seems to match that of all other browsers, so breakages should be minimal. Bug: 896233 Change-Id: I1acb26a3face0eef8d47c1dbe727aab5ec7dd7bd Reviewed-on: https://chromium-review.googlesource.com/c/1297251 Commit-Queue: Matt Menke <mmenke@chromium.org> Reviewed-by: danakj <danakj@chromium.org> Reviewed-by: Asanka Herath <asanka@chromium.org> Cr-Commit-Position: refs/heads/master@{#604450} [modify] https://crrev.com/56f794cd1a0f6f5dbd3b28c60b030ffdd0106b90/base/strings/string_tokenizer.h [modify] https://crrev.com/56f794cd1a0f6f5dbd3b28c60b030ffdd0106b90/base/strings/string_tokenizer_fuzzer.cc [modify] https://crrev.com/56f794cd1a0f6f5dbd3b28c60b030ffdd0106b90/base/strings/string_tokenizer_unittest.cc [modify] https://crrev.com/56f794cd1a0f6f5dbd3b28c60b030ffdd0106b90/net/http/http_response_headers.cc [modify] https://crrev.com/56f794cd1a0f6f5dbd3b28c60b030ffdd0106b90/net/http/http_response_headers_unittest.cc [modify] https://crrev.com/56f794cd1a0f6f5dbd3b28c60b030ffdd0106b90/net/http/http_util.cc [modify] https://crrev.com/56f794cd1a0f6f5dbd3b28c60b030ffdd0106b90/net/http/http_util.h [modify] https://crrev.com/56f794cd1a0f6f5dbd3b28c60b030ffdd0106b90/net/http/http_util_unittest.cc
,
Oct 31
|
||||
►
Sign in to add a comment |
||||
Comment 1 by mmenke@chromium.org
, Oct 17