fix(service-worker): properly handle invalid hashes in all scenarios

packages/service-worker/worker/src/assets.ts

@@ -9,6 +9,7 @@
 import {Adapter, Context} from './adapter';
 import {CacheState, UpdateCacheStatus, UpdateSource, UrlMetadata} from './api';
 import {Database, Table} from './database';
+import {SwCriticalError} from './error';
 import {IdleScheduler} from './idle';
 import {AssetGroupConfig} from './manifest';
 import {sha1Binary} from './sha1';
@@ -345,7 +346,7 @@ export abstract class AssetGroup {
     if ((res as any)['redirected'] && !!res.url) {
       // If the redirect limit is exhausted, fail with an error.
       if (redirectLimit === 0) {
-        throw new Error(
+        throw new SwCriticalError(
             `Response hit redirect limit (fetchFromNetwork): request redirected too many times, next is ${res.url}`);
       }
 
@@ -409,7 +410,7 @@ export abstract class AssetGroup {
 
         // If the response was unsuccessful, there's nothing more that can be done.
         if (!cacheBustedResult.ok) {
-          throw new Error(
+          throw new SwCriticalError(
               `Response not Ok (cacheBustedFetchFromNetwork): cache busted request for ${req.url} returned response ${cacheBustedResult.status} ${cacheBustedResult.statusText}`);
         }
 
@@ -419,7 +420,7 @@ export abstract class AssetGroup {
         // If the cache-busted version doesn't match, then the manifest is not an accurate
         // representation of the server's current set of files, and the SW should give up.
         if (canonicalHash !== cacheBustedHash) {
-          throw new Error(
+          throw new SwCriticalError(
               `Hash mismatch (cacheBustedFetchFromNetwork): ${req.url}: expected ${canonicalHash}, got ${cacheBustedHash} (after cache busting)`);
         }
 

packages/service-worker/worker/src/driver.ts

@@ -11,6 +11,7 @@ import {CacheState, DebugIdleState, DebugState, DebugVersion, Debuggable, Update
 import {AppVersion} from './app-version';
 import {Database, Table} from './database';
 import {DebugHandler} from './debug';
+import {SwCriticalError} from './error';
 import {IdleScheduler} from './idle';
 import {Manifest, ManifestHash, hashManifest} from './manifest';
 import {MsgAny, isMsgActivateUpdate, isMsgCheckForUpdates} from './msg';
@@ -38,7 +39,7 @@ interface LatestEntry {
   latest: string;
 }
 
-enum DriverReadyState {
+export enum DriverReadyState {
   // The SW is operating in a normal mode, responding to all traffic.
   NORMAL,
 
@@ -57,7 +58,7 @@ export class Driver implements Debuggable, UpdateSource {
    * Tracks the current readiness condition under which the SW is operating. This controls
    * whether the SW attempts to respond to some or all requests.
    */
-  private state: DriverReadyState = DriverReadyState.NORMAL;
+  state: DriverReadyState = DriverReadyState.NORMAL;
   private stateMessage: string = '(nominal)';
 
   /**
@@ -351,8 +352,22 @@ export class Driver implements Debuggable, UpdateSource {
       return this.safeFetch(event.request);
     }
 
-    // Handle the request. First try the AppVersion. If that doesn't work, fall back on the network.
-    const res = await appVersion.handleFetch(event.request, event);
+    let res: Response|null = null;
+    try {
+      // Handle the request. First try the AppVersion. If that doesn't work, fall back on the
+      // network.
+      res = await appVersion.handleFetch(event.request, event);
+    } catch (err) {
+      if (err.isCritical) {
+        // Something went wrong with the activation of this version.
+        await this.versionFailed(appVersion, err, this.latestHash === appVersion.manifestHash);
+
+        event.waitUntil(this.idle.trigger());
+        return this.safeFetch(event.request);
+      }
+      throw err;
+    }
+
 
     // The AppVersion will only return null if the manifest doesn't specify what to do about this
     // request. In that case, just fall back on the network.
@@ -482,7 +497,7 @@ export class Driver implements Debuggable, UpdateSource {
         // Attempt to schedule or initialize this version. If this operation is
         // successful, then initialization either succeeded or was scheduled. If
         // it fails, then full initialization was attempted and failed.
-        await this.scheduleInitialization(this.versions.get(hash) !);
+        await this.scheduleInitialization(this.versions.get(hash) !, this.latestHash === hash);
       } catch (err) {
         this.debugger.log(err, `initialize: schedule init of ${hash}`);
         return false;
@@ -623,13 +638,13 @@ export class Driver implements Debuggable, UpdateSource {
    * when the SW is not busy and has connectivity. This returns a Promise which must be
    * awaited, as under some conditions the AppVersion might be initialized immediately.
    */
-  private async scheduleInitialization(appVersion: AppVersion): Promise<void> {
+  private async scheduleInitialization(appVersion: AppVersion, latest: boolean): Promise<void> {
     const initialize = async() => {
       try {
         await appVersion.initializeFully();
       } catch (err) {
         this.debugger.log(err, `initializeFully for ${appVersion.manifestHash}`);
-        await this.versionFailed(appVersion, err);
+        await this.versionFailed(appVersion, err, latest);
       }
     };
     // TODO: better logic for detecting localhost.
@@ -639,7 +654,7 @@ export class Driver implements Debuggable, UpdateSource {
     this.idle.schedule(`initialization(${appVersion.manifestHash})`, initialize);
   }
 
-  private async versionFailed(appVersion: AppVersion, err: Error): Promise<void> {
+  private async versionFailed(appVersion: AppVersion, err: Error, latest: boolean): Promise<void> {
     // This particular AppVersion is broken. First, find the manifest hash.
     const broken =
         Array.from(this.versions.entries()).find(([hash, version]) => version === appVersion);
@@ -653,7 +668,7 @@ export class Driver implements Debuggable, UpdateSource {
 
     // The action taken depends on whether the broken manifest is the active (latest) or not.
     // If so, the SW cannot accept new clients, but can continue to service old ones.
-    if (this.latestHash === brokenHash) {
+    if (this.latestHash === brokenHash || latest) {
       // The latest manifest is broken. This means that new clients are at the mercy of the
       // network, but caches continue to be valid for previous versions. This is
       // unfortunate but unavoidable.
@@ -711,9 +726,10 @@ export class Driver implements Debuggable, UpdateSource {
   }
 
   async checkForUpdate(): Promise<boolean> {
+    let hash: string = '(unknown)';
     try {
       const manifest = await this.fetchLatestManifest();
-      const hash = hashManifest(manifest);
+      hash = hashManifest(manifest);
 
       // Check whether this is really an update.
       if (this.versions.has(hash)) {
@@ -723,7 +739,12 @@ export class Driver implements Debuggable, UpdateSource {
       await this.setupUpdate(manifest, hash);
 
       return true;
-    } catch (_) {
+    } catch (err) {
+      this.debugger.log(err, `Error occurred while updating to manifest ${hash}`);
+
+      this.state = DriverReadyState.EXISTING_CLIENTS_ONLY;
+      this.stateMessage = `Degraded due to failed initialization: ${errorToString(err)}`;
+
       return false;
     }
   }

packages/service-worker/worker/src/error.ts

@@ -0,0 +1,9 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+export class SwCriticalError extends Error { readonly isCritical: boolean = true; }

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

@@ -7,7 +7,7 @@
  */
 
 import {CacheDatabase} from '../src/db-cache';
-import {Driver} from '../src/driver';
+import {Driver, DriverReadyState} from '../src/driver';
 import {Manifest} from '../src/manifest';
 import {sha1} from '../src/sha1';
 import {MockRequest} from '../testing/fetch';
@@ -36,6 +36,24 @@ const distUpdate =
         .addUnhashedFile('/unhashed/a.txt', 'this is unhashed v2', {'Cache-Control': 'max-age=10'})
         .build();
 
+const brokenFs = new MockFileSystemBuilder().addFile('/foo.txt', 'this is foo').build();
+
+const brokenManifest: Manifest = {
+  configVersion: 1,
+  index: '/foo.txt',
+  assetGroups: [{
+    name: 'assets',
+    installMode: 'prefetch',
+    updateMode: 'prefetch',
+    urls: [
+      '/foo.txt',
+    ],
+    patterns: [],
+  }],
+  dataGroups: [],
+  hashTable: tmpHashTableForFs(brokenFs, {'/foo.txt': true}),
+};
+
 const manifest: Manifest = {
   configVersion: 1,
   appData: {
@@ -132,6 +150,9 @@ const serverUpdate =
         .withRedirect('/redirected.txt', '/redirect-target.txt', 'this was a redirect')
         .build();
 
+const brokenServer =
+    new MockServerStateBuilder().withStaticFiles(brokenFs).withManifest(brokenManifest).build();
+
 const server404 = new MockServerStateBuilder().withStaticFiles(dist).build();
 
 const scope = new SwTestHarnessBuilder().withServerState(server).build();
@@ -673,6 +694,34 @@ const manifestUpdateHash = sha1(JSON.stringify(manifestUpdate));
         server.assertSawRequestFor('/baz');
       });
     });
+
+    describe('bugs', () => {
+      async_it('does not crash with bad index hash', async() => {
+        scope = new SwTestHarnessBuilder().withServerState(brokenServer).build();
+        (scope.registration as any).scope = 'http://site.com';
+        driver = new Driver(scope, scope, new CacheDatabase(scope, scope));
+
+        expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo');
+      });
+
+      async_it('enters degraded mode when update has a bad index', async() => {
+        expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo');
+        await driver.initialized;
+        server.clearRequests();
+
+        scope = new SwTestHarnessBuilder()
+                    .withCacheState(scope.caches.dehydrate())
+                    .withServerState(brokenServer)
+                    .build();
+        driver = new Driver(scope, scope, new CacheDatabase(scope, scope));
+        await driver.checkForUpdate();
+
+        scope.advance(12000);
+        await driver.idle.empty;
+
+        expect(driver.state).toEqual(DriverReadyState.EXISTING_CLIENTS_ONLY);
+      });
+    });
   });
 })();
 

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

@@ -194,12 +194,16 @@ export function tmpManifestSingleAssetGroup(fs: MockFileSystem): Manifest {
   };
 }
 
-export function tmpHashTableForFs(fs: MockFileSystem): {[url: string]: string} {
+export function tmpHashTableForFs(
+    fs: MockFileSystem, breakHashes: {[url: string]: boolean} = {}): {[url: string]: string} {
   const table: {[url: string]: string} = {};
   fs.list().forEach(path => {
     const file = fs.lookup(path) !;
     if (file.hashThisFile) {
       table[path] = file.hash;
+      if (breakHashes[path]) {
+        table[path] = table[path].split('').reverse().join('');
+      }
     }
   });
   return table;