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

Issue 669363 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Launch-OWP
Launch-Accessibility: ----
Launch-Exp-Leadership: ----
Launch-Leadership: ----
Launch-Legal: ----
Launch-M-Approved: ----
Launch-M-Target: 57-Stable
Launch-Privacy: ----
Launch-Security: ----
Launch-Test: ----
Launch-UI: ----
Rollout-Type: ----

Blocked on:
issue 658249



Sign in to add a comment

Response.redirected and a new security restriction

Project Member Reported by horo@chromium.org, Nov 29 2016

Issue description

Change description:
- Add .redirected attribute to Response class of Fetch API. Web developers can check it to avoid untrustworthy responses.
- To avoid the risk of open redirectors (https://cwe.mitre.org/data/definitions/601.html) introduce a new security restriction which disallows service workers from responding with a redirected response to requests with a redirect mode different from "follow". This restriction also disallows to respond to navigation requests with redirected responses, because the redirect mode of navigation request is “manual”.

Changes to API surface:
  interface Response {
    ...
    readonly attribute boolean redirected;
    ...
  };

Links:
Public standards discussion: https://github.com/whatwg/fetch/issues/79
Spec of Response.redirected: https://fetch.spec.whatwg.org/#dom-response-redirected
Spec change: https://github.com/whatwg/fetch/commit/e54f6bd1e75f46cd4b8202f5ee3bfa68e9ded906
MDN: https://developer.mozilla.org/en-US/docs/Web/API/Response/redirected

Support in other browsers:
Edge: Edge has implemented Fetch API, but has not implemented Response.redirected attribute. Edge has not implemented Service Worker. (Checked with Edge 38.14393.0.0)
Firefox: Firefox has already implemented Response.redirected and the new security restriction in Firefox 49 (Release channel since September 20, 2016) which means that the impacted websites are already broken in Firefox. (https://www.fxsitecompat.com/en-CA/docs/2016/fetch-and-request-now-throws-if-redirect-is-detected-when-in-non-follow-redirect-mode/).
Safari: Safari has implemented Fetch API and Response.redirected attribute (https://trac.webkit.org/changeset/201324). And shipped in Technology Preview. But not implemented Service Worker.
Internet Explorer: No support for Fetch API and Service Worker.


 

Comment 1 by horo@chromium.org, Nov 29 2016

This new security restriction impacts existing code.


Scenario:
Let’s assume that your service worker behaves as follows:
 1. the install event handler stores the response of ‘/’ to the CacheStorage and,
 2. the fetch event handler responds to the navigation request of ‘/’ with the response in the CacheStorage.
  self.oninstall = evt => {
    evt.waitUntil(
        caches.open('cache_name')
          .then(cache => {
              return fetch('/').then(response => cache.put('/', response));
            }));
  };
  self.onfetch = evt => {
    if (new URL(evt.request.url).pathname != '/')
      return;
    evt.respondWith(caches.match(evt.request, {cacheName: 'cache_name'}));
  };


Problem:
If the server respond to the request of ‘/’ with a redirect response to ‘/index.html’, the response in the install event handler is, in effect, a redirected response. So with the new security restriction, respondWith() in the fetch event handler will fail. 


Solution:
To avoid this failure, you have 2 options.
1. You can either change the install event handler to store the response generated from res.body:
  self.oninstall = evt => {
    evt.waitUntil(
        caches.open('cache_name')
          .then(cache => {
              return fetch('/')
                .then(response => cache.put('/', new Response(response.body));
            }));
  };
2. Or change both handlers to store the non-redirected response by setting redirect mode to ‘manual’:
  self.oninstall = evt => {
    evt.waitUntil(
        caches.open('cache_name')
          .then(cache =>
              Promise.all(['/', '/index.html']
                .map(url =>
                     fetch(new Request(url, {redirect: 'manual'}))
                       .then(res => cache.put(url, res))))));
  };
  self.onfetch = evt => {
    var url = new URL(evt.request.url);
    if (url.pathname != '/' && url.pathname != '/index.html')
      return;
    evt.respondWith(caches.match(evt.request, {cacheName: 'cache_name'}));
  }; 

Comment 3 by horo@chromium.org, Nov 29 2016

Blockedon: 658249

Comment 4 by horo@chromium.org, Nov 29 2016

Description: Show this description

Comment 5 by horo@chromium.org, Jan 11 2017

After 57.0.2951.0, Chrome shows the error messages on DevTools.

As I announced in blink-dev, I will introduce the restriction in M59.
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/ZPaAeG3N0tw
gdgnd-error.png
137 KB View Download

Comment 6 by horo@chromium.org, Feb 14 2017

Description: Show this description
Labels: -M-57

Comment 9 by horo@chromium.org, Mar 30 2017

Status: Fixed (was: Started)
343f57b30319e935a538ef99853a8eaafe7926e5 is in 59.0.3045.0

Sign in to add a comment