fix(service-worker): correctly handle requests with empty `clientId` (#23625)

Requests from clients that are not assigned a client ID by the browser
will produce `fetch` events with `null` or empty (`''`) `clientId`s.

Previously, the ServiceWorker only handled `null` values correctly. Yet
empty strings are also valid (see for example [here][1] and [there][2]).
With this commit, the SW will interpret _all_ falsy `clientId` values
the same (i.e. "no client ID assigned") and handle them appropriately.

Related Chromium issue/discussion: [#832105][3]

[1]: 4cc72bd0f1/docs/index.bs (L1392)
[2]: https://w3c.github.io/ServiceWorker/#fetchevent-interface
[3]: https://bugs.chromium.org/p/chromium/issues/detail?id=832105

Fixes #23526

PR Close #23625
This commit is contained in:
George Kalpakas 2018-05-01 01:55:32 +03:00 committed by Igor Minar
parent d6b1466c81
commit e0ed59e55f
2 changed files with 23 additions and 2 deletions

View File

@ -543,10 +543,10 @@ export class Driver implements Debuggable, UpdateSource {
* Decide which version of the manifest to use for the event. * Decide which version of the manifest to use for the event.
*/ */
private async assignVersion(event: FetchEvent): Promise<AppVersion|null> { private async assignVersion(event: FetchEvent): Promise<AppVersion|null> {
// First, check whether the event has a client ID. If it does, the version may // First, check whether the event has a (non empty) client ID. If it does, the version may
// already be associated. // already be associated.
const clientId = event.clientId; const clientId = event.clientId;
if (clientId !== null) { if (clientId) {
// Check if there is an assigned client id. // Check if there is an assigned client id.
if (this.clientVersionMap.has(clientId)) { if (this.clientVersionMap.has(clientId)) {
// There is an assignment for this client already. // There is an assignment for this client already.

View File

@ -320,6 +320,27 @@ const manifestUpdateHash = sha1(JSON.stringify(manifestUpdate));
expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo v2'); expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo v2');
}); });
async_it('handles empty client ID', async() => {
const navRequest = (url: string, clientId: string | null) =>
makeRequest(scope, url, clientId, {
headers: {Accept: 'text/plain, text/html, text/css'},
mode: 'navigate',
});
// Initialize the SW.
expect(await navRequest('/foo/file1', '')).toEqual('this is foo');
expect(await navRequest('/bar/file2', null)).toEqual('this is foo');
await driver.initialized;
// Update to a new version.
scope.updateServerState(serverUpdate);
expect(await driver.checkForUpdate()).toEqual(true);
// Correctly handle navigation requests, even if `clientId` is null/empty.
expect(await navRequest('/foo/file1', '')).toEqual('this is foo v2');
expect(await navRequest('/bar/file2', null)).toEqual('this is foo v2');
});
async_it('checks for updates on restart', async() => { async_it('checks for updates on restart', async() => {
expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo'); expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo');
await driver.initialized; await driver.initialized;