From d63ecf4c5f4523457c7dd344a3c603306cf94a74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrien=20Verg=C3=A9?= Date: Fri, 5 Jun 2020 11:30:07 +0200 Subject: [PATCH] fix(service-worker): Don't stay locked in EXISTING_CLIENTS_ONLY if corrupted data (#37453) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Problem** After #31109 and #31865, it's still possible to get locked in state `EXISTING_CLIENTS_ONLY`, without any possibility to get out (even by pushing new updates on the server). More specifically, if control doc `/latest` of `ngsw:/:db:control` once gets a bad value, then the service worker will fail early, and won't be able to overwrite `/latest` with new, valid values (the ones from future updates). For example, once in this state, URL `/ngsw/state` will show: NGSW Debug Info: Driver state: EXISTING_CLIENTS_ONLY (Degraded due to failed initialization: Invariant violated (initialize): latest hash 8b75… has no known manifest Error: Invariant violated (initialize): latest hash 8b75… has no known manifest at Driver. (https://my.app/ngsw-worker.js:2302:27) at Generator.next () at fulfilled (https://my.app/ngsw-worker.js:175:62)) Latest manifest hash: 8b75… Last update check: 22s971u ... with hash `8b75…` corresponding to no installed version. **Solution** Currently, when such a case happens, the service worker [simply fails with an assertion][1]. Because this failure happens early, and is not handled, the service worker is not able to update `/latest` to new installed app versions. I propose to detect this corrupted case (a `latest` hash that doesn't match any installed version) a few lines above, so that the service worker can correctly call its [already existing cleaning code][2]. [1]: https://github.com/angular/angular/blob/3569fdf/packages/service-worker/worker/src/driver.ts#L559-L563 [2]: https://github.com/angular/angular/blob/3569fdf/packages/service-worker/worker/src/driver.ts#L505-L519 This change successfully fixes the problem described above. Unit test written with the help of George Kalpakas. Thank you! PR Close #37453 --- packages/service-worker/worker/src/driver.ts | 9 +++++++ .../service-worker/worker/test/happy_spec.ts | 24 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/packages/service-worker/worker/src/driver.ts b/packages/service-worker/worker/src/driver.ts index 2a3476fd24..5377dc4ff2 100644 --- a/packages/service-worker/worker/src/driver.ts +++ b/packages/service-worker/worker/src/driver.ts @@ -490,6 +490,15 @@ export class Driver implements Debuggable, UpdateSource { table.read('latest'), ]); + // Make sure latest manifest is correctly installed. If not (e.g. corrupted data), + // it could stay locked in EXISTING_CLIENTS_ONLY or SAFE_MODE state. + if (!this.versions.has(latest.latest) && !manifests.hasOwnProperty(latest.latest)) { + this.debugger.log( + `Missing manifest for latest version hash ${latest.latest}`, + 'initialize: read from DB'); + throw new Error(`Missing manifest for latest hash ${latest.latest}`); + } + // Successfully loaded from saved state. This implies a manifest exists, so // the update check needs to happen in the background. this.idle.schedule('init post-load (update, cleanup)', async () => { diff --git a/packages/service-worker/worker/test/happy_spec.ts b/packages/service-worker/worker/test/happy_spec.ts index 7699c4c3cd..fbb3d84eeb 100644 --- a/packages/service-worker/worker/test/happy_spec.ts +++ b/packages/service-worker/worker/test/happy_spec.ts @@ -1426,6 +1426,30 @@ describe('Driver', () => { expect(driver.state).toBe(DriverReadyState.NORMAL); }); + it('should not enter degraded mode if manifest for latest hash is missing upon initialization', + async () => { + // Initialize the SW. + scope.handleMessage({action: 'INITIALIZE'}, null); + await driver.initialized; + expect(driver.state).toBe(DriverReadyState.NORMAL); + + // Ensure the data has been stored in the DB. + const db: MockCache = await scope.caches.open('ngsw:/:db:control') as any; + const getLatestHashFromDb = async () => (await (await db.match('/latest')).json()).latest; + expect(await getLatestHashFromDb()).toBe(manifestHash); + + // Change the latest hash to not correspond to any manifest. + await db.put('/latest', new MockResponse('{"latest": "wrong-hash"}')); + expect(await getLatestHashFromDb()).toBe('wrong-hash'); + + // Re-initialize the SW and ensure it does not enter a degraded mode. + driver.initialized = null; + scope.handleMessage({action: 'INITIALIZE'}, null); + await driver.initialized; + expect(driver.state).toBe(DriverReadyState.NORMAL); + expect(await getLatestHashFromDb()).toBe(manifestHash); + }); + it('ignores invalid `only-if-cached` requests ', async () => { const requestFoo = (cache: RequestCache|'only-if-cached', mode: RequestMode) => makeRequest(scope, '/foo.txt', undefined, {cache, mode});