From 0bdd30e34f87305085afc2825749ea2dc04a9409 Mon Sep 17 00:00:00 2001 From: Heo Sangmin Date: Thu, 10 May 2018 00:11:43 +0900 Subject: [PATCH] fix(service-worker): check platformBrowser before accessing navigator.serviceWorker (#21231) PR Close #21231 --- packages/service-worker/src/low_level.ts | 9 +- packages/service-worker/src/module.ts | 4 +- packages/service-worker/test/comm_spec.ts | 103 ++++++++++++++++-- .../service-worker/test/integration_spec.ts | 2 +- 4 files changed, 99 insertions(+), 19 deletions(-) diff --git a/packages/service-worker/src/low_level.ts b/packages/service-worker/src/low_level.ts index fbb201d078..6929c90d22 100644 --- a/packages/service-worker/src/low_level.ts +++ b/packages/service-worker/src/low_level.ts @@ -6,8 +6,6 @@ * found in the LICENSE file at https://angular.io/license */ -import {isPlatformBrowser} from '@angular/common'; -import {Inject, PLATFORM_ID} from '@angular/core'; import {ConnectableObservable, Observable, concat, defer, fromEvent, of , throwError} from 'rxjs'; import {filter, map, publish, switchMap, take, tap} from 'rxjs/operators'; @@ -71,11 +69,8 @@ export class NgswCommChannel { */ readonly events: Observable; - constructor( - private serviceWorker: ServiceWorkerContainer|undefined, - @Inject(PLATFORM_ID) platformId: string) { - if (!serviceWorker || !isPlatformBrowser(platformId)) { - this.serviceWorker = undefined; + constructor(private serviceWorker: ServiceWorkerContainer|undefined) { + if (!serviceWorker) { this.worker = this.events = this.registration = errorObservable(ERR_SW_NOT_SUPPORTED); } else { const controllerChangeEvents = diff --git a/packages/service-worker/src/module.ts b/packages/service-worker/src/module.ts index fd38a8a136..1947582891 100644 --- a/packages/service-worker/src/module.ts +++ b/packages/service-worker/src/module.ts @@ -8,7 +8,6 @@ import {isPlatformBrowser} from '@angular/common'; import {APP_INITIALIZER, ApplicationRef, InjectionToken, Injector, ModuleWithProviders, NgModule, PLATFORM_ID} from '@angular/core'; -import {Observable} from 'rxjs'; import {filter, take} from 'rxjs/operators'; import {NgswCommChannel} from './low_level'; @@ -53,7 +52,8 @@ export function ngswAppInitializer( export function ngswCommChannelFactory( opts: RegistrationOptions, platformId: string): NgswCommChannel { return new NgswCommChannel( - opts.enabled !== false ? navigator.serviceWorker : undefined, platformId); + isPlatformBrowser(platformId) && opts.enabled !== false ? navigator.serviceWorker : + undefined); } /** diff --git a/packages/service-worker/test/comm_spec.ts b/packages/service-worker/test/comm_spec.ts index 46400c4242..49a5a312fb 100644 --- a/packages/service-worker/test/comm_spec.ts +++ b/packages/service-worker/test/comm_spec.ts @@ -6,9 +6,11 @@ * found in the LICENSE file at https://angular.io/license */ +import {PLATFORM_ID} from '@angular/core'; import {TestBed} from '@angular/core/testing'; import {NgswCommChannel} from '../src/low_level'; +import {RegistrationOptions, ngswCommChannelFactory} from '../src/module'; import {SwPush} from '../src/push'; import {SwUpdate} from '../src/update'; import {MockServiceWorkerContainer, MockServiceWorkerRegistration} from '../testing/mock'; @@ -19,12 +21,12 @@ import {MockServiceWorkerContainer, MockServiceWorkerRegistration} from '../test let comm: NgswCommChannel; beforeEach(() => { mock = new MockServiceWorkerContainer(); - comm = new NgswCommChannel(mock as any, 'browser'); + comm = new NgswCommChannel(mock as any); }); describe('NgswCommsChannel', () => { it('can access the registration when it comes before subscription', (done: DoneFn) => { const mock = new MockServiceWorkerContainer(); - const comm = new NgswCommChannel(mock as any, 'browser'); + const comm = new NgswCommChannel(mock as any); const regPromise = mock.getRegistration() as any as MockServiceWorkerRegistration; mock.setupSw(); @@ -33,18 +35,101 @@ import {MockServiceWorkerContainer, MockServiceWorkerRegistration} from '../test }); it('can access the registration when it comes after subscription', (done: DoneFn) => { const mock = new MockServiceWorkerContainer(); - const comm = new NgswCommChannel(mock as any, 'browser'); + const comm = new NgswCommChannel(mock as any); const regPromise = mock.getRegistration() as any as MockServiceWorkerRegistration; (comm as any).registration.subscribe((reg: any) => { done(); }); mock.setupSw(); }); - it('is disabled for platform-server', () => { - const mock = new MockServiceWorkerContainer(); - const comm = new NgswCommChannel(mock as any, 'server'); - expect(comm.isEnabled).toEqual(false); + }); + describe('ngswCommChannelFactory', () => { + it('gives disabled NgswCommChannel for platform-server', () => { + TestBed.configureTestingModule({ + providers: [ + {provide: PLATFORM_ID, useValue: 'server'}, + {provide: RegistrationOptions, useValue: {enabled: true}}, { + provide: NgswCommChannel, + useFactory: ngswCommChannelFactory, + deps: [RegistrationOptions, PLATFORM_ID] + } + ] + }); + + expect(TestBed.get(NgswCommChannel).isEnabled).toEqual(false); }); + it('gives disabled NgswCommChannel when \'enabled\' option is false', () => { + TestBed.configureTestingModule({ + providers: [ + {provide: PLATFORM_ID, useValue: 'browser'}, + {provide: RegistrationOptions, useValue: {enabled: false}}, { + provide: NgswCommChannel, + useFactory: ngswCommChannelFactory, + deps: [RegistrationOptions, PLATFORM_ID] + } + ] + }); + + expect(TestBed.get(NgswCommChannel).isEnabled).toEqual(false); + }); + it('gives disabled NgswCommChannel when navigator.serviceWorker is undefined', () => { + TestBed.configureTestingModule({ + providers: [ + {provide: PLATFORM_ID, useValue: 'browser'}, + {provide: RegistrationOptions, useValue: {enabled: true}}, + { + provide: NgswCommChannel, + useFactory: ngswCommChannelFactory, + deps: [RegistrationOptions, PLATFORM_ID], + }, + ], + }); + + const context: any = global || window; + const originalDescriptor = Object.getOwnPropertyDescriptor(context, 'navigator'); + const patchedDescriptor = {value: {serviceWorker: undefined}, configurable: true}; + + try { + // Set `navigator` to `{serviceWorker: undefined}`. + Object.defineProperty(context, 'navigator', patchedDescriptor); + expect(TestBed.get(NgswCommChannel).isEnabled).toBe(false); + } finally { + if (originalDescriptor) { + Object.defineProperty(context, 'navigator', originalDescriptor); + } else { + delete context.navigator; + } + } + }); + it('gives enabled NgswCommChannel when browser supports SW and enabled option is true', + () => { + TestBed.configureTestingModule({ + providers: [ + {provide: PLATFORM_ID, useValue: 'browser'}, + {provide: RegistrationOptions, useValue: {enabled: true}}, { + provide: NgswCommChannel, + useFactory: ngswCommChannelFactory, + deps: [RegistrationOptions, PLATFORM_ID] + } + ] + }); + + const context: any = global || window; + const originalDescriptor = Object.getOwnPropertyDescriptor(context, 'navigator'); + const patchedDescriptor = {value: {serviceWorker: mock}, configurable: true}; + + try { + // Set `navigator` to `{serviceWorker: mock}`. + Object.defineProperty(context, 'navigator', patchedDescriptor); + expect(TestBed.get(NgswCommChannel).isEnabled).toBe(true); + } finally { + if (originalDescriptor) { + Object.defineProperty(context, 'navigator', originalDescriptor); + } else { + delete context.navigator; + } + } + }); }); describe('SwPush', () => { let push: SwPush; @@ -76,7 +161,7 @@ import {MockServiceWorkerContainer, MockServiceWorkerRegistration} from '../test expect(() => TestBed.get(SwPush)).not.toThrow(); }); describe('with no SW', () => { - beforeEach(() => { comm = new NgswCommChannel(undefined, 'browser'); }); + beforeEach(() => { comm = new NgswCommChannel(undefined); }); it('can be instantiated', () => { push = new SwPush(comm); }); it('does not crash on subscription to observables', () => { push = new SwPush(comm); @@ -170,7 +255,7 @@ import {MockServiceWorkerContainer, MockServiceWorkerRegistration} from '../test expect(() => TestBed.get(SwUpdate)).not.toThrow(); }); describe('with no SW', () => { - beforeEach(() => { comm = new NgswCommChannel(undefined, 'browser'); }); + beforeEach(() => { comm = new NgswCommChannel(undefined); }); it('can be instantiated', () => { update = new SwUpdate(comm); }); it('does not crash on subscription to observables', () => { update = new SwUpdate(comm); diff --git a/packages/service-worker/test/integration_spec.ts b/packages/service-worker/test/integration_spec.ts index f4ff957fa4..acd1ed0b81 100644 --- a/packages/service-worker/test/integration_spec.ts +++ b/packages/service-worker/test/integration_spec.ts @@ -80,7 +80,7 @@ const serverUpdate = async_beforeEach(async() => { // Fire up the client. mock = new MockServiceWorkerContainer(); - comm = new NgswCommChannel(mock as any, 'browser'); + comm = new NgswCommChannel(mock as any); scope = new SwTestHarnessBuilder().withServerState(server).build(); driver = new Driver(scope, scope, new CacheDatabase(scope, scope));