Reland of Set the request mode and the credentials mode of FetchEvent in the service worker correct…

Reland of Set the request mode and the credentials mode of FetchEvent in the service worker correctly.

Currently the request mode and the credentials mode of FetchEvent.request are
not correctly set.

1. The credentials mode of no-cors resource request must be 'include', but
   currently 'same-origin'. (https://crbug.com/576534)
  ex: <img src="img.png"> <script src="test.js">

2. When fetch() is called in the page, the FetchEvent.request.mode is always
   'cors' and the FetchEvent.request.credentials is always 'same-origin' in the
   service worker. (https://crbug.com/543895)

3. When an audio element fetches a request, the FetchEvent.request.mode is
   always 'cors' and the FetchEvent.request.credentials is always 'same-origin'
   in the service worker.
Expected:
 - <audio>
   mode: no-cors, credentials: include
 - <audio crossOrigin='anonymous'>
   mode: cors, credentials: same-origin
 - <audio crossOrigin='use-credentials'>
   mode: cors, credentials: include

This CL includes many changes in LayoutTests/http/tests/fetch/. It is because
the credentials mode for script tag is changed from 'same-origin' to 'include'.
And fetch's SW-thorough tests are using script tags.

The original change (https://codereview.chromium.org/1665533003/) didn't taint
the response correctly. It was possible to read the body of opaque responses.
(https://crbug.com/589740)
So this change correctly taints the response when the response was returned
from the Service Worker.

BUG=576534, 543895
Committed: https://crrev.com/aa0375d2076441c85069620d7b8c5a29989cde9f
Cr-Commit-Position: refs/heads/master@{#379958}

[horo, 2016-03-04 10:05:10]

Patchset #2 (id:20001) has been deleted

[horo, 2016-03-07 01:58:33]

Patchset #2 (id:40001) has been deleted

[horo, 2016-03-07 02:01:40]

Description was changed from

==========
Reland of Set the request mode and the credentials mode of FetchEvent in the
service worker correctly.

Currently the request mode and the credentials mode of FetchEvent.request are
not correctly set.

1. The credentials mode of no-cors resource request must be 'include', but
   currently 'same-origin'. (https://crbug.com/576534)
  ex: <img src="img.png"> <script src="test.js">

2. When fetch() is called in the page, the FetchEvent.request.mode is always
   'cors' and the FetchEvent.request.credentials is always 'same-origin' in the
   service worker. (https://crbug.com/543895)

3. When an audio element fetches a request, the FetchEvent.request.mode is
   always 'cors' and the FetchEvent.request.credentials is always 'same-origin'
   in the service worker.
Expected:
 - <audio>
   mode: no-cors, credentials: include
 - <audio crossOrigin='anonymous'>
   mode: cors, credentials: same-origin
 - <audio crossOrigin='use-credentials'>
   mode: cors, credentials: include

This CL includes many changes in LayoutTests/http/tests/fetch/. It is because
the credentials mode for script tag is changed from 'same-origin' to 'include'.
And fetch's SW-thorough tests are using script tags.

BUG=576534, 543895
==========

to

==========
Reland of Set the request mode and the credentials mode of FetchEvent in the
service worker correctly.

Currently the request mode and the credentials mode of FetchEvent.request are
not correctly set.

1. The credentials mode of no-cors resource request must be 'include', but
   currently 'same-origin'. (https://crbug.com/576534)
  ex: <img src="img.png"> <script src="test.js">

2. When fetch() is called in the page, the FetchEvent.request.mode is always
   'cors' and the FetchEvent.request.credentials is always 'same-origin' in the
   service worker. (https://crbug.com/543895)

3. When an audio element fetches a request, the FetchEvent.request.mode is
   always 'cors' and the FetchEvent.request.credentials is always 'same-origin'
   in the service worker.
Expected:
 - <audio>
   mode: no-cors, credentials: include
 - <audio crossOrigin='anonymous'>
   mode: cors, credentials: same-origin
 - <audio crossOrigin='use-credentials'>
   mode: cors, credentials: include

This CL includes many changes in LayoutTests/http/tests/fetch/. It is because
the credentials mode for script tag is changed from 'same-origin' to 'include'.
And fetch's SW-thorough tests are using script tags.

The original change (https://codereview.chromium.org/1665533003/) didn't taint
the response correctly.
It was possible to read the body of opaque responses. (https://crbug.com/589740)
So this change correctly taints the response when the response was returned from
the Service Worker.


BUG=576534, 543895
==========

[horo, 2016-03-07 02:02:11]

Description was changed from

==========
Reland of Set the request mode and the credentials mode of FetchEvent in the
service worker correctly.

Currently the request mode and the credentials mode of FetchEvent.request are
not correctly set.

1. The credentials mode of no-cors resource request must be 'include', but
   currently 'same-origin'. (https://crbug.com/576534)
  ex: <img src="img.png"> <script src="test.js">

2. When fetch() is called in the page, the FetchEvent.request.mode is always
   'cors' and the FetchEvent.request.credentials is always 'same-origin' in the
   service worker. (https://crbug.com/543895)

3. When an audio element fetches a request, the FetchEvent.request.mode is
   always 'cors' and the FetchEvent.request.credentials is always 'same-origin'
   in the service worker.
Expected:
 - <audio>
   mode: no-cors, credentials: include
 - <audio crossOrigin='anonymous'>
   mode: cors, credentials: same-origin
 - <audio crossOrigin='use-credentials'>
   mode: cors, credentials: include

This CL includes many changes in LayoutTests/http/tests/fetch/. It is because
the credentials mode for script tag is changed from 'same-origin' to 'include'.
And fetch's SW-thorough tests are using script tags.

The original change (https://codereview.chromium.org/1665533003/) didn't taint
the response correctly.
It was possible to read the body of opaque responses. (https://crbug.com/589740)
So this change correctly taints the response when the response was returned from
the Service Worker.


BUG=576534, 543895
==========

to

==========
Reland of Set the request mode and the credentials mode of FetchEvent in the
service worker correctly.

Currently the request mode and the credentials mode of FetchEvent.request are
not correctly set.

1. The credentials mode of no-cors resource request must be 'include', but
   currently 'same-origin'. (https://crbug.com/576534)
  ex: <img src="img.png"> <script src="test.js">

2. When fetch() is called in the page, the FetchEvent.request.mode is always
   'cors' and the FetchEvent.request.credentials is always 'same-origin' in the
   service worker. (https://crbug.com/543895)

3. When an audio element fetches a request, the FetchEvent.request.mode is
   always 'cors' and the FetchEvent.request.credentials is always 'same-origin'
   in the service worker.
Expected:
 - <audio>
   mode: no-cors, credentials: include
 - <audio crossOrigin='anonymous'>
   mode: cors, credentials: same-origin
 - <audio crossOrigin='use-credentials'>
   mode: cors, credentials: include

This CL includes many changes in LayoutTests/http/tests/fetch/. It is because
the credentials mode for script tag is changed from 'same-origin' to 'include'.
And fetch's SW-thorough tests are using script tags.

The original change (https://codereview.chromium.org/1665533003/) didn't taint
the response correctly. It was possible to read the body of opaque responses. 
(https://crbug.com/589740)
So this change correctly taints the response when the response was returned
from the Service Worker.


BUG=576534, 543895
==========

[horo, 2016-03-07 02:04:40]

horo@chromium.org changed reviewers:
+ tyoshino@chromium.org

[horo, 2016-03-07 02:04:41]

tyoshino@
Could you please review this?

[tyoshino (SeeGerritForStatus), 2016-03-07 14:48:22]

lgtm

Sorry for not catching the issue in the last review.

Nice coverage!

https://codereview.chromium.org/1762893002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/fetch-response-taint.html
(right):

https://codereview.chromium.org/1762893002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/fetch-response-taint.html:13:
'/serviceworker/resources/fetch-access-control.php?';
you could use base_path() if you think the relative path between
fetch-access-control.php and this would be stable.

https://codereview.chromium.org/1762893002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/fetch-response-taint.html:59:
function rewrite_url(origin, url, mode, credentials) {
naming this "build_rewrite_url" would be slightly more readable, I think.

https://codereview.chromium.org/1762893002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/fetch/FetchManager.cpp (right):

https://codereview.chromium.org/1762893002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/fetch/FetchManager.cpp:270: switch
(response.serviceWorkerResponseType()) {
it looks good to just honor the tainting of the response created in the
ServiceWorker for now.

is this likely to change in the future e.g. with foreign fetch?

i.e. is there any possibility we need to choose the stronger tainting from one
set for the fetch() in the controlled page and one set for the response from the
ServiceWorker?

https://codereview.chromium.org/1762893002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/fetch/FetchManager.cpp:282: // ServiceWorker
can't respond to the request from fetch() with an opaque redirect response.
let's wrap comments to fit 80 col

https://codereview.chromium.org/1762893002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/fetch/FetchManager.cpp:284: // When
ServiceWorker respond to the request from fetch() with an error response,
FetchManager::Loader::didFail() must be called instead.
let's wrap comments to fit 80 col

[horo, 2016-03-08 03:11:02]

https://codereview.chromium.org/1762893002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/fetch-response-taint.html
(right):

https://codereview.chromium.org/1762893002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/fetch-response-taint.html:13:
'/serviceworker/resources/fetch-access-control.php?';
On 2016/03/07 14:48:22, tyoshino wrote:
> you could use base_path() if you think the relative path between
> fetch-access-control.php and this would be stable.

Done.

https://codereview.chromium.org/1762893002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/fetch-response-taint.html:59:
function rewrite_url(origin, url, mode, credentials) {
On 2016/03/07 14:48:22, tyoshino wrote:
> naming this "build_rewrite_url" would be slightly more readable, I think.

Done.

https://codereview.chromium.org/1762893002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/fetch/FetchManager.cpp (right):

https://codereview.chromium.org/1762893002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/fetch/FetchManager.cpp:270: switch
(response.serviceWorkerResponseType()) {
On 2016/03/07 14:48:22, tyoshino wrote:
> it looks good to just honor the tainting of the response created in the
> ServiceWorker for now.
> 
> is this likely to change in the future e.g. with foreign fetch?
> 
> i.e. is there any possibility we need to choose the stronger tainting from one
> set for the fetch() in the controlled page and one set for the response from
the
> ServiceWorker?

+CC:mek@

According to
https://github.com/slightlyoff/ServiceWorker/blob/master/foreign_fetch_explai...

> by default all responses passed to respondWith in a foreign fetch handler will
be treated as opaque responses

So I think the response will be opaque-tainted here.

https://codereview.chromium.org/1762893002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/fetch/FetchManager.cpp:282: // ServiceWorker
can't respond to the request from fetch() with an opaque redirect response.
On 2016/03/07 14:48:22, tyoshino wrote:
> let's wrap comments to fit 80 col

Done.

https://codereview.chromium.org/1762893002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/fetch/FetchManager.cpp:284: // When
ServiceWorker respond to the request from fetch() with an error response,
FetchManager::Loader::didFail() must be called instead.
On 2016/03/07 14:48:22, tyoshino wrote:
> let's wrap comments to fit 80 col

Done.

[horo, 2016-03-08 03:11:31]

horo@chromium.org changed reviewers:
+ mkwst@chromium.org

[horo, 2016-03-08 03:11:32]

mkwst@
Could you please review this?

[tyoshino (SeeGerritForStatus), 2016-03-08 05:30:05]

https://codereview.chromium.org/1762893002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/fetch/FetchManager.cpp (right):

https://codereview.chromium.org/1762893002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/fetch/FetchManager.cpp:270: switch
(response.serviceWorkerResponseType()) {
On 2016/03/08 at 03:11:02, horo wrote:
> On 2016/03/07 14:48:22, tyoshino wrote:
> > it looks good to just honor the tainting of the response created in the
> > ServiceWorker for now.
> > 
> > is this likely to change in the future e.g. with foreign fetch?
> > 
> > i.e. is there any possibility we need to choose the stronger tainting from
one
> > set for the fetch() in the controlled page and one set for the response from
the
> > ServiceWorker?
> 
> +CC:mek@
> 
> According to
https://github.com/slightlyoff/ServiceWorker/blob/master/foreign_fetch_explai...
> 
> > by default all responses passed to respondWith in a foreign fetch handler
will be treated as opaque responses
> 
> So I think the response will be opaque-tainted here.

Great! Thanks for the explanation.

[Mike West, 2016-03-08 20:20:05]

Thank you for the clear explanations and tests. LGTM.

[horo, 2016-03-08 21:52:41]

The CQ bit was checked by horo@chromium.org

[horo, 2016-03-08 21:52:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
tyoshino@chromium.org
Link to the patchset: https://codereview.chromium.org/1762893002/#ps80001
(title: "incorporated tyoshino's comment")

[commit-bot: I haz the power, 2016-03-08 21:53:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1762893002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1762893002/80001

[commit-bot: I haz the power, 2016-03-08 23:08:15]

Description was changed from

==========
Reland of Set the request mode and the credentials mode of FetchEvent in the
service worker correctly.

Currently the request mode and the credentials mode of FetchEvent.request are
not correctly set.

1. The credentials mode of no-cors resource request must be 'include', but
   currently 'same-origin'. (https://crbug.com/576534)
  ex: <img src="img.png"> <script src="test.js">

2. When fetch() is called in the page, the FetchEvent.request.mode is always
   'cors' and the FetchEvent.request.credentials is always 'same-origin' in the
   service worker. (https://crbug.com/543895)

3. When an audio element fetches a request, the FetchEvent.request.mode is
   always 'cors' and the FetchEvent.request.credentials is always 'same-origin'
   in the service worker.
Expected:
 - <audio>
   mode: no-cors, credentials: include
 - <audio crossOrigin='anonymous'>
   mode: cors, credentials: same-origin
 - <audio crossOrigin='use-credentials'>
   mode: cors, credentials: include

This CL includes many changes in LayoutTests/http/tests/fetch/. It is because
the credentials mode for script tag is changed from 'same-origin' to 'include'.
And fetch's SW-thorough tests are using script tags.

The original change (https://codereview.chromium.org/1665533003/) didn't taint
the response correctly. It was possible to read the body of opaque responses. 
(https://crbug.com/589740)
So this change correctly taints the response when the response was returned
from the Service Worker.


BUG=576534, 543895
==========

to

==========
Reland of Set the request mode and the credentials mode of FetchEvent in the
service worker correctly.

Currently the request mode and the credentials mode of FetchEvent.request are
not correctly set.

1. The credentials mode of no-cors resource request must be 'include', but
   currently 'same-origin'. (https://crbug.com/576534)
  ex: <img src="img.png"> <script src="test.js">

2. When fetch() is called in the page, the FetchEvent.request.mode is always
   'cors' and the FetchEvent.request.credentials is always 'same-origin' in the
   service worker. (https://crbug.com/543895)

3. When an audio element fetches a request, the FetchEvent.request.mode is
   always 'cors' and the FetchEvent.request.credentials is always 'same-origin'
   in the service worker.
Expected:
 - <audio>
   mode: no-cors, credentials: include
 - <audio crossOrigin='anonymous'>
   mode: cors, credentials: same-origin
 - <audio crossOrigin='use-credentials'>
   mode: cors, credentials: include

This CL includes many changes in LayoutTests/http/tests/fetch/. It is because
the credentials mode for script tag is changed from 'same-origin' to 'include'.
And fetch's SW-thorough tests are using script tags.

The original change (https://codereview.chromium.org/1665533003/) didn't taint
the response correctly. It was possible to read the body of opaque responses. 
(https://crbug.com/589740)
So this change correctly taints the response when the response was returned
from the Service Worker.


BUG=576534, 543895
==========

[commit-bot: I haz the power, 2016-03-08 23:08:16]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2016-03-08 23:09:23]

Description was changed from

==========
Reland of Set the request mode and the credentials mode of FetchEvent in the
service worker correctly.

Currently the request mode and the credentials mode of FetchEvent.request are
not correctly set.

1. The credentials mode of no-cors resource request must be 'include', but
   currently 'same-origin'. (https://crbug.com/576534)
  ex: <img src="img.png"> <script src="test.js">

2. When fetch() is called in the page, the FetchEvent.request.mode is always
   'cors' and the FetchEvent.request.credentials is always 'same-origin' in the
   service worker. (https://crbug.com/543895)

3. When an audio element fetches a request, the FetchEvent.request.mode is
   always 'cors' and the FetchEvent.request.credentials is always 'same-origin'
   in the service worker.
Expected:
 - <audio>
   mode: no-cors, credentials: include
 - <audio crossOrigin='anonymous'>
   mode: cors, credentials: same-origin
 - <audio crossOrigin='use-credentials'>
   mode: cors, credentials: include

This CL includes many changes in LayoutTests/http/tests/fetch/. It is because
the credentials mode for script tag is changed from 'same-origin' to 'include'.
And fetch's SW-thorough tests are using script tags.

The original change (https://codereview.chromium.org/1665533003/) didn't taint
the response correctly. It was possible to read the body of opaque responses. 
(https://crbug.com/589740)
So this change correctly taints the response when the response was returned
from the Service Worker.


BUG=576534, 543895
==========

to

==========
Reland of Set the request mode and the credentials mode of FetchEvent in the
service worker correctly.

Currently the request mode and the credentials mode of FetchEvent.request are
not correctly set.

1. The credentials mode of no-cors resource request must be 'include', but
   currently 'same-origin'. (https://crbug.com/576534)
  ex: <img src="img.png"> <script src="test.js">

2. When fetch() is called in the page, the FetchEvent.request.mode is always
   'cors' and the FetchEvent.request.credentials is always 'same-origin' in the
   service worker. (https://crbug.com/543895)

3. When an audio element fetches a request, the FetchEvent.request.mode is
   always 'cors' and the FetchEvent.request.credentials is always 'same-origin'
   in the service worker.
Expected:
 - <audio>
   mode: no-cors, credentials: include
 - <audio crossOrigin='anonymous'>
   mode: cors, credentials: same-origin
 - <audio crossOrigin='use-credentials'>
   mode: cors, credentials: include

This CL includes many changes in LayoutTests/http/tests/fetch/. It is because
the credentials mode for script tag is changed from 'same-origin' to 'include'.
And fetch's SW-thorough tests are using script tags.

The original change (https://codereview.chromium.org/1665533003/) didn't taint
the response correctly. It was possible to read the body of opaque responses.
(https://crbug.com/589740)
So this change correctly taints the response when the response was returned
from the Service Worker.

BUG=576534, 543895

Committed: https://crrev.com/aa0375d2076441c85069620d7b8c5a29989cde9f
Cr-Commit-Position: refs/heads/master@{#379958}
==========

[commit-bot: I haz the power, 2016-03-08 23:09:24]

Patchset 3 (id:??) landed as
https://crrev.com/aa0375d2076441c85069620d7b8c5a29989cde9f
Cr-Commit-Position: refs/heads/master@{#379958}