Skip to content

Commit

Permalink
Do not use integrity when request is no-cors (#3099)
Browse files Browse the repository at this point in the history 
* Do not use integrity when request is no-cors

* removing build files that got modified

* modify chrome driver version

* modify chrome driver version
  • Loading branch information
@tropicadri
tropicadri authored Jul 25, 2022
1 parent 4e80b58 commit 978c4f6
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 23 deletions.
16 changes: 14 additions & 2 deletions packages/workbox-precaching/src/PrecacheStrategy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,15 @@ class PrecacheStrategy extends Strategy {
const integrityInRequest = request.integrity;
const noIntegrityConflict =
!integrityInRequest || integrityInRequest === integrityInManifest;

// Do not add integrity if the original request is no-cors
// See https://github.com/GoogleChrome/workbox/issues/3096
response = await handler.fetch(
new Request(request, {
integrity: integrityInRequest || integrityInManifest,
integrity:
request.mode !== 'no-cors'
? integrityInRequest || integrityInManifest
: undefined,
}),
);

Expand All @@ -142,7 +148,13 @@ class PrecacheStrategy extends Strategy {
// and there's either a) no integrity property in the incoming request
// or b) there is an integrity, and it matches the precache manifest.
// See https://github.com/GoogleChrome/workbox/issues/2858
if (integrityInManifest && noIntegrityConflict) {
// Also if the original request users no-cors we don't use integrity.
// See https://github.com/GoogleChrome/workbox/issues/3096
if (
integrityInManifest &&
noIntegrityConflict &&
request.mode !== 'no-cors'
) {
this._useDefaultCacheabilityPluginIfNeeded();
const wasCached = await handler.cachePut(request, response.clone());
if (process.env.NODE_ENV !== 'production') {
Expand Down
60 changes: 39 additions & 21 deletions test/workbox-precaching/sw/test-PrecacheStrategy.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,23 @@ function createFetchEvent(url, requestInit) {
return event;
}

describe(`PrecacheStrategy()`, function() {
describe(`PrecacheStrategy()`, function () {
const sandbox = sinon.createSandbox();

beforeEach(async function() {
beforeEach(async function () {
const keys = await caches.keys();
await Promise.all(keys.map((key) => caches.delete(key)));
sandbox.restore();
});

after(async function() {
after(async function () {
const keys = await caches.keys();
await Promise.all(keys.map((key) => caches.delete(key)));
sandbox.restore();
});

describe(`handle()`, function() {
it(`falls back to network by default on fetch`, async function() {
describe(`handle()`, function () {
it(`falls back to network by default on fetch`, async function () {
sandbox.stub(self, 'fetch').callsFake((request) => {
const response = new Response('Fetched Response');
sandbox.replaceGetter(response, 'url', () => request.url);
Expand All @@ -62,7 +62,7 @@ describe(`PrecacheStrategy()`, function() {
expect(cachedUrls).to.eql([`${location.origin}/one`]);
});

it(`falls back to network by default on fetch, and populates the cache if integrity is used`, async function() {
it(`falls back to network by default on fetch, and populates the cache if integrity is used`, async function () {
sandbox.stub(self, 'fetch').callsFake((request) => {
const response = new Response('Fetched Response');
sandbox.replaceGetter(response, 'url', () => request.url);
Expand Down Expand Up @@ -114,6 +114,24 @@ describe(`PrecacheStrategy()`, function() {
expect(await response4.text()).to.equal('Fetched Response');
expect(self.fetch.callCount).to.equal(3);

// This should not populate the cache, because the request is no-cors
// so we won't use integrity and won't populate the cache
const request5 = new Request('/five', {
integrity,
mode: 'no-cors',
});
const event5 = createFetchEvent(request.url, request);
const response5 = await ps.handle({
event: event5,
request: request5,
params: {
integrity,
},
});

expect(await response5.text()).to.equal('Fetched Response');
expect(self.fetch.callCount).to.equal(4);

// /two should be there, since request.integrity matches params.integrity.
// /three and /four shouldn't.
const cachedUrls = (await cache.keys()).map((request) => request.url);
Expand All @@ -123,7 +141,7 @@ describe(`PrecacheStrategy()`, function() {
]);
});

it(`just checks cache if fallbackToNetwork is false`, async function() {
it(`just checks cache if fallbackToNetwork is false`, async function () {
sandbox.stub(self, 'fetch').callsFake((request) => {
const response = new Response('Fetched Response');
sandbox.replaceGetter(response, 'url', () => request.url);
Expand All @@ -145,7 +163,7 @@ describe(`PrecacheStrategy()`, function() {
);
});

it(`copies redirected responses`, async function() {
it(`copies redirected responses`, async function () {
sandbox.stub(self, 'fetch').callsFake((request) => {
const response = new Response('Redirected Response');
sandbox.replaceGetter(response, 'redirected', () => true);
Expand All @@ -171,7 +189,7 @@ describe(`PrecacheStrategy()`, function() {
expect(await cachedResponse.text()).to.equal('Redirected Response');
});

it(`errors during install if the default plugin returns null`, async function() {
it(`errors during install if the default plugin returns null`, async function () {
// Also ensure that we don't cache the bad response;
// see https://github.com/GoogleChrome/workbox/issues/2737
const putStub = sandbox.stub().resolves();
Expand Down Expand Up @@ -204,7 +222,7 @@ describe(`PrecacheStrategy()`, function() {
expect(defaultPluginSpy.callCount).to.eql(1);
});

it(`doesn't error during install if the cacheWillUpdate plugin allows it`, async function() {
it(`doesn't error during install if the cacheWillUpdate plugin allows it`, async function () {
const errorResponse = new Response('Server Error', {
status: 400,
});
Expand Down Expand Up @@ -245,7 +263,7 @@ describe(`PrecacheStrategy()`, function() {
expect(defaultPluginSpy.callCount).to.eql(0);
});

it(`errors during install if any of the cacheWillUpdate plugins return null`, async function() {
it(`errors during install if any of the cacheWillUpdate plugins return null`, async function () {
const errorResponse = new Response('Server Error', {
status: 400,
});
Expand Down Expand Up @@ -292,8 +310,8 @@ describe(`PrecacheStrategy()`, function() {
});
});

describe('_useDefaultCacheabilityPluginIfNeeded()', function() {
it(`should include the expected plugins by default`, async function() {
describe('_useDefaultCacheabilityPluginIfNeeded()', function () {
it(`should include the expected plugins by default`, async function () {
const ps = new PrecacheStrategy();

ps._useDefaultCacheabilityPluginIfNeeded();
Expand All @@ -313,7 +331,7 @@ describe(`PrecacheStrategy()`, function() {
]);
});

it(`should include the default plugin when the strategy has only non-cacheWillUpdate plugins`, async function() {
it(`should include the default plugin when the strategy has only non-cacheWillUpdate plugins`, async function () {
const cacheKeyWillBeUsedPlugin = {
cacheKeyWillBeUsed: sandbox.stub(),
};
Expand All @@ -340,7 +358,7 @@ describe(`PrecacheStrategy()`, function() {
]);
});

it(`should not include the default plugin when the strategy has one cacheWillUpdate plugin`, async function() {
it(`should not include the default plugin when the strategy has one cacheWillUpdate plugin`, async function () {
const cacheWillUpdatePlugin = {
cacheWillUpdate: sandbox.stub(),
};
Expand All @@ -365,7 +383,7 @@ describe(`PrecacheStrategy()`, function() {
]);
});

it(`should not include the default plugin when the strategy has multiple cacheWillUpdate plugins`, async function() {
it(`should not include the default plugin when the strategy has multiple cacheWillUpdate plugins`, async function () {
const cacheWillUpdatePlugin1 = {
cacheWillUpdate: sandbox.stub(),
};
Expand Down Expand Up @@ -404,7 +422,7 @@ describe(`PrecacheStrategy()`, function() {
]);
});

it(`should remove the default plugin if a cacheWillUpdate plugin has been added after the initial call`, async function() {
it(`should remove the default plugin if a cacheWillUpdate plugin has been added after the initial call`, async function () {
const cacheWillUpdatePlugin = {
cacheWillUpdate: sandbox.stub(),
};
Expand All @@ -430,8 +448,8 @@ describe(`PrecacheStrategy()`, function() {
});
});

describe('defaultPrecacheCacheabilityPlugin', function() {
it(`should return the same response when the status is 200`, async function() {
describe('defaultPrecacheCacheabilityPlugin', function () {
it(`should return the same response when the status is 200`, async function () {
const response = new Response('', {status: 200});

const returnedResponse =
Expand All @@ -444,7 +462,7 @@ describe(`PrecacheStrategy()`, function() {
expect(returnedResponse).to.eql(response);
});

it(`should return the same response when the status is 0`, async function() {
it(`should return the same response when the status is 0`, async function () {
// You can't construct opaque responses, so stub out the getter.
const response = new Response('', {status: 599});
sandbox.stub(response, 'status').get(() => 0);
Expand All @@ -459,7 +477,7 @@ describe(`PrecacheStrategy()`, function() {
expect(returnedResponse).to.eql(response);
});

it(`should return null when the status is 404`, async function() {
it(`should return null when the status is 404`, async function () {
const response = new Response('', {status: 404});

const returnedResponse =
Expand Down

0 comments on commit 978c4f6

Please sign in to comment.