fix(service-worker): ignore invalid `only-if-cached` requests (#22883)

Under some circumstances (possibly related to opening Chrome DevTools),
requests are made with `cache: 'only-if-cached'` and `mode: 'no-cors'`.
These request will eventually fail, because `only-if-cached` is only
allowed to be used with `mode: 'same-origin'`.
This is likely a bug in Chrome DevTools.

This commit avoids errors related to such requests by not handling them.

Fixes #22362

PR Close #22883
This commit is contained in:
George Kalpakas 2018-03-20 15:10:59 +02:00 committed by Alex Rickabaugh
parent 9e9b8dd494
commit d9dc46e651
3 changed files with 43 additions and 6 deletions

View File

@ -94,6 +94,12 @@ export class Driver implements Debuggable, UpdateSource {
*/ */
private scheduledNavUpdateCheck: boolean = false; private scheduledNavUpdateCheck: boolean = false;
/**
* Keep track of whether we have logged an invalid `only-if-cached` request.
* (See `.onFetch()` for details.)
*/
private loggedInvalidOnlyIfCachedRequest: boolean = false;
/** /**
* A scheduler which manages a queue of tasks that need to be executed when the SW is * A scheduler which manages a queue of tasks that need to be executed when the SW is
* not doing any other work (not processing any other requests). * not doing any other work (not processing any other requests).
@ -156,12 +162,13 @@ export class Driver implements Debuggable, UpdateSource {
* asynchronous execution that eventually resolves for respondWith() and waitUntil(). * asynchronous execution that eventually resolves for respondWith() and waitUntil().
*/ */
private onFetch(event: FetchEvent): void { private onFetch(event: FetchEvent): void {
const req = event.request;
// The only thing that is served unconditionally is the debug page. // The only thing that is served unconditionally is the debug page.
if (this.adapter.parseUrl(event.request.url, this.scope.registration.scope).path === if (this.adapter.parseUrl(req.url, this.scope.registration.scope).path === '/ngsw/state') {
'/ngsw/state') {
// Allow the debugger to handle the request, but don't affect SW state in any // Allow the debugger to handle the request, but don't affect SW state in any
// other way. // other way.
event.respondWith(this.debugger.handleFetch(event.request)); event.respondWith(this.debugger.handleFetch(req));
return; return;
} }
@ -177,6 +184,24 @@ export class Driver implements Debuggable, UpdateSource {
return; return;
} }
// When opening DevTools in Chrome, a request is made for the current URL (and possibly related
// resources, e.g. scripts) with `cache: 'only-if-cached'` and `mode: 'no-cors'`. These request
// will eventually fail, because `only-if-cached` is only allowed to be used with
// `mode: 'same-origin'`.
// This is likely a bug in Chrome DevTools. Avoid handling such requests.
// (See also https://github.com/angular/angular/issues/22362.)
// TODO(gkalpak): Remove once no longer necessary (i.e. fixed in Chrome DevTools).
if ((req.cache as string) === 'only-if-cached' && req.mode !== 'same-origin') {
// Log the incident only the first time it happens, to avoid spamming the logs.
if (!this.loggedInvalidOnlyIfCachedRequest) {
this.loggedInvalidOnlyIfCachedRequest = true;
this.debugger.log(
`Ignoring invalid request: 'only-if-cached' can be set only with 'same-origin' mode`,
`Driver.fetch(${req.url}, cache: ${req.cache}, mode: ${req.mode})`);
}
return;
}
// Past this point, the SW commits to handling the request itself. This could still // Past this point, the SW commits to handling the request itself. This could still
// fail (and result in `state` being set to `SAFE_MODE`), but even in that case the // fail (and result in `state` being set to `SAFE_MODE`), but even in that case the
// SW will still deliver a response. // SW will still deliver a response.
@ -611,8 +636,8 @@ export class Driver implements Debuggable, UpdateSource {
* offline (detected as response status 504). * offline (detected as response status 504).
*/ */
private async fetchLatestManifest(ignoreOfflineError?: false): Promise<Manifest>; private async fetchLatestManifest(ignoreOfflineError?: false): Promise<Manifest>;
private async fetchLatestManifest(ignoreOfflineError: true): Promise<Manifest | null>; private async fetchLatestManifest(ignoreOfflineError: true): Promise<Manifest|null>;
private async fetchLatestManifest(ignoreOfflineError = false): Promise<Manifest | null> { private async fetchLatestManifest(ignoreOfflineError = false): Promise<Manifest|null> {
const res = const res =
await this.safeFetch(this.adapter.newRequest('ngsw.json?ngsw-cache-bust=' + Math.random())); await this.safeFetch(this.adapter.newRequest('ngsw.json?ngsw-cache-bust=' + Math.random()));
if (!res.ok) { if (!res.ok) {

View File

@ -708,6 +708,15 @@ const manifestUpdateHash = sha1(JSON.stringify(manifestUpdate));
expect(driver.state).toEqual(DriverReadyState.EXISTING_CLIENTS_ONLY); expect(driver.state).toEqual(DriverReadyState.EXISTING_CLIENTS_ONLY);
}); });
async_it('ignores invalid `only-if-cached` requests ', async() => {
const requestFoo = (cache: RequestCache | 'only-if-cached', mode: RequestMode) =>
makeRequest(scope, '/foo.txt', undefined, {cache, mode});
expect(await requestFoo('default', 'no-cors')).toBe('this is foo');
expect(await requestFoo('only-if-cached', 'same-origin')).toBe('this is foo');
expect(await requestFoo('only-if-cached', 'no-cors')).toBeNull();
});
}); });
}); });
})(); })();

View File

@ -110,6 +110,9 @@ export class MockRequest extends MockBody implements Request {
Object.keys(headers).forEach(header => { this.headers.set(header, headers[header]); }); Object.keys(headers).forEach(header => { this.headers.set(header, headers[header]); });
} }
} }
if (init.cache !== undefined) {
this.cache = init.cache;
}
if (init.mode !== undefined) { if (init.mode !== undefined) {
this.mode = init.mode; this.mode = init.mode;
} }