From 8ab815ae932dd337d45a2b31bd7c1fae8a1d98c4 Mon Sep 17 00:00:00 2001 From: silverwind Date: Wed, 12 May 2021 20:36:53 +0200 Subject: [PATCH] Unregister non-matching serviceworkers (#15834) * Unregister non-matching serviceworkers With the addition of the /assets url, users who visited a previous version of the site now may have two active service workers, one with the old scope `/` and one with scope `/assets`. This check for serviceworkers that do not match the current script path and unregisters them. Also included is a small refactor to publicpath.js which was simplified because AssetUrlPrefix is always present now. Also it makes use of the new joinPaths helper too. Fixes: https://github.com/go-gitea/gitea/pull/15823 --- models/user.go | 3 +- web_src/js/features/serviceworker.js | 50 +++++++++++++++------------- web_src/js/publicpath.js | 8 ++--- web_src/js/utils.js | 10 ++++++ web_src/js/utils.test.js | 41 ++++++++++++++++++++++- 5 files changed, 81 insertions(+), 31 deletions(-) diff --git a/models/user.go b/models/user.go index 02ccfea47..c7b44bdb7 100644 --- a/models/user.go +++ b/models/user.go @@ -789,6 +789,7 @@ var ( "debug", "error", "explore", + "favicon.ico", "ghost", "help", "install", @@ -807,10 +808,10 @@ var ( "repo", "robots.txt", "search", + "serviceworker.js", "stars", "template", "user", - "favicon.ico", } reservedUserPatterns = []string{"*.keys", "*.gpg"} diff --git a/web_src/js/features/serviceworker.js b/web_src/js/features/serviceworker.js index 57b2c399f..e92d21cde 100644 --- a/web_src/js/features/serviceworker.js +++ b/web_src/js/features/serviceworker.js @@ -1,18 +1,26 @@ -const {UseServiceWorker, AppSubUrl, AppVer} = window.config; -const cachePrefix = 'static-cache-v'; // actual version is set in the service worker script +import {joinPaths} from '../utils.js'; -async function unregister() { - const registrations = await navigator.serviceWorker.getRegistrations(); - await Promise.all(registrations.map((registration) => { - return registration.active && registration.unregister(); - })); +const {UseServiceWorker, AppSubUrl, AssetUrlPrefix, AppVer} = window.config; +const cachePrefix = 'static-cache-v'; // actual version is set in the service worker script +const workerAssetPath = joinPaths(AssetUrlPrefix, 'serviceworker.js'); + +async function unregisterAll() { + for (const registration of await navigator.serviceWorker.getRegistrations()) { + if (registration.active) await registration.unregister(); + } +} + +async function unregisterOtherWorkers() { + for (const registration of await navigator.serviceWorker.getRegistrations()) { + const scriptURL = registration.active?.scriptURL || ''; + if (!scriptURL.endsWith(workerAssetPath)) await registration.unregister(); + } } async function invalidateCache() { - const cacheKeys = await caches.keys(); - await Promise.all(cacheKeys.map((key) => { - return key.startsWith(cachePrefix) && caches.delete(key); - })); + for (const key of await caches.keys()) { + if (key.startsWith(cachePrefix)) caches.delete(key); + } } async function checkCacheValidity() { @@ -30,24 +38,20 @@ export default async function initServiceWorker() { if (!('serviceWorker' in navigator)) return; if (UseServiceWorker) { + // unregister all service workers where scriptURL does not match the current one + await unregisterOtherWorkers(); try { // normally we'd serve the service worker as a static asset from AssetUrlPrefix but // the spec strictly requires it to be same-origin so it has to be AppSubUrl to work - await Promise.all([ - checkCacheValidity(), - navigator.serviceWorker.register(`${AppSubUrl}/assets/serviceworker.js`), - ]); + await checkCacheValidity(); + await navigator.serviceWorker.register(joinPaths(AppSubUrl, workerAssetPath)); } catch (err) { console.error(err); - await Promise.all([ - invalidateCache(), - unregister(), - ]); + await invalidateCache(); + await unregisterAll(); } } else { - await Promise.all([ - invalidateCache(), - unregister(), - ]); + await invalidateCache(); + await unregisterAll(); } } diff --git a/web_src/js/publicpath.js b/web_src/js/publicpath.js index bfc81b876..b68a91d86 100644 --- a/web_src/js/publicpath.js +++ b/web_src/js/publicpath.js @@ -1,10 +1,6 @@ // This sets up the URL prefix used in webpack's chunk loading. // This file must be imported before any lazy-loading is being attempted. +import {joinPaths} from './utils.js'; const {AssetUrlPrefix} = window.config; -if (AssetUrlPrefix) { - __webpack_public_path__ = AssetUrlPrefix.endsWith('/') ? AssetUrlPrefix : `${AssetUrlPrefix}/`; -} else { - const url = new URL(document.currentScript.src); - __webpack_public_path__ = url.pathname.replace(/\/[^/]*?\/[^/]*?$/, '/'); -} +__webpack_public_path__ = joinPaths(AssetUrlPrefix, '/'); diff --git a/web_src/js/utils.js b/web_src/js/utils.js index bc8474506..58d0d6b39 100644 --- a/web_src/js/utils.js +++ b/web_src/js/utils.js @@ -9,6 +9,16 @@ export function extname(path = '') { return ext || ''; } +// join a list of path segments with slashes, ensuring no double slashes +export function joinPaths(...parts) { + let str = ''; + for (const part of parts) { + if (!part) continue; + str = !str ? part : `${str.replace(/\/$/, '')}/${part.replace(/^\//, '')}`; + } + return str; +} + // test whether a variable is an object export function isObject(obj) { return Object.prototype.toString.call(obj) === '[object Object]'; diff --git a/web_src/js/utils.test.js b/web_src/js/utils.test.js index 0176f8ec5..859046c87 100644 --- a/web_src/js/utils.test.js +++ b/web_src/js/utils.test.js @@ -1,5 +1,5 @@ import { - basename, extname, isObject, uniq, stripTags, + basename, extname, isObject, uniq, stripTags, joinPaths, } from './utils.js'; test('basename', () => { @@ -15,6 +15,45 @@ test('extname', () => { expect(extname('file.js')).toEqual('.js'); }); +test('joinPaths', () => { + expect(joinPaths('', '')).toEqual(''); + expect(joinPaths('', 'b')).toEqual('b'); + expect(joinPaths('', '/b')).toEqual('/b'); + expect(joinPaths('', '/b/')).toEqual('/b/'); + expect(joinPaths('a', '')).toEqual('a'); + expect(joinPaths('/a', '')).toEqual('/a'); + expect(joinPaths('/a/', '')).toEqual('/a/'); + expect(joinPaths('a', 'b')).toEqual('a/b'); + expect(joinPaths('a', '/b')).toEqual('a/b'); + expect(joinPaths('/a', '/b')).toEqual('/a/b'); + expect(joinPaths('/a', '/b')).toEqual('/a/b'); + expect(joinPaths('/a/', '/b')).toEqual('/a/b'); + expect(joinPaths('/a', '/b/')).toEqual('/a/b/'); + expect(joinPaths('/a/', '/b/')).toEqual('/a/b/'); + + expect(joinPaths('', '', '')).toEqual(''); + expect(joinPaths('', 'b', '')).toEqual('b'); + expect(joinPaths('', 'b', 'c')).toEqual('b/c'); + expect(joinPaths('', '', 'c')).toEqual('c'); + expect(joinPaths('', '/b', '/c')).toEqual('/b/c'); + expect(joinPaths('/a', '', '/c')).toEqual('/a/c'); + expect(joinPaths('/a', '/b', '')).toEqual('/a/b'); + + expect(joinPaths('', '/')).toEqual('/'); + expect(joinPaths('a', '/')).toEqual('a/'); + expect(joinPaths('', '/', '/')).toEqual('/'); + expect(joinPaths('/', '/')).toEqual('/'); + expect(joinPaths('/', '')).toEqual('/'); + expect(joinPaths('/', 'b')).toEqual('/b'); + expect(joinPaths('/', 'b/')).toEqual('/b/'); + expect(joinPaths('/', '', '/')).toEqual('/'); + expect(joinPaths('/', 'b', '/')).toEqual('/b/'); + expect(joinPaths('/', 'b/', '/')).toEqual('/b/'); + expect(joinPaths('a', '/', '/')).toEqual('a/'); + expect(joinPaths('/', '/', 'c')).toEqual('/c'); + expect(joinPaths('/', '/', 'c/')).toEqual('/c/'); +}); + test('isObject', () => { expect(isObject({})).toBeTrue(); expect(isObject([])).toBeFalse();