From 56a0582d79dbb4a8922951bb881b1887464f4527 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Thu, 10 Jun 2021 15:06:51 +0300 Subject: [PATCH] fix(docs-infra): correctly serve `index.html` with a query string (#42547) Previously, due to a bug in Firebase hosting, requests to `/index.html?` 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 https://github.com/angular/angular/issues/42518#issuecomment-858545483 This commit temporarily works around the bug by explicitly redirecting `/index.html?` to `/?`. Fixes #42518 PR Close #42547 --- aio/firebase.json | 4 ++ .../deployment/shared/URLS_TO_REDIRECT.txt | 1 + .../FirebaseRedirectSource.spec.ts | 44 +++++++++++++++++++ .../FirebaseRedirectSource.ts | 12 ++--- 4 files changed, 55 insertions(+), 6 deletions(-) diff --git a/aio/firebase.json b/aio/firebase.json index 8a88687ec5067..5e3775a2e90c0 100644 --- a/aio/firebase.json +++ b/aio/firebase.json @@ -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?` 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"}, diff --git a/aio/tests/deployment/shared/URLS_TO_REDIRECT.txt b/aio/tests/deployment/shared/URLS_TO_REDIRECT.txt index 331976050b394..de503ed0bf73a 100644 --- a/aio/tests/deployment/shared/URLS_TO_REDIRECT.txt +++ b/aio/tests/deployment/shared/URLS_TO_REDIRECT.txt @@ -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 diff --git a/aio/tools/firebase-test-utils/FirebaseRedirectSource.spec.ts b/aio/tools/firebase-test-utils/FirebaseRedirectSource.spec.ts index 1c0fe71ec60a8..eeb2d6375a55d 100644 --- a/aio/tools/firebase-test-utils/FirebaseRedirectSource.spec.ts +++ b/aio/tools/firebase-test-utils/FirebaseRedirectSource.spec.ts @@ -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'] @@ -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'], diff --git a/aio/tools/firebase-test-utils/FirebaseRedirectSource.ts b/aio/tools/firebase-test-utils/FirebaseRedirectSource.ts index 2eff990929a01..fe57927115039 100644 --- a/aio/tools/firebase-test-utils/FirebaseRedirectSource.ts +++ b/aio/tools/firebase-test-utils/FirebaseRedirectSource.ts @@ -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 @@ -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