Skip to content

Commit d19ef65

Browse files
gkalpakatscott
authored andcommitted
fix(service-worker): correctly handle relative base href (#37922)
In some cases, it is useful to use a relative base href in the app (e.g. when an app has to be accessible on different URLs, such as on an intranet and the internet - see #25055 for a related discussion). Previously, the Angular ServiceWorker was not able to handle relative base hrefs (for example when building the with `--base-href=./`). This commit fixes this by normalizing all URLs from the ServiceWorker configuration wrt the ServiceWorker's scope. Fixes #25055 PR Close #37922
1 parent 667aba7 commit d19ef65

File tree

5 files changed

+251
-23
lines changed

5 files changed

+251
-23
lines changed

packages/service-worker/config/src/generator.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,10 @@ function matches(file: string, patterns: {positive: boolean, regex: RegExp}[]):
131131

132132
function urlToRegex(url: string, baseHref: string, literalQuestionMark?: boolean): string {
133133
if (!url.startsWith('/') && url.indexOf('://') === -1) {
134-
url = joinUrls(baseHref, url);
134+
// Prefix relative URLs with `baseHref`.
135+
// Strip a leading `.` from a relative `baseHref` (e.g. `./foo/`), since it would result in an
136+
// incorrect regex (matching a literal `.`).
137+
url = joinUrls(baseHref.replace(/^\.(?=\/)/, ''), url);
135138
}
136139

137140
return globToRegex(url, literalQuestionMark);

packages/service-worker/config/test/generator_spec.ts

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {Generator} from '../src/generator';
9+
import {Generator, processNavigationUrls} from '../src/generator';
1010
import {AssetGroup} from '../src/in';
1111
import {MockFilesystem} from '../testing/mock';
1212

@@ -255,4 +255,56 @@ describe('Generator', () => {
255255
},
256256
});
257257
});
258+
259+
describe('processNavigationUrls()', () => {
260+
const customNavigationUrls = [
261+
'https://host/positive/external/**',
262+
'!https://host/negative/external/**',
263+
'/positive/absolute/**',
264+
'!/negative/absolute/**',
265+
'positive/relative/**',
266+
'!negative/relative/**',
267+
];
268+
269+
it('uses the default `navigationUrls` if not provided', () => {
270+
expect(processNavigationUrls('/')).toEqual([
271+
{positive: true, regex: '^\\/.*$'},
272+
{positive: false, regex: '^\\/(?:.+\\/)?[^/]*\\.[^/]*$'},
273+
{positive: false, regex: '^\\/(?:.+\\/)?[^/]*__[^/]*$'},
274+
{positive: false, regex: '^\\/(?:.+\\/)?[^/]*__[^/]*\\/.*$'},
275+
]);
276+
});
277+
278+
it('prepends `baseHref` to relative URL patterns only', () => {
279+
expect(processNavigationUrls('/base/href/', customNavigationUrls)).toEqual([
280+
{positive: true, regex: '^https:\\/\\/host\\/positive\\/external\\/.*$'},
281+
{positive: false, regex: '^https:\\/\\/host\\/negative\\/external\\/.*$'},
282+
{positive: true, regex: '^\\/positive\\/absolute\\/.*$'},
283+
{positive: false, regex: '^\\/negative\\/absolute\\/.*$'},
284+
{positive: true, regex: '^\\/base\\/href\\/positive\\/relative\\/.*$'},
285+
{positive: false, regex: '^\\/base\\/href\\/negative\\/relative\\/.*$'},
286+
]);
287+
});
288+
289+
it('strips a leading single `.` from a relative `baseHref`', () => {
290+
expect(processNavigationUrls('./relative/base/href/', customNavigationUrls)).toEqual([
291+
{positive: true, regex: '^https:\\/\\/host\\/positive\\/external\\/.*$'},
292+
{positive: false, regex: '^https:\\/\\/host\\/negative\\/external\\/.*$'},
293+
{positive: true, regex: '^\\/positive\\/absolute\\/.*$'},
294+
{positive: false, regex: '^\\/negative\\/absolute\\/.*$'},
295+
{positive: true, regex: '^\\/relative\\/base\\/href\\/positive\\/relative\\/.*$'},
296+
{positive: false, regex: '^\\/relative\\/base\\/href\\/negative\\/relative\\/.*$'},
297+
]);
298+
299+
// We can't correctly handle double dots in `baseHref`, so leave them as literal matches.
300+
expect(processNavigationUrls('../double/dots/', customNavigationUrls)).toEqual([
301+
{positive: true, regex: '^https:\\/\\/host\\/positive\\/external\\/.*$'},
302+
{positive: false, regex: '^https:\\/\\/host\\/negative\\/external\\/.*$'},
303+
{positive: true, regex: '^\\/positive\\/absolute\\/.*$'},
304+
{positive: false, regex: '^\\/negative\\/absolute\\/.*$'},
305+
{positive: true, regex: '^\\.\\.\\/double\\/dots\\/positive\\/relative\\/.*$'},
306+
{positive: false, regex: '^\\.\\.\\/double\\/dots\\/negative\\/relative\\/.*$'},
307+
]);
308+
});
309+
});
258310
});

packages/service-worker/worker/src/app-version.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,12 @@ export class AppVersion implements UpdateSource {
5252
*/
5353
private navigationUrls: {include: RegExp[], exclude: RegExp[]};
5454

55+
/**
56+
* The normalized URL to the file that serves as the index page to satisfy navigation requests.
57+
* Usually this is `/index.html`.
58+
*/
59+
private indexUrl = this.adapter.normalizeUrl(this.manifest.index);
60+
5561
/**
5662
* Tracks whether the manifest has encountered any inconsistencies.
5763
*/
@@ -67,7 +73,7 @@ export class AppVersion implements UpdateSource {
6773
readonly manifestHash: string) {
6874
// The hashTable within the manifest is an Object - convert it to a Map for easier lookups.
6975
Object.keys(this.manifest.hashTable).forEach(url => {
70-
this.hashTable.set(url, this.manifest.hashTable[url]);
76+
this.hashTable.set(adapter.normalizeUrl(url), this.manifest.hashTable[url]);
7177
});
7278

7379
// Process each `AssetGroup` declared in the manifest. Each declared group gets an `AssetGroup`
@@ -179,10 +185,10 @@ export class AppVersion implements UpdateSource {
179185

180186
// Next, check if this is a navigation request for a route. Detect circular
181187
// navigations by checking if the request URL is the same as the index URL.
182-
if (req.url !== this.manifest.index && this.isNavigationRequest(req)) {
188+
if (this.adapter.normalizeUrl(req.url) !== this.indexUrl && this.isNavigationRequest(req)) {
183189
// This was a navigation request. Re-enter `handleFetch` with a request for
184190
// the URL.
185-
return this.handleFetch(this.adapter.newRequest(this.manifest.index), context);
191+
return this.handleFetch(this.adapter.newRequest(this.indexUrl), context);
186192
}
187193

188194
return null;

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

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,11 @@ export abstract class AssetGroup {
2626
*/
2727
private inFlightRequests = new Map<string, Promise<Response>>();
2828

29+
/**
30+
* Normalized resource URLs.
31+
*/
32+
protected urls: string[] = [];
33+
2934
/**
3035
* Regular expression patterns.
3136
*/
@@ -52,29 +57,33 @@ export abstract class AssetGroup {
5257
protected idle: IdleScheduler, protected config: AssetGroupConfig,
5358
protected hashes: Map<string, string>, protected db: Database, protected prefix: string) {
5459
this.name = config.name;
60+
61+
// Normalize the config's URLs to take the ServiceWorker's scope into account.
62+
this.urls = config.urls.map(url => adapter.normalizeUrl(url));
63+
5564
// Patterns in the config are regular expressions disguised as strings. Breathe life into them.
56-
this.patterns = this.config.patterns.map(pattern => new RegExp(pattern));
65+
this.patterns = config.patterns.map(pattern => new RegExp(pattern));
5766

5867
// This is the primary cache, which holds all of the cached requests for this group. If a
5968
// resource
6069
// isn't in this cache, it hasn't been fetched yet.
61-
this.cache = this.scope.caches.open(`${this.prefix}:${this.config.name}:cache`);
70+
this.cache = scope.caches.open(`${this.prefix}:${config.name}:cache`);
6271

6372
// This is the metadata table, which holds specific information for each cached URL, such as
6473
// the timestamp of when it was added to the cache.
65-
this.metadata =
66-
this.db.open(`${this.prefix}:${this.config.name}:meta`, this.config.cacheQueryOptions);
74+
this.metadata = this.db.open(`${this.prefix}:${config.name}:meta`, config.cacheQueryOptions);
6775
}
6876

6977
async cacheStatus(url: string): Promise<UpdateCacheStatus> {
7078
const cache = await this.cache;
7179
const meta = await this.metadata;
72-
const res = await cache.match(this.adapter.newRequest(url), this.config.cacheQueryOptions);
80+
const req = this.adapter.newRequest(url);
81+
const res = await cache.match(req, this.config.cacheQueryOptions);
7382
if (res === undefined) {
7483
return UpdateCacheStatus.NOT_CACHED;
7584
}
7685
try {
77-
const data = await meta.read<UrlMetadata>(url);
86+
const data = await meta.read<UrlMetadata>(req.url);
7887
if (!data.used) {
7988
return UpdateCacheStatus.CACHED_BUT_UNUSED;
8089
}
@@ -105,7 +114,7 @@ export abstract class AssetGroup {
105114
// Either the request matches one of the known resource URLs, one of the patterns for
106115
// dynamically matched URLs, or neither. Determine which is the case for this request in
107116
// order to decide how to handle it.
108-
if (this.config.urls.indexOf(url) !== -1 || this.patterns.some(pattern => pattern.test(url))) {
117+
if (this.urls.indexOf(url) !== -1 || this.patterns.some(pattern => pattern.test(url))) {
109118
// This URL matches a known resource. Either it's been cached already or it's missing, in
110119
// which case it needs to be loaded from the network.
111120

@@ -235,7 +244,8 @@ export abstract class AssetGroup {
235244
const metaTable = await this.metadata;
236245

237246
// Lookup the response in the cache.
238-
const response = await cache.match(this.adapter.newRequest(url), this.config.cacheQueryOptions);
247+
const request = this.adapter.newRequest(url);
248+
const response = await cache.match(request, this.config.cacheQueryOptions);
239249
if (response === undefined) {
240250
// It's not found, return null.
241251
return null;
@@ -244,7 +254,7 @@ export abstract class AssetGroup {
244254
// Next, lookup the cached metadata.
245255
let metadata: UrlMetadata|undefined = undefined;
246256
try {
247-
metadata = await metaTable.read<UrlMetadata>(url);
257+
metadata = await metaTable.read<UrlMetadata>(request.url);
248258
} catch {
249259
// Do nothing, not found. This shouldn't happen, but it can be handled.
250260
}
@@ -258,9 +268,10 @@ export abstract class AssetGroup {
258268
*/
259269
async unhashedResources(): Promise<string[]> {
260270
const cache = await this.cache;
261-
// Start with the set of all cached URLs.
271+
// Start with the set of all cached requests.
262272
return (await cache.keys())
263-
.map(request => request.url)
273+
// Normalize their URLs.
274+
.map(request => this.adapter.normalizeUrl(request.url))
264275
// Exclude the URLs which have hashes.
265276
.filter(url => !this.hashes.has(url));
266277
}
@@ -307,7 +318,7 @@ export abstract class AssetGroup {
307318

308319
// If the request is not hashed, update its metadata, especially the timestamp. This is
309320
// needed for future determination of whether this cached response is stale or not.
310-
if (!this.hashes.has(req.url)) {
321+
if (!this.hashes.has(this.adapter.normalizeUrl(req.url))) {
311322
// Metadata is tracked for requests that are unhashed.
312323
const meta: UrlMetadata = {ts: this.adapter.time, used};
313324
const metaTable = await this.metadata;
@@ -492,7 +503,7 @@ export class PrefetchAssetGroup extends AssetGroup {
492503
// Cache all known resources serially. As this reduce proceeds, each Promise waits
493504
// on the last before starting the fetch/cache operation for the next request. Any
494505
// errors cause fall-through to the final Promise which rejects.
495-
await this.config.urls.reduce(async (previous: Promise<void>, url: string) => {
506+
await this.urls.reduce(async (previous: Promise<void>, url: string) => {
496507
// Wait on all previous operations to complete.
497508
await previous;
498509

@@ -527,8 +538,8 @@ export class PrefetchAssetGroup extends AssetGroup {
527538
// First, narrow down the set of resources to those which are handled by this group.
528539
// Either it's a known URL, or it matches a given pattern.
529540
.filter(
530-
url => this.config.urls.indexOf(url) !== -1 ||
531-
this.patterns.some(pattern => pattern.test(url)))
541+
url =>
542+
this.urls.indexOf(url) !== -1 || this.patterns.some(pattern => pattern.test(url)))
532543
// Finally, process each resource in turn.
533544
.reduce(async (previous, url) => {
534545
await previous;
@@ -552,7 +563,7 @@ export class PrefetchAssetGroup extends AssetGroup {
552563
// Write it into the cache. It may already be expired, but it can still serve
553564
// traffic until it's updated (stale-while-revalidate approach).
554565
await cache.put(req, res.response);
555-
await metaTable.write(url, {...res.metadata, used: false} as UrlMetadata);
566+
await metaTable.write(req.url, {...res.metadata, used: false} as UrlMetadata);
556567
}, Promise.resolve());
557568
}
558569
}
@@ -570,7 +581,7 @@ export class LazyAssetGroup extends AssetGroup {
570581
const cache = await this.cache;
571582

572583
// Loop through the listed resources, caching any which are available.
573-
await this.config.urls.reduce(async (previous: Promise<void>, url: string) => {
584+
await this.urls.reduce(async (previous: Promise<void>, url: string) => {
574585
// Wait on all previous operations to complete.
575586
await previous;
576587

0 commit comments

Comments
 (0)