Skip to content
This repository has been archived by the owner on Jan 23, 2021. It is now read-only.

"Clean up" redirected responses #241

Merged
merged 7 commits into from
Feb 7, 2017
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions lib/functions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 Blob.
var bodyPromise = 'body' in originalResponse ?
Promise.resolve(originalResponse.body) :
originalResponse.blob();

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would adding /* eslint-env browser */ fix the need for this eslint-disable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added worker, since that's more accurate than browser. But yeah, it takes care of it.

headers: originalResponse.headers,
status: originalResponse.status,
statusText: originalResponse.statusText
});
});
}
};
2 changes: 1 addition & 1 deletion lib/sw-precache.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
17 changes: 13 additions & 4 deletions service-worker.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,19 @@ 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);
}

return cleanResponse(response).then(function(responseToCache) {
return cache.put(cacheKey, responseToCache);
});
});
}
})
);
Expand Down
40 changes: 40 additions & 0 deletions test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you use new global.Response() here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, switching.

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;
}
});
});