Allow precaching "repair" when using subresource integrity (#2921)

* Allow precaching "repair" when using SRI

* Ensure cacheability plugin is used

* Fixed up some logic

* Linting

* Better fallback logic

* Fix up tests

Author: jeffposnick

packages/workbox-precaching/src/PrecacheController.ts

@@ -284,6 +284,15 @@ class PrecacheController {
     return this._urlsToCacheKeys.get(urlObject.href);
   }
 
+  /**
+   * @param {string} url A cache key whose SRI you want to look up.
+   * @return {string} The subresource integrity associated with the cache key,
+   * or undefined if it's not set.
+   */
+  getIntegrityForCacheKey(cacheKey: string): string | undefined {
+    return this._cacheKeysToIntegrities.get(cacheKey);
+  }
+
   /**
    * This acts as a drop-in replacement for
    * [`cache.match()`](https://developer.mozilla.org/en-US/docs/Web/API/Cache/match)

packages/workbox-precaching/src/PrecacheRoute.ts

@@ -50,7 +50,8 @@ class PrecacheRoute extends Route {
       for (const possibleURL of generateURLVariations(request.url, options)) {
         const cacheKey = urlsToCacheKeys.get(possibleURL);
         if (cacheKey) {
-          return {cacheKey};
+          const integrity = precacheController.getIntegrityForCacheKey(cacheKey);
+          return {cacheKey, integrity};
         }
       }
       if (process.env.NODE_ENV !== 'production') {

packages/workbox-precaching/src/PrecacheStrategy.ts

@@ -43,13 +43,13 @@ class PrecacheStrategy extends Strategy {
       }
 
       return response;
-    }
+    },
   };
 
   static readonly copyRedirectedCacheableResponsesPlugin: WorkboxPlugin = {
     async cacheWillUpdate({response}) {
       return response.redirected ? await copyResponse(response) : response;
-    }
+    },
   };
 
   /**
@@ -73,7 +73,8 @@ class PrecacheStrategy extends Strategy {
     options.cacheName = cacheNames.getPrecacheName(options.cacheName);
     super(options);
 
-    this._fallbackToNetwork = options.fallbackToNetwork === false ? false: true;
+    this._fallbackToNetwork =
+      options.fallbackToNetwork === false ? false : true;
 
     // Redirected responses cannot be used to satisfy a navigation request, so
     // any redirected response must be "copied" rather than cloned, so the new
@@ -91,31 +92,68 @@ class PrecacheStrategy extends Strategy {
    */
   async _handle(request: Request, handler: StrategyHandler): Promise<Response> {
     const response = await handler.cacheMatch(request);
-    if (!response) {
-      // If this is an `install` event then populate the cache. If this is a
-      // `fetch` event (or any other event) then respond with the cached
-      // response.
-      if (handler.event && handler.event.type === 'install') {
-        return await this._handleInstall(request, handler);
-      }
-      return await this._handleFetch(request, handler);
+    if (response) {
+      return response;
     }
 
-    return response;
+    // If this is an `install` event for an entry that isn't already cached,
+    // then populate the cache.
+    if (handler.event && handler.event.type === 'install') {
+      return await this._handleInstall(request, handler);
+    }
+
+    // Getting here means something went wrong. An entry that should have been
+    // precached wasn't found in the cache.
+    return await this._handleFetch(request, handler);
   }
 
-  async _handleFetch(request: Request, handler: StrategyHandler): Promise<Response> {
+  async _handleFetch(
+    request: Request,
+    handler: StrategyHandler,
+  ): Promise<Response> {
     let response;
+    const params = (handler.params || {}) as {
+      cacheKey?: string;
+      integrity?: string;
+    };
 
-    // Fall back to the network if we don't have a cached response
-    // (perhaps due to manual cache cleanup).
+    // Fall back to the network if we're configured to do so.
     if (this._fallbackToNetwork) {
       if (process.env.NODE_ENV !== 'production') {
-        logger.warn(`The precached response for ` +
+        logger.warn(
+          `The precached response for ` +
             `${getFriendlyURL(request.url)} in ${this.cacheName} was not ` +
-            `found. Falling back to the network instead.`);
+            `found. Falling back to the network.`,
+        );
+      }
+
+      const integrityInManifest = params.integrity;
+      const integrityInRequest = request.integrity;
+      const noIntegrityConflict =
+        !integrityInRequest || integrityInRequest === integrityInManifest;
+      response = await handler.fetch(
+        new Request(request, {
+          integrity: integrityInRequest || integrityInManifest,
+        }),
+      );
+
+      // It's only "safe" to repair the cache if we're using SRI to guarantee
+      // that the response matches the precache manifest's expectations,
+      // 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) {
+        this._useDefaultCacheabilityPluginIfNeeded();
+        const wasCached = await handler.cachePut(request, response.clone());
+        if (process.env.NODE_ENV !== 'production') {
+          if (wasCached) {
+            logger.log(
+              `A response for ${getFriendlyURL(request.url)} ` +
+                `was used to "repair" the precache.`,
+            );
+          }
+        }
       }
-      response = await handler.fetch(request);
     } else {
       // This shouldn't normally happen, but there are edge cases:
       // https://github.com/GoogleChrome/workbox/issues/1441
@@ -126,18 +164,19 @@ class PrecacheStrategy extends Strategy {
     }
 
     if (process.env.NODE_ENV !== 'production') {
-      // Params in handlers is type any, can't change right now.
-      // eslint-disable-next-line
-      const cacheKey = handler.params && handler.params.cacheKey ||
-          await handler.getCacheKey(request, 'read');
+      const cacheKey =
+        params.cacheKey || (await handler.getCacheKey(request, 'read'));
 
       // Workbox is going to handle the route.
       // print the routing details to the console.
-      logger.groupCollapsed(`Precaching is responding to: ` +
-          getFriendlyURL(request.url));
-      // cacheKey is type any, can't change right now.
-      // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
-      logger.log(`Serving the precached url: ${getFriendlyURL(cacheKey.url)}`);
+      logger.groupCollapsed(
+        `Precaching is responding to: ` + getFriendlyURL(request.url),
+      );
+      logger.log(
+        `Serving the precached url: ${getFriendlyURL(
+          cacheKey instanceof Request ? cacheKey.url : cacheKey,
+        )}`,
+      );
 
       logger.groupCollapsed(`View request details here.`);
       logger.log(request);
@@ -149,10 +188,14 @@ class PrecacheStrategy extends Strategy {
 
       logger.groupEnd();
     }
+
     return response;
   }
 
-  async _handleInstall(request: Request, handler: StrategyHandler): Promise<Response> {
+  async _handleInstall(
+    request: Request,
+    handler: StrategyHandler,
+  ): Promise<Response> {
     this._useDefaultCacheabilityPluginIfNeeded();
 
     const response = await handler.fetch(request);

test/workbox-precaching/sw/test-PrecacheStrategy.mjs

@@ -11,9 +11,9 @@ import {PrecacheStrategy} from 'workbox-precaching/PrecacheStrategy.mjs';
 import {eventDoneWaiting, spyOnEvent} from '../../../infra/testing/helpers/extendable-event-utils.mjs';
 
 
-function createFetchEvent(url) {
+function createFetchEvent(url, requestInit) {
   const event = new FetchEvent('fetch', {
-    request: new Request(url),
+    request: new Request(url, requestInit),
   });
   spyOnEvent(event);
   return event;
@@ -54,6 +54,71 @@ describe(`PrecacheStrategy()`, function() {
       const response2 = await ps.handle(createFetchEvent('/two'));
       expect(await response2.text()).to.equal('Fetched Response');
       expect(self.fetch.callCount).to.equal(1);
+
+      // /two should not be there, since integrity isn't used.
+      const cachedUrls = (await cache.keys()).map((request) => request.url);
+      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() {
+      sandbox.stub(self, 'fetch').callsFake((request) => {
+        const response = new Response('Fetched Response');
+        sandbox.replaceGetter(response, 'url', () => request.url);
+        return response;
+      });
+
+      const cache = await caches.open(cacheNames.getPrecacheName());
+      await cache.put(new Request('/one'), new Response('Cached Response'));
+
+      const ps = new PrecacheStrategy();
+
+      const response1 = await ps.handle(createFetchEvent('/one'));
+      expect(await response1.text()).to.equal('Cached Response');
+      expect(self.fetch.callCount).to.equal(0);
+
+      const integrity = 'some-hash';
+      const request = new Request('/two', {
+        integrity,
+      });
+      const event = createFetchEvent(request.url, request);
+      const response2 = await ps.handle({
+        event,
+        request,
+        params: {
+          integrity,
+        },
+      });
+      expect(await response2.text()).to.equal('Fetched Response');
+      expect(self.fetch.callCount).to.equal(1);
+
+      // No integrity is used, so it shouldn't populate cache.
+      const response3 = await ps.handle(createFetchEvent('/three'));
+      expect(await response3.text()).to.equal('Fetched Response');
+      expect(self.fetch.callCount).to.equal(2);
+
+      // This should not populate the cache, because the params.integrity
+      // doesn't match the request.integrity.
+      const request4 = new Request('/four', {
+        integrity,
+      });
+      const event4 = createFetchEvent(request4.url, request4);
+      const response4 = await ps.handle({
+        event: event4,
+        request: request4,
+        params: {
+          integrity: 'does-not-match',
+        },
+      });
+      expect(await response4.text()).to.equal('Fetched Response');
+      expect(self.fetch.callCount).to.equal(3);
+
+      // /two should be there, since request.integrity matches params.integrity.
+      // /three and /four shouldn't.
+      const cachedUrls = (await cache.keys()).map((request) => request.url);
+      expect(cachedUrls).to.eql([
+        `${location.origin}/one`,
+        `${location.origin}/two`,
+      ]);
     });
 
     it(`just checks cache if fallbackToNetwork is false`, async function() {