Skip to content

Commit 0fe572e

Browse files
committed
fix(@angular/build): ensure correct URL joining for prerender routes
This commit addresses an issue where prerendering with i18n and a `routesFile` could lead to infinite redirect loops or failure to prerender `index.html`. The previous `urlJoin` utility was replaced with more robust URL manipulation functions (`joinUrlParts`, `addTrailingSlash`, `stripLeadingSlash`) to ensure that paths are correctly constructed, especially when dealing with base hrefs and locale subpaths. This ensures that routes from the `routesFile` are correctly joined with the base href, preventing malformed URLs that cause the redirection issues. Closes angular#31877
1 parent 439c463 commit 0fe572e

File tree

3 files changed

+120
-24
lines changed

3 files changed

+120
-24
lines changed

packages/angular/build/src/builders/application/options.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import {
2525
loadPostcssConfiguration,
2626
} from '../../utils/postcss-configuration';
2727
import { getProjectRootPaths, normalizeDirectoryPath } from '../../utils/project-metadata';
28-
import { urlJoin } from '../../utils/url';
28+
import { addTrailingSlash, joinUrlParts } from '../../utils/url';
2929
import {
3030
Schema as ApplicationBuilderOptions,
3131
ExperimentalPlatform,
@@ -681,7 +681,9 @@ export function getLocaleBaseHref(
681681

682682
const baseHrefSuffix = localeData.baseHref ?? localeData.subPath + '/';
683683

684-
return baseHrefSuffix !== '' ? urlJoin(baseHref, baseHrefSuffix) : undefined;
684+
return baseHrefSuffix !== ''
685+
? addTrailingSlash(joinUrlParts(baseHref, baseHrefSuffix))
686+
: undefined;
685687
}
686688

687689
/**

packages/angular/build/src/utils/server-rendering/prerender.ts

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { BuildOutputFile, BuildOutputFileType } from '../../tools/esbuild/bundle
1414
import { BuildOutputAsset } from '../../tools/esbuild/bundler-execution-result';
1515
import { assertIsError } from '../error';
1616
import { toPosixPath } from '../path';
17-
import { urlJoin } from '../url';
17+
import { addLeadingSlash, addTrailingSlash, joinUrlParts, stripLeadingSlash } from '../url';
1818
import { WorkerPool } from '../worker-pool';
1919
import { IMPORT_EXEC_ARGV } from './esm-in-memory-loader/utils';
2020
import { SERVER_APP_MANIFEST_FILENAME } from './manifest';
@@ -240,7 +240,7 @@ async function renderPages(
240240
? addLeadingSlash(route.slice(baseHrefPathnameWithLeadingSlash.length))
241241
: route;
242242

243-
const outPath = posix.join(removeLeadingSlash(routeWithoutBaseHref), 'index.html');
243+
const outPath = stripLeadingSlash(posix.join(routeWithoutBaseHref, 'index.html'));
244244

245245
if (typeof redirectTo === 'string') {
246246
output[outPath] = { content: generateRedirectStaticPage(redirectTo), appShellRoute: false };
@@ -298,7 +298,7 @@ async function getAllRoutes(
298298
let appShellRoute: string | undefined;
299299

300300
if (appShellOptions) {
301-
appShellRoute = urlJoin(baseHref, appShellOptions.route);
301+
appShellRoute = joinUrlParts(baseHref, appShellOptions.route);
302302

303303
routes.push({
304304
renderMode: RouteRenderMode.Prerender,
@@ -311,7 +311,7 @@ async function getAllRoutes(
311311
for (const route of routesFromFile) {
312312
routes.push({
313313
renderMode: RouteRenderMode.Prerender,
314-
route: urlJoin(baseHref, route.trim()),
314+
route: joinUrlParts(baseHref, route.trim()),
315315
});
316316
}
317317
}
@@ -369,15 +369,3 @@ async function getAllRoutes(
369369
void renderWorker.destroy();
370370
}
371371
}
372-
373-
function addLeadingSlash(value: string): string {
374-
return value[0] === '/' ? value : '/' + value;
375-
}
376-
377-
function addTrailingSlash(url: string): string {
378-
return url[url.length - 1] === '/' ? url : `${url}/`;
379-
}
380-
381-
function removeLeadingSlash(value: string): string {
382-
return value[0] === '/' ? value.slice(1) : value;
383-
}

packages/angular/build/src/utils/url.ts

Lines changed: 112 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,117 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
export function urlJoin(...parts: string[]): string {
10-
const [p, ...rest] = parts;
9+
/**
10+
* Removes the trailing slash from a URL if it exists.
11+
*
12+
* @param url - The URL string from which to remove the trailing slash.
13+
* @returns The URL string without a trailing slash.
14+
*
15+
* @example
16+
* ```js
17+
* stripTrailingSlash('path/'); // 'path'
18+
* stripTrailingSlash('/path'); // '/path'
19+
* stripTrailingSlash('/'); // '/'
20+
* stripTrailingSlash(''); // ''
21+
* ```
22+
*/
23+
export function stripTrailingSlash(url: string): string {
24+
// Check if the last character of the URL is a slash
25+
return url.length > 1 && url[url.length - 1] === '/' ? url.slice(0, -1) : url;
26+
}
27+
28+
/**
29+
* Removes the leading slash from a URL if it exists.
30+
*
31+
* @param url - The URL string from which to remove the leading slash.
32+
* @returns The URL string without a leading slash.
33+
*
34+
* @example
35+
* ```js
36+
* stripLeadingSlash('/path'); // 'path'
37+
* stripLeadingSlash('/path/'); // 'path/'
38+
* stripLeadingSlash('/'); // '/'
39+
* stripLeadingSlash(''); // ''
40+
* ```
41+
*/
42+
export function stripLeadingSlash(url: string): string {
43+
// Check if the first character of the URL is a slash
44+
return url.length > 1 && url[0] === '/' ? url.slice(1) : url;
45+
}
46+
47+
/**
48+
* Adds a leading slash to a URL if it does not already have one.
49+
*
50+
* @param url - The URL string to which the leading slash will be added.
51+
* @returns The URL string with a leading slash.
52+
*
53+
* @example
54+
* ```js
55+
* addLeadingSlash('path'); // '/path'
56+
* addLeadingSlash('/path'); // '/path'
57+
* ```
58+
*/
59+
export function addLeadingSlash(url: string): string {
60+
// Check if the URL already starts with a slash
61+
return url[0] === '/' ? url : `/${url}`;
62+
}
63+
64+
/**
65+
* Adds a trailing slash to a URL if it does not already have one.
66+
*
67+
* @param url - The URL string to which the trailing slash will be added.
68+
* @returns The URL string with a trailing slash.
69+
*
70+
* @example
71+
* ```js
72+
* addTrailingSlash('path'); // 'path/'
73+
* addTrailingSlash('path/'); // 'path/'
74+
* ```
75+
*/
76+
export function addTrailingSlash(url: string): string {
77+
// Check if the URL already end with a slash
78+
return url[url.length - 1] === '/' ? url : `${url}/`;
79+
}
80+
81+
/**
82+
* Joins URL parts into a single URL string.
83+
*
84+
* This function takes multiple URL segments, normalizes them by removing leading
85+
* and trailing slashes where appropriate, and then joins them into a single URL.
86+
*
87+
* @param parts - The parts of the URL to join. Each part can be a string with or without slashes.
88+
* @returns The joined URL string, with normalized slashes.
89+
*
90+
* @example
91+
* ```js
92+
* joinUrlParts('path/', '/to/resource'); // '/path/to/resource'
93+
* joinUrlParts('/path/', 'to/resource'); // '/path/to/resource'
94+
* joinUrlParts('http://localhost/path/', 'to/resource'); // 'http://localhost/path/to/resource'
95+
* joinUrlParts('', ''); // '/'
96+
* ```
97+
*/
98+
export function joinUrlParts(...parts: string[]): string {
99+
const normalizeParts: string[] = [];
100+
for (const part of parts) {
101+
if (part === '') {
102+
// Skip any empty parts
103+
continue;
104+
}
105+
106+
let normalizedPart = part;
107+
if (part[0] === '/') {
108+
normalizedPart = normalizedPart.slice(1);
109+
}
110+
if (part[part.length - 1] === '/') {
111+
normalizedPart = normalizedPart.slice(0, -1);
112+
}
113+
if (normalizedPart !== '') {
114+
normalizeParts.push(normalizedPart);
115+
}
116+
}
117+
118+
const protocolMatch = normalizeParts.length && /^https?:\/\//.test(normalizeParts[0]);
119+
const joinedParts = normalizeParts.join('/');
11120

12-
// Remove trailing slash from first part
13-
// Join all parts with `/`
14-
// Dedupe double slashes from path names
15-
return p.replace(/\/$/, '') + ('/' + rest.join('/')).replace(/\/\/+/g, '/');
121+
return protocolMatch ? joinedParts : addLeadingSlash(joinedParts);
16122
}

0 commit comments

Comments
 (0)