From e80a63bb1183e719490c17dbed97fb9a8450617d Mon Sep 17 00:00:00 2001 From: Jeff Posnick Date: Tue, 24 Jan 2017 10:51:51 -0500 Subject: [PATCH 1/7] Handle redirection by creating a synthetic, unredirected response --- service-worker.tmpl | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/service-worker.tmpl b/service-worker.tmpl index bbec851..dce221f 100644 --- a/service-worker.tmpl +++ b/service-worker.tmpl @@ -77,10 +77,21 @@ self.addEventListener('install', function(event) { Array.from(urlsToCacheKeys.values()).map(function(cacheKey) { // If we don't have a key matching url in the cache already, add it. if (!cachedUrls.has(cacheKey)) { - return cache.add(new Request(cacheKey, { - credentials: 'same-origin', - redirect: 'follow' - })); + var request = new Request(cacheKey, {credentials: 'same-origin'}); + return fetch(request).then(function(response) { + // Bail out of installation unless we get back a 200 OK for + // every request. + if (!response.ok) { + throw new Error('Request for ' + cacheKey + ' returned a ' + + 'response with status ' + response.status); + } + + // See https://bugs.chromium.org/p/chromium/issues/detail?id=669363&desc=2#c1 + var responseToCache = response.redirected ? + new Response(response.body) : + response; + return cache.put(cacheKey, responseToCache); + }); } }) ); From ffcbf4030a91d2f8e9fefe9eac08ac6e1c0789f1 Mon Sep 17 00:00:00 2001 From: Jeff Posnick Date: Tue, 24 Jan 2017 13:27:16 -0500 Subject: [PATCH 2/7] Make a copy of response headers and status, too. --- service-worker.tmpl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/service-worker.tmpl b/service-worker.tmpl index dce221f..df819d9 100644 --- a/service-worker.tmpl +++ b/service-worker.tmpl @@ -88,8 +88,11 @@ self.addEventListener('install', function(event) { // See https://bugs.chromium.org/p/chromium/issues/detail?id=669363&desc=2#c1 var responseToCache = response.redirected ? - new Response(response.body) : - response; + new Response(response.body, { + headers: response.headers, + status: response.status, + statusText: response.statusText + }) : response; return cache.put(cacheKey, responseToCache); }); } From 0745f41c4d3e7fc047721d5c138d1bad1045df86 Mon Sep 17 00:00:00 2001 From: Jeff Posnick Date: Tue, 24 Jan 2017 13:27:44 -0500 Subject: [PATCH 3/7] Bump cache version to v3 to trigger refetches --- lib/sw-precache.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/sw-precache.js b/lib/sw-precache.js index 0f45a30..8faa8e0 100644 --- a/lib/sw-precache.js +++ b/lib/sw-precache.js @@ -30,7 +30,7 @@ var util = require('util'); require('es6-promise').polyfill(); // This should only change if there are breaking changes in the cache format used by the SW. -var VERSION = 'v2'; +var VERSION = 'v3'; function absolutePath(relativePath) { return path.resolve(process.cwd(), relativePath); From f8ba8c4d4cdd2443b2e78de35a36e425212f6567 Mon Sep 17 00:00:00 2001 From: Jeff Posnick Date: Tue, 24 Jan 2017 16:13:39 -0500 Subject: [PATCH 4/7] More robust response cleaning --- lib/functions.js | 25 +++++++++++++++++++++++++ package.json | 1 + service-worker.tmpl | 11 +++-------- test/test.js | 40 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 8 deletions(-) diff --git a/lib/functions.js b/lib/functions.js index 8b4d0e4..eb04821 100644 --- a/lib/functions.js +++ b/lib/functions.js @@ -77,5 +77,30 @@ module.exports = { } return url.toString(); + }, + + // When passed a redirected response, this will create a new, "clean" response + // that can be used to respond to a navigation request. + // See https://bugs.chromium.org/p/chromium/issues/detail?id=669363&desc=2#c1 + cleanResponse: function(originalResponse) { + // If this is not a redirected response, then we don't have to do anything. + if (!originalResponse.redirected) { + return Promise.resolve(originalResponse); + } + + // Firefox 50 and below doesn't support the Response.body stream, so we may + // need to read the entire body to memory as a string via text(). + var bodyPromise = originalResponse.body ? + Promise.resolve(originalResponse.body) : + originalResponse.text(); + + return bodyPromise.then(function(body) { + // new Response() is happy when passed either a stream or a string. + return new Response(body, { // eslint-disable-line no-undef + headers: originalResponse.headers, + status: originalResponse.status, + statusText: originalResponse.statusText + }); + }); } }; diff --git a/package.json b/package.json index c560423..cd77de7 100644 --- a/package.json +++ b/package.json @@ -37,6 +37,7 @@ "gulp-util": "^3.0.7", "jade": "^1.11.0", "mocha": "^3.1.2", + "node-fetch": "^1.6.3", "run-sequence": "^1.2.2" }, "dependencies": { diff --git a/service-worker.tmpl b/service-worker.tmpl index df819d9..5d9b88e 100644 --- a/service-worker.tmpl +++ b/service-worker.tmpl @@ -86,14 +86,9 @@ self.addEventListener('install', function(event) { 'response with status ' + response.status); } - // See https://bugs.chromium.org/p/chromium/issues/detail?id=669363&desc=2#c1 - var responseToCache = response.redirected ? - new Response(response.body, { - headers: response.headers, - status: response.status, - statusText: response.statusText - }) : response; - return cache.put(cacheKey, responseToCache); + return cleanResponse(response).then(function(responseToCache) { + return cache.put(cacheKey, responseToCache); + }); }); } }) diff --git a/test/test.js b/test/test.js index 5bb1770..3254710 100644 --- a/test/test.js +++ b/test/test.js @@ -468,3 +468,43 @@ describe('generateRuntimeCaching', function() { assert.equal(code, '\ntoolbox.router.get("/*", toolbox.testHandler, {"origin":"http://www.example.com"});'); }); }); + +describe('cleanResponse', function() { + var responseText = 'test response body'; + var globalResponse = global.Response; + + before(function() { + if (!globalResponse) { + global.Response = require('node-fetch').Response; + } + }); + + it('should return the same response when redirected is false', function() { + var originalResponse = new Response(responseText); // eslint-disable-line no-undef + originalResponse.redirected = false; + + return externalFunctions.cleanResponse(originalResponse).then(function(cleanedResponse) { + assert.strictEqual(originalResponse, cleanedResponse); + }); + }); + + it('should return a new response with the same body when redirected is true', function() { + var originalResponse = new Response(responseText); // eslint-disable-line no-undef + originalResponse.redirected = true; + + return externalFunctions.cleanResponse(originalResponse).then(function(cleanedResponse) { + assert.notStrictEqual(originalResponse, cleanedResponse); + + var bodyPromises = [originalResponse.text(), cleanedResponse.text()]; + return Promise.all(bodyPromises).then(function(bodies) { + assert.equal(bodies[0], bodies[1]); + }); + }); + }); + + after(function() { + if (!globalResponse) { + delete global.Response; + } + }); +}); From bdcfc90a2a9870adfd94938a99b0a865c9f0fc67 Mon Sep 17 00:00:00 2001 From: Jeff Posnick Date: Thu, 26 Jan 2017 10:34:59 -0500 Subject: [PATCH 5/7] Use 'in' to check for Response.body --- lib/functions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/functions.js b/lib/functions.js index eb04821..a0fe128 100644 --- a/lib/functions.js +++ b/lib/functions.js @@ -90,7 +90,7 @@ module.exports = { // Firefox 50 and below doesn't support the Response.body stream, so we may // need to read the entire body to memory as a string via text(). - var bodyPromise = originalResponse.body ? + var bodyPromise = 'body' in originalResponse ? Promise.resolve(originalResponse.body) : originalResponse.text(); From 5d893c82a0f96344170ccb93b0443c164fdf73de Mon Sep 17 00:00:00 2001 From: Jeff Posnick Date: Tue, 31 Jan 2017 11:17:11 -0500 Subject: [PATCH 6/7] Switch to using a Blob instead of a string if Response.body isn't available. --- lib/functions.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/functions.js b/lib/functions.js index a0fe128..c0990c4 100644 --- a/lib/functions.js +++ b/lib/functions.js @@ -89,13 +89,13 @@ module.exports = { } // Firefox 50 and below doesn't support the Response.body stream, so we may - // need to read the entire body to memory as a string via text(). + // need to read the entire body to memory as a Blob. var bodyPromise = 'body' in originalResponse ? Promise.resolve(originalResponse.body) : - originalResponse.text(); + originalResponse.blob(); return bodyPromise.then(function(body) { - // new Response() is happy when passed either a stream or a string. + // new Response() is happy when passed either a stream or a Blob. return new Response(body, { // eslint-disable-line no-undef headers: originalResponse.headers, status: originalResponse.status, From 1899583f40453ddc32af5c241ef9c7a0fd5935a5 Mon Sep 17 00:00:00 2001 From: Jeff Posnick Date: Tue, 7 Feb 2017 10:19:22 -0500 Subject: [PATCH 7/7] Review feedback. --- cli.js | 3 ++- lib/functions.js | 4 ++-- test/test.js | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/cli.js b/cli.js index a73c9a0..862d796 100755 --- a/cli.js +++ b/cli.js @@ -56,7 +56,8 @@ function setDefaults(cli, configFileFlags) { compositeFlags.navigateFallback = compositeFlags.navigateFallback || configFileFlags.navigateFallback; - compositeFlags.navigateFallbackWhitelist = compositeFlags.navigateFallbackWhitelist || + compositeFlags.navigateFallbackWhitelist = + compositeFlags.navigateFallbackWhitelist || configFileFlags.navigateFallbackWhitelist; compositeFlags.staticFileGlobs = compositeFlags.staticFileGlobs || diff --git a/lib/functions.js b/lib/functions.js index c0990c4..54bdd4d 100644 --- a/lib/functions.js +++ b/lib/functions.js @@ -14,7 +14,7 @@ * limitations under the License. */ -/* eslint-env node */ +/* eslint-env node,worker */ 'use strict'; var URL = require('dom-urls'); @@ -96,7 +96,7 @@ module.exports = { return bodyPromise.then(function(body) { // new Response() is happy when passed either a stream or a Blob. - return new Response(body, { // eslint-disable-line no-undef + return new Response(body, { headers: originalResponse.headers, status: originalResponse.status, statusText: originalResponse.statusText diff --git a/test/test.js b/test/test.js index 3254710..26b68e9 100644 --- a/test/test.js +++ b/test/test.js @@ -480,7 +480,7 @@ describe('cleanResponse', function() { }); it('should return the same response when redirected is false', function() { - var originalResponse = new Response(responseText); // eslint-disable-line no-undef + var originalResponse = new global.Response(responseText); originalResponse.redirected = false; return externalFunctions.cleanResponse(originalResponse).then(function(cleanedResponse) { @@ -489,7 +489,7 @@ describe('cleanResponse', function() { }); it('should return a new response with the same body when redirected is true', function() { - var originalResponse = new Response(responseText); // eslint-disable-line no-undef + var originalResponse = new global.Response(responseText); originalResponse.redirected = true; return externalFunctions.cleanResponse(originalResponse).then(function(cleanedResponse) {