From e2a6ba61517229bec818eb2c4020bff7a951fd85 Mon Sep 17 00:00:00 2001 From: Jeff Posnick Date: Wed, 14 Jun 2017 16:42:54 -0400 Subject: [PATCH 1/2] Strips the hash from a URL in a couple of places --- .../src/lib/cache-expiration.js | 14 ++++++----- .../test/sw/cache-expiration.js | 24 +++++++++++++++++++ packages/workbox-sw/src/lib/workbox-sw.js | 3 +++ .../test/browser/ignore-url-params.js | 12 ++++++++++ 4 files changed, 47 insertions(+), 6 deletions(-) diff --git a/packages/workbox-cache-expiration/src/lib/cache-expiration.js b/packages/workbox-cache-expiration/src/lib/cache-expiration.js index a9294b3a7..0a5901e22 100644 --- a/packages/workbox-cache-expiration/src/lib/cache-expiration.js +++ b/packages/workbox-cache-expiration/src/lib/cache-expiration.js @@ -187,16 +187,18 @@ class CacheExpiration { * * @param {Object} input * @param {string} input.cacheName Name of the cache the Responses belong to. - * @param {string} input.url The URL for the entry to update. - * @param {Number} [input.now] A timestamp. - * - * Defaults to the current time. - * + * @param {string} input.url The URL for the entry to update. The hash portion + * of the URL will be ignored. + * @param {Number} [input.now] A timestamp. Defaults to the current time. */ async updateTimestamp({cacheName, url, now} = {}) { isType({url}, 'string'); isType({cacheName}, 'string'); + // Remove the hash, if present. + const urlObject = new URL(url, location); + urlObject.hash = ''; + if (typeof now === 'undefined') { now = Date.now(); } @@ -205,7 +207,7 @@ class CacheExpiration { const tx = db.transaction(cacheName, 'readwrite'); tx.objectStore(cacheName).put({ [timestampPropertyName]: now, - [urlPropertyName]: url, + [urlPropertyName]: urlObject.href, }); await tx.complete; diff --git a/packages/workbox-cache-expiration/test/sw/cache-expiration.js b/packages/workbox-cache-expiration/test/sw/cache-expiration.js index 3de713684..a356ffcc5 100644 --- a/packages/workbox-cache-expiration/test/sw/cache-expiration.js +++ b/packages/workbox-cache-expiration/test/sw/cache-expiration.js @@ -198,4 +198,28 @@ describe('Test of the CacheExpiration class', function() { .then((idbEntryUrls) => expect(idbEntryUrls).to.eql(urls.slice(extraEntryCount))); }); }); + + it(`should ignore the URL's hash when updateTimestamp() is called`, async function() { + const cacheExpiration = new CacheExpiration({maxEntries: MAX_ENTRIES}); + const cacheName = getUniqueCacheName(); + const url = getUniqueUrl(); + const firstNow = NOW; + + await cacheExpiration.updateTimestamp({cacheName, url, now: firstNow}); + const db = await cacheExpiration.getDB({cacheName}); + const firstTx = db.transaction(cacheName, 'readonly'); + const firstStore = firstTx.objectStore(cacheName); + const firstEntry = await firstStore.get(url); + expect(firstEntry[timestampPropertyName]).to.equal(firstNow); + + const urlWithHash = `${url}#hashvalue`; + const secondNow = firstNow + 1; + + await cacheExpiration.updateTimestamp({cacheName, url: urlWithHash, now: secondNow}); + const secondTx = db.transaction(cacheName, 'readonly'); + const secondStore = secondTx.objectStore(cacheName); + // Get the entry for the original url. + const secondEntry = await secondStore.get(url); + expect(secondEntry[timestampPropertyName]).to.equal(secondNow); + }); }); diff --git a/packages/workbox-sw/src/lib/workbox-sw.js b/packages/workbox-sw/src/lib/workbox-sw.js index 61081ad12..642ddf904 100644 --- a/packages/workbox-sw/src/lib/workbox-sw.js +++ b/packages/workbox-sw/src/lib/workbox-sw.js @@ -305,6 +305,9 @@ class WorkboxSW { const route = new Route({ match: ({url}) => { + // See https://github.com/GoogleChrome/workbox/issues/488 + url.hash = ''; + const cachedUrls = this._revisionedCacheManager.getCachedUrls(); if (cachedUrls.indexOf(url.href) !== -1) { return true; diff --git a/packages/workbox-sw/test/browser/ignore-url-params.js b/packages/workbox-sw/test/browser/ignore-url-params.js index 7304c02e9..562e64619 100644 --- a/packages/workbox-sw/test/browser/ignore-url-params.js +++ b/packages/workbox-sw/test/browser/ignore-url-params.js @@ -110,4 +110,16 @@ describe('Test ignore url params matching', function() { }); }); }); + + it(`should ignore the URL's hash when routing to a precached URL`, async function() { + const responseBody = 'hello'; + const urlObject = new URL(`/__echo/date/${responseBody}`, location); + urlObject.hash = 'should-be-ignored'; + + const iframe = await goog.swUtils.activateSW('../static/sw/ignore-url-params.js'); + const fetchResponse = await iframe.contentWindow.fetch(urlObject.href); + const fetchResponseBody = await fetchResponse.text(); + + expect(fetchResponseBody.startsWith(responseBody)).to.be.true; + }); }); From 053b50e3df2168137733c06e9ee835613fd06274 Mon Sep 17 00:00:00 2001 From: Jeff Posnick Date: Tue, 27 Jun 2017 11:33:51 -0400 Subject: [PATCH 2/2] Added an additional comment. --- packages/workbox-sw/src/lib/workbox-sw.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/workbox-sw/src/lib/workbox-sw.js b/packages/workbox-sw/src/lib/workbox-sw.js index 642ddf904..b3773f7c3 100644 --- a/packages/workbox-sw/src/lib/workbox-sw.js +++ b/packages/workbox-sw/src/lib/workbox-sw.js @@ -305,7 +305,10 @@ class WorkboxSW { const route = new Route({ match: ({url}) => { - // See https://github.com/GoogleChrome/workbox/issues/488 + // See https://github.com/GoogleChrome/workbox/issues/488. + // The incoming URL might include a hash/URL fragment, and the URLs in + // the cachedUrls array will never include a hash. We need to normalize + // the incoming URL to ensure that the string comparison works. url.hash = ''; const cachedUrls = this._revisionedCacheManager.getCachedUrls();