Skip to content

Commit 2254ac2

Browse files
gkalpakIgorMinar
authored andcommitted
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]: https://github.com/w3c/ServiceWorker/blob/4cc72bd0f13359f16cc733d88bed4ea52ee063c0/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
1 parent 38c678f commit 2254ac2

File tree

2 files changed

+23
-2
lines changed

2 files changed

+23
-2
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -543,10 +543,10 @@ export class Driver implements Debuggable, UpdateSource {
543543
* Decide which version of the manifest to use for the event.
544544
*/
545545
private async assignVersion(event: FetchEvent): Promise<AppVersion|null> {
546-
// First, check whether the event has a client ID. If it does, the version may
546+
// First, check whether the event has a (non empty) client ID. If it does, the version may
547547
// already be associated.
548548
const clientId = event.clientId;
549-
if (clientId !== null) {
549+
if (clientId) {
550550
// Check if there is an assigned client id.
551551
if (this.clientVersionMap.has(clientId)) {
552552
// There is an assignment for this client already.

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,27 @@ const manifestUpdateHash = sha1(JSON.stringify(manifestUpdate));
320320
expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo v2');
321321
});
322322

323+
async_it('handles empty client ID', async() => {
324+
const navRequest = (url: string, clientId: string | null) =>
325+
makeRequest(scope, url, clientId, {
326+
headers: {Accept: 'text/plain, text/html, text/css'},
327+
mode: 'navigate',
328+
});
329+
330+
// Initialize the SW.
331+
expect(await navRequest('/foo/file1', '')).toEqual('this is foo');
332+
expect(await navRequest('/bar/file2', null)).toEqual('this is foo');
333+
await driver.initialized;
334+
335+
// Update to a new version.
336+
scope.updateServerState(serverUpdate);
337+
expect(await driver.checkForUpdate()).toEqual(true);
338+
339+
// Correctly handle navigation requests, even if `clientId` is null/empty.
340+
expect(await navRequest('/foo/file1', '')).toEqual('this is foo v2');
341+
expect(await navRequest('/bar/file2', null)).toEqual('this is foo v2');
342+
});
343+
323344
async_it('checks for updates on restart', async() => {
324345
expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo');
325346
await driver.initialized;

0 commit comments

Comments
 (0)