Skip to content

Commit

Permalink
fix(service-worker): correctly handle relative base href (#37922)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@gkalpak @atscott
gkalpak authored and atscott committed Jul 9, 2020
1 parent 324b6f1 commit b186db7
Show file tree
Hide file tree
Showing 5 changed files with 251 additions and 23 deletions.
5 changes: 4 additions & 1 deletion packages/service-worker/config/src/generator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,10 @@ function matches(file: string, patterns: {positive: boolean, regex: RegExp}[]):

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

return globToRegex(url, literalQuestionMark);
Expand Down
54 changes: 53 additions & 1 deletion packages/service-worker/config/test/generator_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Generator} from '../src/generator';
import {Generator, processNavigationUrls} from '../src/generator';
import {AssetGroup} from '../src/in';
import {MockFilesystem} from '../testing/mock';

Expand Down Expand Up @@ -255,4 +255,56 @@ describe('Generator', () => {
},
});
});

describe('processNavigationUrls()', () => {
const customNavigationUrls = [
'https://host/positive/external/**',
'!https://host/negative/external/**',
'/positive/absolute/**',
'!/negative/absolute/**',
'positive/relative/**',
'!negative/relative/**',
];

it('uses the default `navigationUrls` if not provided', () => {
expect(processNavigationUrls('/')).toEqual([
{positive: true, regex: '^\\/.*$'},
{positive: false, regex: '^\\/(?:.+\\/)?[^/]*\\.[^/]*$'},
{positive: false, regex: '^\\/(?:.+\\/)?[^/]*__[^/]*$'},
{positive: false, regex: '^\\/(?:.+\\/)?[^/]*__[^/]*\\/.*$'},
]);
});

it('prepends `baseHref` to relative URL patterns only', () => {
expect(processNavigationUrls('/base/href/', customNavigationUrls)).toEqual([
{positive: true, regex: '^https:\\/\\/host\\/positive\\/external\\/.*$'},
{positive: false, regex: '^https:\\/\\/host\\/negative\\/external\\/.*$'},
{positive: true, regex: '^\\/positive\\/absolute\\/.*$'},
{positive: false, regex: '^\\/negative\\/absolute\\/.*$'},
{positive: true, regex: '^\\/base\\/href\\/positive\\/relative\\/.*$'},
{positive: false, regex: '^\\/base\\/href\\/negative\\/relative\\/.*$'},
]);
});

it('strips a leading single `.` from a relative `baseHref`', () => {
expect(processNavigationUrls('./relative/base/href/', customNavigationUrls)).toEqual([
{positive: true, regex: '^https:\\/\\/host\\/positive\\/external\\/.*$'},
{positive: false, regex: '^https:\\/\\/host\\/negative\\/external\\/.*$'},
{positive: true, regex: '^\\/positive\\/absolute\\/.*$'},
{positive: false, regex: '^\\/negative\\/absolute\\/.*$'},
{positive: true, regex: '^\\/relative\\/base\\/href\\/positive\\/relative\\/.*$'},
{positive: false, regex: '^\\/relative\\/base\\/href\\/negative\\/relative\\/.*$'},
]);

// We can't correctly handle double dots in `baseHref`, so leave them as literal matches.
expect(processNavigationUrls('../double/dots/', customNavigationUrls)).toEqual([
{positive: true, regex: '^https:\\/\\/host\\/positive\\/external\\/.*$'},
{positive: false, regex: '^https:\\/\\/host\\/negative\\/external\\/.*$'},
{positive: true, regex: '^\\/positive\\/absolute\\/.*$'},
{positive: false, regex: '^\\/negative\\/absolute\\/.*$'},
{positive: true, regex: '^\\.\\.\\/double\\/dots\\/positive\\/relative\\/.*$'},
{positive: false, regex: '^\\.\\.\\/double\\/dots\\/negative\\/relative\\/.*$'},
]);
});
});
});
12 changes: 9 additions & 3 deletions packages/service-worker/worker/src/app-version.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ export class AppVersion implements UpdateSource {
*/
private navigationUrls: {include: RegExp[], exclude: RegExp[]};

/**
* The normalized URL to the file that serves as the index page to satisfy navigation requests.
* Usually this is `/index.html`.
*/
private indexUrl = this.adapter.normalizeUrl(this.manifest.index);

/**
* Tracks whether the manifest has encountered any inconsistencies.
*/
Expand All @@ -67,7 +73,7 @@ export class AppVersion implements UpdateSource {
readonly manifestHash: string) {
// The hashTable within the manifest is an Object - convert it to a Map for easier lookups.
Object.keys(this.manifest.hashTable).forEach(url => {
this.hashTable.set(url, this.manifest.hashTable[url]);
this.hashTable.set(adapter.normalizeUrl(url), this.manifest.hashTable[url]);
});

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

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

return null;
Expand Down
45 changes: 28 additions & 17 deletions packages/service-worker/worker/src/assets.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ export abstract class AssetGroup {
*/
private inFlightRequests = new Map<string, Promise<Response>>();

/**
* Normalized resource URLs.
*/
protected urls: string[] = [];

/**
* Regular expression patterns.
*/
Expand All @@ -52,29 +57,33 @@ export abstract class AssetGroup {
protected idle: IdleScheduler, protected config: AssetGroupConfig,
protected hashes: Map<string, string>, protected db: Database, protected prefix: string) {
this.name = config.name;

// Normalize the config's URLs to take the ServiceWorker's scope into account.
this.urls = config.urls.map(url => adapter.normalizeUrl(url));

// Patterns in the config are regular expressions disguised as strings. Breathe life into them.
this.patterns = this.config.patterns.map(pattern => new RegExp(pattern));
this.patterns = config.patterns.map(pattern => new RegExp(pattern));

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

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

async cacheStatus(url: string): Promise<UpdateCacheStatus> {
const cache = await this.cache;
const meta = await this.metadata;
const res = await cache.match(this.adapter.newRequest(url), this.config.cacheQueryOptions);
const req = this.adapter.newRequest(url);
const res = await cache.match(req, this.config.cacheQueryOptions);
if (res === undefined) {
return UpdateCacheStatus.NOT_CACHED;
}
try {
const data = await meta.read<UrlMetadata>(url);
const data = await meta.read<UrlMetadata>(req.url);
if (!data.used) {
return UpdateCacheStatus.CACHED_BUT_UNUSED;
}
Expand Down Expand Up @@ -105,7 +114,7 @@ export abstract class AssetGroup {
// Either the request matches one of the known resource URLs, one of the patterns for
// dynamically matched URLs, or neither. Determine which is the case for this request in
// order to decide how to handle it.
if (this.config.urls.indexOf(url) !== -1 || this.patterns.some(pattern => pattern.test(url))) {
if (this.urls.indexOf(url) !== -1 || this.patterns.some(pattern => pattern.test(url))) {
// This URL matches a known resource. Either it's been cached already or it's missing, in
// which case it needs to be loaded from the network.

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

// Lookup the response in the cache.
const response = await cache.match(this.adapter.newRequest(url), this.config.cacheQueryOptions);
const request = this.adapter.newRequest(url);
const response = await cache.match(request, this.config.cacheQueryOptions);
if (response === undefined) {
// It's not found, return null.
return null;
Expand All @@ -244,7 +254,7 @@ export abstract class AssetGroup {
// Next, lookup the cached metadata.
let metadata: UrlMetadata|undefined = undefined;
try {
metadata = await metaTable.read<UrlMetadata>(url);
metadata = await metaTable.read<UrlMetadata>(request.url);
} catch {
// Do nothing, not found. This shouldn't happen, but it can be handled.
}
Expand All @@ -258,9 +268,10 @@ export abstract class AssetGroup {
*/
async unhashedResources(): Promise<string[]> {
const cache = await this.cache;
// Start with the set of all cached URLs.
// Start with the set of all cached requests.
return (await cache.keys())
.map(request => request.url)
// Normalize their URLs.
.map(request => this.adapter.normalizeUrl(request.url))
// Exclude the URLs which have hashes.
.filter(url => !this.hashes.has(url));
}
Expand Down Expand Up @@ -307,7 +318,7 @@ export abstract class AssetGroup {

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

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

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

Expand Down

0 comments on commit b186db7

Please sign in to comment.