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

Issue 771742 link

Starred by 10 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

"Bypass for network" prevents CORS requests from sending Origin header

Reported by kenn...@twitter.com, Oct 4 2017

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3230.0 Safari/537.36

Steps to reproduce the problem:
1. Load https://mobile.twitter.com, & log in.
2. Open Devtools, Application, Service Workers. Check "Bypass for network"
3. Refresh the page

What is the expected behavior?
Page loads with all content populated by API requests to api.twitter.com.

What went wrong?
The origin header is missing from CORS requests, so they throw errors in the console for requests to api.twitter.com. Page will not show any tweets.

Did this work before? N/A 

Chrome version: 63.0.3230.0  Channel: dev
OS Version: OS X 10.12.6
Flash Version: 

Probable cause of our issues in #260239
 
Screen Shot 2017-10-04 at 11.51.05.png
136 KB View Download
Cc: paulir...@chromium.org slightlyoff@chromium.org chenwilliam@chromium.org
Components: -Platform>DevTools Platform>DevTools>Network
Labels: Needs-Triage-M63
Unable to reproduce the issue on reported version 63.0.3230.0 and latest Canary 63.0.3236.0 using Mac 10.12.6 as per the steps below
1. Login https://mobile.twitter
2. Open Devtools>> Application>> click onService Workers and Check "Bypass for network"
3. Refresh the page

@ kenneth : Please find the screenshot and let us know if we have missed any steps from TE- End in the process of reproducing the issue
771742 2017-10-09 at 2.29.27 PM.png
396 KB View Download
771742 2017-10-09 at 2.35.26 PM.png
447 KB View Download
Adding to comment#3: As per the screenshot "771742 2017-10-09 at 2.29.27 PM.png "
We are able to see the tweets, so could you please confirm if you are seeing the same error in the console from your end. If not, please share the screenshot of the issue which would help in triaging further from chrome TE end
Labels: Triaged-ET Needs-Feedback

Comment 6 by alph@chromium.org, Oct 9 2017

Owner: dgozman@chromium.org
Status: Assigned (was: Unconfirmed)
I can repro it on ToT. The page says "Sorry, Twitter is taking too long to load".
There's an error in the console:
Failed to load https://api.twitter.com/1.1/prompts/suggest.json?formats=home_timeline%2Cmobile_timeline&lang=en: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'https://mobile.twitter.com' is therefore not allowed access.

Dmitry, can you please take a look.
Cc: tyoshino@chromium.org dgozman@chromium.org
Labels: -Needs-Feedback
Owner: horo@chromium.org
@horo: mind taking a look? Cc'ing tyoshino@ for CORS as well.

Comment 8 by horo@chromium.org, Oct 12 2017

Components: Blink>ServiceWorker
Components: Blink>SecurityFeature>CORS

Comment 10 by horo@chromium.org, Oct 13 2017

Hummm...

I think CORS requests can't work at all when "Bypass for network" is enabled.

Currently the CORS logic is implemented in DocumentThreadableLoader in the renderer process.
If the page is not controlled by a service worker, DocumentThreadableLoader sets the Origin header for the CORS request and sends the request to the browser process.
And the browser process sends the request to the network server.

But if the page is controlled by a service worker, DocumentThreadableLoader just sends the original request to the browser process.
And the browser process sends the request to the service worker.
If the service worker doesn't call FetchEvent.respondWith(), the browser process returns a "FallbackRequired" response to the DocumentThreadableLoader.
And DocumentThreadableLoader will start the CORS logic such as setting the Origin header.

When "Bypass for network" is enabled, InspectorNetworkAgent::WillSendRequest() sets the request service worker mode to None.
So the browser process doesn't pass the request to the service worker.

But InspectorNetworkAgent::WillSendRequest() is called just before the renderer process sends the request to the browser process.
So the request which is sent to the service worker by the DocumentThreadableLoader will directly go to the network server.

tyoshino@
What is the current status of the project of moving the CORS logic?

Comment 11 by horo@chromium.org, Oct 13 2017

Status: Started (was: Assigned)

Comment 12 by horo@chromium.org, Oct 13 2017

I think the simple solution is to check the "Bypass for network" flag in DocumentThreadableLoader.
Created a CL: https://chromium-review.googlesource.com/c/chromium/src/+/717960
Cc: toyoshim@chromium.org
horo@:

Out-of-Blink CORS is still ongoing and won't be shipped so soon. This signal from the Inspector needs to be propagated to the Out-of-Blink CORS impl as well as the other signals. We discussed it a bit when hintzed@ was working on this but haven't figured out any design yet. Takashi will take it over.

Is your question about possible future architecture where Out-of-renderer CORS can be directly used by the Service Workers?
Thanks for investigating and working on this!
To address the earlier comment by Divya in comment#3 - your screenshots show twitter.com rather than mobile.twitter.com. You may need to check the address bar after logging in to make sure you're still on mobile.twitter.com.

Comment 15 by horo@chromium.org, Oct 17 2017

tyoshino@
Thank you for the explanation.
I just thought that if Out-of-Blink CORS is coming soon, I should have more sophisticated way to handle "Bypass for network". But I don't have any good design idea yet.
horo@: Understood. Yes, we definitely work together to figure out the right design for this for OOB/OOR CORS era, later.
Project Member

Comment 17 by bugdroid1@chromium.org, Oct 17 2017

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

commit 4d76cc4bacd3b4090ccd17623483e9965cb78d43
Author: Tsuyoshi Horo <horo@chromium.org>
Date: Tue Oct 17 23:05:03 2017

Check "Bypass for network" flag in DocumentThreadableLoader

There is a bug that CORS requests can't work at all when "Bypass for network"
in DevTools is enabled.

Currently the CORS logic is implemented in DocumentThreadableLoader in the
renderer process. If the page is not controlled by a service worker,
DocumentThreadableLoader sets the Origin header for the CORS request and sends
the request to the browser process. And the browser process sends the request to
the network server.

But if the page is controlled by a service worker, DocumentThreadableLoader just
sends the original request to the browser process. And the browser process sends 
the request to the service worker. If the service worker doesn't call
FetchEvent.respondWith(), the browser process returns a "FallbackRequired"
response to the DocumentThreadableLoader. And DocumentThreadableLoader will
start the CORS logic such as setting the Origin header.

When "Bypass for network" is enabled, InspectorNetworkAgent::WillSendRequest()
sets the request service worker mode to None. So the browser process doesn't
pass the request to the service worker.

But InspectorNetworkAgent::WillSendRequest() is called just before the renderer
process sends the request to the browser process. So the request which should be
sent to the service worker will directly go to the network server.

This CL adds a check of the bypass flag in DocumentThreadableLoader, so that the
CORS logic will be correctly executed.

Bug:  771742 
Change-Id: I923fc9f9e2e361aea3d25ace653c138707d9a682
Reviewed-on: https://chromium-review.googlesource.com/717960
Reviewed-by: Will Chen <chenwilliam@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Takeshi Yoshino <tyoshino@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509578}
[add] https://crrev.com/4d76cc4bacd3b4090ccd17623483e9965cb78d43/third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/resources/service-workers-bypass-for-network-cors-iframe.html
[add] https://crrev.com/4d76cc4bacd3b4090ccd17623483e9965cb78d43/third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/resources/service-workers-bypass-for-network-cors-target.php
[add] https://crrev.com/4d76cc4bacd3b4090ccd17623483e9965cb78d43/third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/resources/service-workers-bypass-for-network-cors-worker.js
[add] https://crrev.com/4d76cc4bacd3b4090ccd17623483e9965cb78d43/third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/service-workers-bypass-for-network-cors-expected.txt
[add] https://crrev.com/4d76cc4bacd3b4090ccd17623483e9965cb78d43/third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/service-workers-bypass-for-network-cors.html
[modify] https://crrev.com/4d76cc4bacd3b4090ccd17623483e9965cb78d43/third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/service-workers-bypass-for-network-redirect.html
[modify] https://crrev.com/4d76cc4bacd3b4090ccd17623483e9965cb78d43/third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/service-workers-force-update-on-page-load.html
[modify] https://crrev.com/4d76cc4bacd3b4090ccd17623483e9965cb78d43/third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/service-workers-navigation-preload.html
[modify] https://crrev.com/4d76cc4bacd3b4090ccd17623483e9965cb78d43/third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/service-workers-redundant.html
[modify] https://crrev.com/4d76cc4bacd3b4090ccd17623483e9965cb78d43/third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.cpp
[modify] https://crrev.com/4d76cc4bacd3b4090ccd17623483e9965cb78d43/third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.h
[modify] https://crrev.com/4d76cc4bacd3b4090ccd17623483e9965cb78d43/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp
[modify] https://crrev.com/4d76cc4bacd3b4090ccd17623483e9965cb78d43/third_party/WebKit/Source/core/probe/CoreProbes.json5
[modify] https://crrev.com/4d76cc4bacd3b4090ccd17623483e9965cb78d43/third_party/WebKit/Source/core/probe/CoreProbes.pidl
[modify] https://crrev.com/4d76cc4bacd3b4090ccd17623483e9965cb78d43/third_party/WebKit/Source/devtools/front_end/application_test_runner/ServiceWorkersTestRunner.js
[modify] https://crrev.com/4d76cc4bacd3b4090ccd17623483e9965cb78d43/third_party/WebKit/Source/devtools/front_end/test_runner/TestRunner.js

Comment 18 by horo@chromium.org, Oct 19 2017

Status: Fixed (was: Started)
The patch #17 landed in 64.0.3243.0.

$ git find-releases 4d76cc4bacd3b4090ccd17623483e9965cb78d43
commit 4d76cc4bacd3b4090ccd17623483e9965cb78d43 was:
  initially in 64.0.3243.0

Comment 19 by horo@chromium.org, Oct 25 2017

Cc: horo@chromium.org pfeldman@chromium.org eostroukhov@chromium.org rbasuvula@chromium.org
 Issue 762533  has been merged into this issue.
Wooohooo. Fix in prod.
Thank you for fixing this so quickly!

Sign in to add a comment