Skip to content

Commit

Permalink
fix(docs-infra): correctly serve index.html with a query string (#4…
Browse files Browse the repository at this point in the history
…2547)

Previously, due to a bug in Firebase hosting, requests to
`/index.html?<query>` would lead to an infinite redirect and eventually
a failure. This affected, for example, cache-busting requests from the
ServiceWorker, which look like: `/index.html?ngsw-cache-bust=...`
For more details see
#42518 (comment)

This commit temporarily works around the bug by explicitly redirecting
`/index.html?<query>` to `/?<query>`.

Fixes #42518

PR Close #42547
  • Loading branch information
gkalpak authored and alxhub committed Jun 14, 2021
1 parent 828fde6 commit 56a0582
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 6 deletions.
4 changes: 4 additions & 0 deletions aio/firebase.json
Expand Up @@ -124,6 +124,10 @@
{"type": 301, "source": "/testing", "destination": "/guide/testing"},
{"type": 301, "source": "/testing/**", "destination": "/guide/testing"},

// Work around Firebase hosting bug with `/index.html?<query>` leading to infinite redirects.
// See https://github.com/angular/angular/issues/42518#issuecomment-858545483 for details.
{"type": 301, "source": "/index.html:query*", "destination": "/:query*"},

// Strip off the `.html` extension, because Firebase will not do this automatically any more
// (unless the new URL points to an existing file, which is not necessarily the case here).
{"type": 301, "source": "/:somePath*/:file.html", "destination": "/:somePath*/:file"},
Expand Down
1 change: 1 addition & 0 deletions aio/tests/deployment/shared/URLS_TO_REDIRECT.txt
Expand Up @@ -188,6 +188,7 @@
/guide/updating-to-version-10 --> https://v10.angular.io/guide/updating-to-version-10
/guide/updating-to-version-11 --> https://v11.angular.io/guide/updating-to-version-11
/guide/webpack --> https://v5.angular.io/guide/webpack
/index.html?foo=bar --> /?foo=bar
/news --> https://blog.angular.io/
/news.html --> https://blog.angular.io/
/start/data --> /start/start-data
Expand Down
44 changes: 44 additions & 0 deletions aio/tools/firebase-test-utils/FirebaseRedirectSource.spec.ts
Expand Up @@ -137,6 +137,21 @@ describe('FirebaseRedirectSource', () => {
});
});

it('should capture a named param not preceded by a slash', () => {
testGlobMatch('/a/b:x', {
named: ['x']
}, {
'/a/bc': {x: 'c'},
'/a/bcd': {x: 'cd'},
'/a/b-c': {x: '-c'},
'/a': undefined,
'/a/': undefined,
'/a/b/': undefined,
'/a/cd': undefined,
'/a/b/c': undefined,
});
});

it('should capture multiple named params', () => {
testGlobMatch('/a/:b/:c', {
named: ['b', 'c']
Expand Down Expand Up @@ -187,6 +202,35 @@ describe('FirebaseRedirectSource', () => {
});
});

it('should capture a rest param not preceded by a slash', () => {
testGlobMatch('/a:bc*', {
rest: ['bc']
}, {
'/ab': {bc: 'b'},
'/a/b': {bc: '/b'},
'/a/bcd': {bc: '/bcd'},
'/a/b/c': {bc: '/b/c'},
'/a//': {bc: '//'},
'/ab/c': {bc: 'b/c'},
'/ab/c/': {bc: 'b/c/'},
'/a': {bc: undefined},
'/bc': undefined,
});

testGlobMatch('/a/b:c*', {
rest: ['c']
}, {
'/a/bc': {c: 'c'},
'/a/bcd': {c: 'cd'},
'/a/b/': {c: '/'},
'/a/b/c/': {c: '/c/'},
'/a/ba/c': {c: 'a/c'},
'/a/ba/c/': {c: 'a/c/'},
'/a/b': {c: undefined},
'/a/': undefined,
});
});

it('should capture a rest param mixed with a named param', () => {
testGlobMatch('/:abc/:rest*', {
named: ['abc'],
Expand Down
12 changes: 6 additions & 6 deletions aio/tools/firebase-test-utils/FirebaseRedirectSource.ts
Expand Up @@ -24,8 +24,8 @@ export class FirebaseRedirectSource {
const star = /\*/g;
const doubleStar = /(^|\/)\*\*($|\/)/g; // e.g. a/**/b or **/b or a/** but not a**b
const modifiedPatterns = /(.)\(([^)]+)\)/g; // e.g. `@(a|b)
const restParam = /\/:([A-Za-z]+)\*/g; // e.g. `:rest*`
const namedParam = /\/:([A-Za-z]+)/g; // e.g. `:api`
const restParam = /(\/?):([A-Za-z]+)\*/g; // e.g. `:rest*`
const namedParam = /(\/?):([A-Za-z]+)/g; // e.g. `:api`
const possiblyEmptyInitialSegments = /^\.🐷\//g; // e.g. `**/a` can also match `a`
const possiblyEmptySegments = /\/\.🐷\//g; // e.g. `a/**/b` can also match `a/b`
const willBeStar = /🐷/g; // e.g. `a**b` not matched by previous rule
Expand All @@ -35,12 +35,12 @@ export class FirebaseRedirectSource {
const pattern = glob
.replace(dot, '\\.')
.replace(modifiedPatterns, replaceModifiedPattern)
.replace(restParam, (_, param) => {
.replace(restParam, (_, leadingSlash, groupName) => {
// capture the rest of the string
restNamedGroups.push(param);
return `(?:/(?<${param}>.🐷))?`;
restNamedGroups.push(groupName);
return `(?:${leadingSlash}(?<${groupName}>.🐷))?`;
})
.replace(namedParam, `/(?<$1>[^/]+)`)
.replace(namedParam, `$1(?<$2>[^/]+)`)
.replace(doubleStar, '$1.🐷$2') // use the pig to avoid replacing ** in next rule
.replace(star, '[^/]*') // match a single segment
.replace(possiblyEmptyInitialSegments, '(?:.*)')// deal with **/ special cases
Expand Down

0 comments on commit 56a0582

Please sign in to comment.