Save-Data header should never be added to CORS Access-Control-Request-Headers |
|||||
Issue description
If there's any non-simple header, Chrome will send a preflight request. In the ACRH, it adds all headers from the original request, including simple headers.
That's why:
fetch(url, { headers: {'content-language' : 'ok'}}) results in no preflight request, and
fetch(url, { headers: {'x-lol': 'hi', 'content-language' : 'ok'}}); results in a preflight request, including content-language in ACRH.
Adding Save-Data to simple headers prevented Save-Data from triggering a preflight request. But if another header triggered a preflight request, Save-Data is in the ACRH.
SW is involved because of the timing of when Save-Data is added to the headers. When SW is not used, Save-Data is appended to headers after the preflight request is created. But when SW is used, the preflight request is performed after Save-Data was appended to headers. We've not confirmed this in detail, but the sequence is roughly:
[without SW] DocumentThreadableLoader (creates preflight) -> ResourceFetcher::initializeResourceRequest (adds "Save-Data")
[with SW] DocumentThreadableLoader (creates preflight) -> ResourceFetcher::initializeResourceRequest (adds "Save-Data") -> browser -> ServiceWorkerURLRequestJob (SW interception) -> back to renderer for preflight -> DocumentThreadableLoader (creates preflight, with Save-Data now)
A short-term fix would be to filter Save-Data out of ACRH always. Longer-term, we'd filter out all simple headers.
,
Apr 5 2016
rajendrant: I've forwarded you the thread by email.
,
Apr 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dcc9b2874edf8fd00070eb4f14176288535101a2 commit dcc9b2874edf8fd00070eb4f14176288535101a2 Author: rajendrant <rajendrant@chromium.org> Date: Thu Apr 07 02:59:51 2016 Exclude Save-Data from CORS Access-Control-Request-Headers When data saver is enabled, and a cross-origin fetch by a webpage is intercepted by a serviceworker and the serviceworker does a fetch, the preflight request will have save-data added to Access-Control-Request-Headers. As a short-term fix save-data will be excluded from ACRH header. Later all simple headers will be excluded from ACRH header. https://github.com/whatwg/fetch/issues/249 https://crbug.com/601092 BUG= 599704 Review URL: https://codereview.chromium.org/1859313002 Cr-Commit-Position: refs/heads/master@{#385637} [modify] https://crrev.com/dcc9b2874edf8fd00070eb4f14176288535101a2/content/browser/service_worker/service_worker_browsertest.cc [add] https://crrev.com/dcc9b2874edf8fd00070eb4f14176288535101a2/content/test/data/service_worker/fetch_cross_origin.html [add] https://crrev.com/dcc9b2874edf8fd00070eb4f14176288535101a2/content/test/data/service_worker/fetch_event_respond_with_fetch.js [modify] https://crrev.com/dcc9b2874edf8fd00070eb4f14176288535101a2/net/test/embedded_test_server/http_request.cc [modify] https://crrev.com/dcc9b2874edf8fd00070eb4f14176288535101a2/net/test/embedded_test_server/http_request.h [modify] https://crrev.com/dcc9b2874edf8fd00070eb4f14176288535101a2/third_party/WebKit/Source/core/fetch/CrossOriginAccessControl.cpp
,
Apr 7 2016
This change is needed in M50, to exempt save-data from CORS serviceworker preflight requests (with any custom header), when Data Saver is enabled. Requesting for approval. Thanks Raj
,
Apr 7 2016
Merge approved for M50 (branch 2661). Pls go ahead merge once it's verified safe in a canary/dev build.
,
Apr 7 2016
Please merge your change to M50 branch 2661 by 5:00 PM PST on April 8th,Friday to make into the desktop Stable final build cut. Thank you.
,
Apr 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/58f95bf087d5077231a092c353af34a9b09d7a3c commit 58f95bf087d5077231a092c353af34a9b09d7a3c Author: Tarun Bansal <tbansal@google.com> Date: Fri Apr 08 21:05:08 2016 Exclude Save-Data from CORS Access-Control-Request-Headers When data saver is enabled, and a cross-origin fetch by a webpage is intercepted by a serviceworker and the serviceworker does a fetch, the preflight request will have save-data added to Access-Control-Request-Headers. As a short-term fix save-data will be excluded from ACRH header. Later all simple headers will be excluded from ACRH header. https://github.com/whatwg/fetch/issues/249 https://crbug.com/601092 BUG= 599704 Review URL: https://codereview.chromium.org/1859313002 Cr-Commit-Position: refs/heads/master@{#385637} (cherry picked from commit dcc9b2874edf8fd00070eb4f14176288535101a2) Review URL: https://codereview.chromium.org/1875553004 . Cr-Commit-Position: refs/branch-heads/2661@{#535} Cr-Branched-From: ef6f6ae5e4c96622286b563658d5cd62a6cf1197-refs/heads/master@{#378081} [modify] https://crrev.com/58f95bf087d5077231a092c353af34a9b09d7a3c/content/browser/service_worker/service_worker_browsertest.cc [add] https://crrev.com/58f95bf087d5077231a092c353af34a9b09d7a3c/content/test/data/service_worker/fetch_cross_origin.html [add] https://crrev.com/58f95bf087d5077231a092c353af34a9b09d7a3c/content/test/data/service_worker/fetch_event_respond_with_fetch.js [modify] https://crrev.com/58f95bf087d5077231a092c353af34a9b09d7a3c/net/test/embedded_test_server/http_request.cc [modify] https://crrev.com/58f95bf087d5077231a092c353af34a9b09d7a3c/net/test/embedded_test_server/http_request.h [modify] https://crrev.com/58f95bf087d5077231a092c353af34a9b09d7a3c/third_party/WebKit/Source/core/fetch/CrossOriginAccessControl.cpp
,
Apr 12 2016
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by rajendrant@chromium.org
, Apr 4 2016