feat(service-worker): use `ignoreVary: true` when retrieving responses from cache (#34663)

The Angular ServiceWorker always uses a copy of the request without
headers for caching assets (in order to avoid issues with opaque
responses). Therefore, it was previously not possible to retrieve
resources from the cache if the response contained [Vary](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Vary) headers.

In addition to that, `Vary` headers do not work in all browsers (or work
differently) and may not work as intended with ServiceWorker caches. See
[this article](https://www.smashingmagazine.com/2017/11/understanding-vary-header) and the linked resources for more info.

This commit avoids the aforementioned issues by making sure the Angular
ServiceWorker always sets the `ignoreVary` option passed to
[Cache#match()](https://developer.mozilla.org/en-US/docs/Web/API/Cache/match) to `true`. This allows the ServiceWorker to correctly
retrieve cached responses with `Vary` headers, which was previously not
possible.

Fixes #36638

BREAKING CHANGE:

Previously, [Vary](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Vary)
headers would be taken into account when retrieving resources from the
cache, completely preventing the retrieval of cached assets (due to
ServiceWorker implementation details) and leading to unpredictable
behavior due to inconsistent/buggy implementations in different
browsers.

Now, `Vary` headers are ignored when retrieving resources from the
ServiceWorker caches, which can result in resources being retrieved even
when their headers are different. If your application needs to
differentiate its responses based on request headers, please make sure
the Angular ServiceWorker is [configured](https://angular.io/guide/service-worker-config)
to avoid caching the affected resources.

PR Close #34663

packages/service-worker/config/src/generator.ts

@@ -153,6 +153,9 @@ function withOrderedKeys<T extends {[key: string]: any}>(unorderedObj: T): T {
 }
 
 function buildCacheQueryOptions(inOptions?: Pick<CacheQueryOptions, 'ignoreSearch'>):
-    CacheQueryOptions|undefined {
+    CacheQueryOptions {
-  return inOptions;
+  return {
+    ignoreVary: true,
+    ...inOptions,
+  };
 }

packages/service-worker/config/test/generator_spec.ts

@@ -92,7 +92,7 @@ describe('Generator', () => {
           '\\/some\\/url\\?with\\+escaped\\+chars',
           '\\/test\\/relative\\/[^/]*\\.txt',
         ],
-        cacheQueryOptions: undefined,
+        cacheQueryOptions: {ignoreVary: true}
       }],
       dataGroups: [{
         name: 'other',
@@ -106,7 +106,7 @@ describe('Generator', () => {
         maxAge: 259200000,
         timeoutMs: 60000,
         version: 1,
-        cacheQueryOptions: undefined,
+        cacheQueryOptions: {ignoreVary: true}
       }],
       navigationUrls: [
         {positive: true, regex: '^\\/included\\/absolute\\/.*$'},
@@ -229,7 +229,7 @@ describe('Generator', () => {
           '/main.js',
         ],
         patterns: [],
-        cacheQueryOptions: {ignoreSearch: true}
+        cacheQueryOptions: {ignoreSearch: true, ignoreVary: true}
       }],
       dataGroups: [{
         name: 'other',
@@ -241,7 +241,7 @@ describe('Generator', () => {
         maxAge: 259200000,
         timeoutMs: 60000,
         version: 1,
-        cacheQueryOptions: {ignoreSearch: false}
+        cacheQueryOptions: {ignoreSearch: false, ignoreVary: true}
       }],
       navigationUrls: [
         {positive: true, regex: '^\\/.*$'},

packages/service-worker/test/integration_spec.ts

@@ -44,6 +44,7 @@ const manifest: Manifest = {
     updateMode: 'prefetch',
     urls: ['/only.txt'],
     patterns: [],
+    cacheQueryOptions: {ignoreVary: true},
   }],
   navigationUrls: [],
   hashTable: tmpHashTableForFs(dist),
@@ -60,6 +61,7 @@ const manifestUpdate: Manifest = {
     updateMode: 'prefetch',
     urls: ['/only.txt'],
     patterns: [],
+    cacheQueryOptions: {ignoreVary: true},
   }],
   navigationUrls: [],
   hashTable: tmpHashTableForFs(distUpdate),

packages/service-worker/worker/test/data_spec.ts

@@ -56,6 +56,7 @@ const manifest: Manifest = {
         '/bar.txt',
       ],
       patterns: [],
+      cacheQueryOptions: {ignoreVary: true},
     },
   ],
   dataGroups: [
@@ -67,7 +68,7 @@ const manifest: Manifest = {
       timeoutMs: 1000,
       maxAge: 5000,
       version: 1,
-      cacheQueryOptions: {ignoreSearch: true},
+      cacheQueryOptions: {ignoreVary: true, ignoreSearch: true},
     },
     {
       name: 'testRefresh',
@@ -78,6 +79,7 @@ const manifest: Manifest = {
       refreshAheadMs: 1000,
       maxAge: 5000,
       version: 1,
+      cacheQueryOptions: {ignoreVary: true},
     },
     {
       name: 'testFresh',
@@ -87,6 +89,7 @@ const manifest: Manifest = {
       timeoutMs: 1000,
       maxAge: 5000,
       version: 1,
+      cacheQueryOptions: {ignoreVary: true},
     },
   ],
   navigationUrls: [],
@@ -201,7 +204,8 @@ describe('data cache', () => {
       await makeRequest(scope, '/api/a');
       // the second one will be loaded from the cache
       await makeRequest(scope, '/api/a');
-      expect(matchSpy).toHaveBeenCalledWith(new MockRequest('/api/a'), {ignoreSearch: true});
+      expect(matchSpy).toHaveBeenCalledWith(
+          new MockRequest('/api/a'), {ignoreVary: true, ignoreSearch: true});
     });
 
     it('still matches if search differs but ignoreSearch is enabled', async () => {
@@ -312,6 +316,27 @@ describe('data cache', () => {
 
       expect(await res2).toBe('');
     });
+
+    it('CacheQueryOptions are passed through when falling back to cache', async () => {
+      const matchSpy = spyOn(MockCache.prototype, 'match').and.callThrough();
+      await makeRequest(scope, '/fresh/data');
+      server.clearRequests();
+      scope.updateServerState(serverUpdate);
+      serverUpdate.pause();
+      const [res, done] = makePendingRequest(scope, '/fresh/data');
+
+      await serverUpdate.nextRequest;
+
+      // Since the network request doesn't return within the timeout of 1,000ms,
+      // this should return cached data.
+      scope.advance(2000);
+      await res;
+      expect(matchSpy).toHaveBeenCalledWith(new MockRequest('/fresh/data'), {ignoreVary: true});
+
+      // Unpausing allows the worker to continue with caching.
+      serverUpdate.unpause();
+      await done;
+    });
   });
 });
 })();

packages/service-worker/worker/test/happy_spec.ts

@@ -70,6 +70,7 @@ const brokenManifest: Manifest = {
       '/foo.txt',
     ],
     patterns: [],
+    cacheQueryOptions: {ignoreVary: true},
   }],
   dataGroups: [],
   navigationUrls: processNavigationUrls(''),
@@ -89,6 +90,7 @@ const brokenLazyManifest: Manifest = {
         '/foo.txt',
       ],
       patterns: [],
+      cacheQueryOptions: {ignoreVary: true},
     },
     {
       name: 'lazy-assets',
@@ -98,6 +100,7 @@ const brokenLazyManifest: Manifest = {
         '/bar.txt',
       ],
       patterns: [],
+      cacheQueryOptions: {ignoreVary: true},
     },
   ],
   dataGroups: [],
@@ -143,6 +146,7 @@ const manifest: Manifest = {
       patterns: [
         '/unhashed/.*',
       ],
+      cacheQueryOptions: {ignoreVary: true},
     },
     {
       name: 'other',
@@ -153,6 +157,7 @@ const manifest: Manifest = {
         '/qux.txt',
       ],
       patterns: [],
+      cacheQueryOptions: {ignoreVary: true},
     },
     {
       name: 'lazy_prefetch',
@@ -165,6 +170,7 @@ const manifest: Manifest = {
         '/lazy/unchanged2.txt',
       ],
       patterns: [],
+      cacheQueryOptions: {ignoreVary: true},
     }
   ],
   dataGroups: [
@@ -177,6 +183,7 @@ const manifest: Manifest = {
       patterns: [
         '/api/.*',
       ],
+      cacheQueryOptions: {ignoreVary: true},
     },
     {
       name: 'api-static',
@@ -187,6 +194,7 @@ const manifest: Manifest = {
       patterns: [
         '/api-static/.*',
       ],
+      cacheQueryOptions: {ignoreVary: true},
     },
   ],
   navigationUrls: processNavigationUrls(''),
@@ -213,6 +221,7 @@ const manifestUpdate: Manifest = {
       patterns: [
         '/unhashed/.*',
       ],
+      cacheQueryOptions: {ignoreVary: true},
     },
     {
       name: 'other',
@@ -223,6 +232,7 @@ const manifestUpdate: Manifest = {
         '/qux.txt',
       ],
       patterns: [],
+      cacheQueryOptions: {ignoreVary: true},
     },
     {
       name: 'lazy_prefetch',
@@ -235,6 +245,7 @@ const manifestUpdate: Manifest = {
         '/lazy/unchanged2.txt',
       ],
       patterns: [],
+      cacheQueryOptions: {ignoreVary: true},
     }
   ],
   navigationUrls: processNavigationUrls(

packages/service-worker/worker/test/prefetch_spec.ts

@@ -9,6 +9,8 @@
 import {PrefetchAssetGroup} from '../src/assets';
 import {CacheDatabase} from '../src/db-cache';
 import {IdleScheduler} from '../src/idle';
+import {MockCache} from '../testing/cache';
+import {MockRequest} from '../testing/fetch';
 import {MockFileSystemBuilder, MockServerStateBuilder, tmpHashTable, tmpManifestSingleAssetGroup} from '../testing/mock';
 import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope';
 
@@ -19,7 +21,7 @@ if (!SwTestHarness.envIsSupported()) {
 }
 
 const dist = new MockFileSystemBuilder()
-                 .addFile('/foo.txt', 'this is foo')
+                 .addFile('/foo.txt', 'this is foo', {Vary: 'Accept'})
                  .addFile('/bar.txt', 'this is bar')
                  .build();
 
@@ -84,6 +86,12 @@ describe('prefetch assets', () => {
     const err = await errorFrom(group.initializeFully());
     expect(err.message).toContain('Hash mismatch');
   });
+  it('CacheQueryOptions are passed through', async () => {
+    await group.initializeFully();
+    const matchSpy = spyOn(MockCache.prototype, 'match').and.callThrough();
+    await group.handleFetch(scope.newRequest('/foo.txt'), scope);
+    expect(matchSpy).toHaveBeenCalledWith(new MockRequest('/foo.txt'), {ignoreVary: true});
+  });
 });
 })();
 

packages/service-worker/worker/testing/mock.ts

@@ -225,6 +225,7 @@ export function tmpManifestSingleAssetGroup(fs: MockFileSystem): Manifest {
         updateMode: 'prefetch',
         urls: files,
         patterns: [],
+        cacheQueryOptions: {ignoreVary: true}
       },
     ],
     navigationUrls: [],