More thorough hash removal

lib/removeURLHash.ts

@@ -0,0 +1,20 @@
+/*
+ Copyright 2021 Google Inc. All Rights Reserved.
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+export function removeURLHash(url: string) {
+  const urlObj = new URL(url);
+  urlObj.hash = '';
+  return urlObj.href;
+}

packages/appcache-polyfill-sw/index.ts

@@ -18,6 +18,7 @@ declare const self: ServiceWorkerGlobalScope;
 
 import {fetchWithFallback} from '../../lib/fetchWithFallback';
 import {longestMatchingPrefix} from '../../lib/longestMatchingPrefix';
+import {removeURLHash} from '../../lib/removeURLHash';
 import * as storage from '../../lib/storageWithDefault';
 
 import {
@@ -84,7 +85,7 @@ async function appCacheLogic(
     hash: string,
     clientUrl: string,
 ) {
-  const requestUrl = event.request.url;
+  const requestUrl = removeURLHash(event.request.url);
 
   // Is our request URL listed in the CACHES section?
   // Or is our request URL the client URL, since any page that
@@ -207,10 +208,8 @@ async function appCacheBehaviorForEvent(event: FetchEvent) {
     return fetch(event.request);
   }
 
-  const originalUrl = event.request.url;
-  const urlWithoutHash = originalUrl.split('#')[0];
-
-  const requestUrl = new URL(urlWithoutHash);
+  // See https://github.com/GoogleChromeLabs/sw-appcache-behavior/pull/17
+  const requestUrl = new URL(removeURLHash(event.request.url));
 
   // Appcache rules only apply to GETs & same-scheme requests.
   if (event.request.method !== 'GET' ||
@@ -219,9 +218,7 @@ async function appCacheBehaviorForEvent(event: FetchEvent) {
   }
 
   const originalClientUrl = await getClientUrlForEvent(event);
-  const clientUrlWithoutHash = originalClientUrl.split('#')[0];
-
-  const clientUrl = clientUrlWithoutHash;
+  const clientUrl = removeURLHash(originalClientUrl);
   const pageURLToManifestURL: PageURLToManifestURL =
       await storage.get('PageURLToManifestURL');
   const manifestUrl = pageURLToManifestURL[clientUrl];

packages/appcache-polyfill-window/index.ts

@@ -15,6 +15,7 @@
 
 import {getHash} from '../../lib/getHash';
 import {parseManifest} from '../../lib/parseManifest';
+import {removeURLHash} from '../../lib/removeURLHash';
 import * as storage from '../../lib/storageWithDefault';
 
 import {
@@ -203,13 +204,14 @@ async function updateManifestAssociationForCurrentPage(
     manifestUrl: string,
     hash: string,
 ) {
+  // See https://github.com/GoogleChromeLabs/sw-appcache-behavior/issues/31
   const pageURLToManifestURL: PageURLToManifestURL =
       await storage.get('PageURLToManifestURL');
-  pageURLToManifestURL[location.href] = manifestUrl;
+  pageURLToManifestURL[removeURLHash(location.href)] = manifestUrl;
 
   await Promise.all([
     storage.set('PageURLToManifestURL', pageURLToManifestURL),
-    addToCache(hash, [location.href]),
+    addToCache(hash, [removeURLHash(location.href)]),
   ]);
 }
 

tests/test-suite.js

@@ -96,6 +96,28 @@
         ]);
       });
 
+      it('should ignore the hash portion of the URL when associating pages with manifests', async function() {
+        await page.goto(`${global.baseUrl}step1.html#should_be_ignored`);
+        await page.evaluate(async () => {
+          await window.setupComplete;
+        });
+
+        const expectedCacheName = 'efe1a9f22297dec3101383dea9f670a4e38d32d86e2917702b0bee26d370d459';
+
+        const caches = await page.evaluate(() => caches.keys());
+        expect(caches).to.have.members([expectedCacheName]);
+
+        const cacheEntries = await page.evaluate(async (expectedCacheName) => {
+          const cache = await caches.open(expectedCacheName);
+          const keys = await cache.keys();
+          return keys.map((request) => request.url);
+        }, expectedCacheName);
+        expect(cacheEntries).to.have.members([
+          `${global.baseUrl}common.css`,
+          `${global.baseUrl}step1.html`,
+        ]);
+      });
+
       it('should add a new master entry for an additional navigation', async function() {
         await page.goto(`${global.baseUrl}step2.html`);
         await page.evaluate(async () => {