Skip to content

Commit c8cd90e

Browse files
committed
fix(@angular/ssr): handle nested redirects not explicitly defined in router config
This commit ensures proper handling of nested redirects that are implicitly structured but not explicitly defined in the router configuration. Closes #28903 (cherry picked from commit 34574b2)
1 parent 8cd6aa9 commit c8cd90e

File tree

7 files changed

+277
-42
lines changed

7 files changed

+277
-42
lines changed

packages/angular/ssr/src/app.ts

+15-16
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,12 @@ import { sha256 } from './utils/crypto';
2424
import { InlineCriticalCssProcessor } from './utils/inline-critical-css';
2525
import { LRUCache } from './utils/lru-cache';
2626
import { AngularBootstrap, renderAngular } from './utils/ng';
27-
import { joinUrlParts, stripIndexHtmlFromURL, stripLeadingSlash } from './utils/url';
27+
import {
28+
buildPathWithParams,
29+
joinUrlParts,
30+
stripIndexHtmlFromURL,
31+
stripLeadingSlash,
32+
} from './utils/url';
2833

2934
/**
3035
* Maximum number of critical CSS entries the cache can store.
@@ -160,11 +165,14 @@ export class AngularServerApp {
160165

161166
const { redirectTo, status, renderMode } = matchedRoute;
162167
if (redirectTo !== undefined) {
163-
// Note: The status code is validated during route extraction.
164-
// 302 Found is used by default for redirections
165-
// See: https://developer.mozilla.org/en-US/docs/Web/API/Response/redirect_static#status
166-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
167-
return Response.redirect(new URL(redirectTo, url), (status as any) ?? 302);
168+
return Response.redirect(
169+
new URL(buildPathWithParams(redirectTo, url.pathname), url),
170+
// Note: The status code is validated during route extraction.
171+
// 302 Found is used by default for redirections
172+
// See: https://developer.mozilla.org/en-US/docs/Web/API/Response/redirect_static#status
173+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
174+
(status as any) ?? 302,
175+
);
168176
}
169177

170178
if (renderMode === RenderMode.Prerender) {
@@ -241,17 +249,8 @@ export class AngularServerApp {
241249
matchedRoute: RouteTreeNodeMetadata,
242250
requestContext?: unknown,
243251
): Promise<Response | null> {
244-
const { redirectTo, status } = matchedRoute;
245-
246-
if (redirectTo !== undefined) {
247-
// Note: The status code is validated during route extraction.
248-
// 302 Found is used by default for redirections
249-
// See: https://developer.mozilla.org/en-US/docs/Web/API/Response/redirect_static#status
250-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
251-
return Response.redirect(new URL(redirectTo, new URL(request.url)), (status as any) ?? 302);
252-
}
252+
const { renderMode, headers, status } = matchedRoute;
253253

254-
const { renderMode, headers } = matchedRoute;
255254
if (!this.allowStaticRouteRender && renderMode === RenderMode.Prerender) {
256255
return null;
257256
}

packages/angular/ssr/src/routes/ng-routes.ts

+44-18
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { ServerAssets } from '../assets';
2121
import { Console } from '../console';
2222
import { AngularAppManifest, getAngularAppManifest } from '../manifest';
2323
import { AngularBootstrap, isNgModule } from '../utils/ng';
24-
import { joinUrlParts, stripLeadingSlash } from '../utils/url';
24+
import { addTrailingSlash, joinUrlParts, stripLeadingSlash } from '../utils/url';
2525
import {
2626
PrerenderFallback,
2727
RenderMode,
@@ -146,31 +146,36 @@ async function* traverseRoutesConfig(options: {
146146
const metadata: ServerConfigRouteTreeNodeMetadata = {
147147
renderMode: RenderMode.Prerender,
148148
...matchedMetaData,
149-
route: currentRoutePath,
149+
// Match Angular router behavior
150+
// ['one', 'two', ''] -> 'one/two/'
151+
// ['one', 'two', 'three'] -> 'one/two/three'
152+
route: path === '' ? addTrailingSlash(currentRoutePath) : currentRoutePath,
150153
};
151154

152155
delete metadata.presentInClientRouter;
153156

154-
// Handle redirects
155-
if (typeof redirectTo === 'string') {
156-
const redirectToResolved = resolveRedirectTo(currentRoutePath, redirectTo);
157+
if (metadata.renderMode === RenderMode.Prerender) {
158+
// Handle SSG routes
159+
yield* handleSSGRoute(
160+
typeof redirectTo === 'string' ? redirectTo : undefined,
161+
metadata,
162+
parentInjector,
163+
invokeGetPrerenderParams,
164+
includePrerenderFallbackRoutes,
165+
);
166+
} else if (typeof redirectTo === 'string') {
167+
// Handle redirects
157168
if (metadata.status && !VALID_REDIRECT_RESPONSE_CODES.has(metadata.status)) {
158169
yield {
159170
error:
160171
`The '${metadata.status}' status code is not a valid redirect response code. ` +
161172
`Please use one of the following redirect response codes: ${[...VALID_REDIRECT_RESPONSE_CODES.values()].join(', ')}.`,
162173
};
174+
163175
continue;
164176
}
165-
yield { ...metadata, redirectTo: redirectToResolved };
166-
} else if (metadata.renderMode === RenderMode.Prerender) {
167-
// Handle SSG routes
168-
yield* handleSSGRoute(
169-
metadata,
170-
parentInjector,
171-
invokeGetPrerenderParams,
172-
includePrerenderFallbackRoutes,
173-
);
177+
178+
yield { ...metadata, redirectTo: resolveRedirectTo(metadata.route, redirectTo) };
174179
} else {
175180
yield metadata;
176181
}
@@ -214,13 +219,15 @@ async function* traverseRoutesConfig(options: {
214219
* Handles SSG (Static Site Generation) routes by invoking `getPrerenderParams` and yielding
215220
* all parameterized paths, returning any errors encountered.
216221
*
222+
* @param redirectTo - Optional path to redirect to, if specified.
217223
* @param metadata - The metadata associated with the route tree node.
218224
* @param parentInjector - The dependency injection container for the parent route.
219225
* @param invokeGetPrerenderParams - A flag indicating whether to invoke the `getPrerenderParams` function.
220226
* @param includePrerenderFallbackRoutes - A flag indicating whether to include fallback routes in the result.
221227
* @returns An async iterable iterator that yields route tree node metadata for each SSG path or errors.
222228
*/
223229
async function* handleSSGRoute(
230+
redirectTo: string | undefined,
224231
metadata: ServerConfigRouteTreeNodeMetadata,
225232
parentInjector: Injector,
226233
invokeGetPrerenderParams: boolean,
@@ -239,6 +246,10 @@ async function* handleSSGRoute(
239246
delete meta['getPrerenderParams'];
240247
}
241248

249+
if (redirectTo !== undefined) {
250+
meta.redirectTo = resolveRedirectTo(currentRoutePath, redirectTo);
251+
}
252+
242253
if (!URL_PARAMETER_REGEXP.test(currentRoutePath)) {
243254
// Route has no parameters
244255
yield {
@@ -279,7 +290,14 @@ async function* handleSSGRoute(
279290
return value;
280291
});
281292

282-
yield { ...meta, route: routeWithResolvedParams };
293+
yield {
294+
...meta,
295+
route: routeWithResolvedParams,
296+
redirectTo:
297+
redirectTo === undefined
298+
? undefined
299+
: resolveRedirectTo(routeWithResolvedParams, redirectTo),
300+
};
283301
}
284302
} catch (error) {
285303
yield { error: `${(error as Error).message}` };
@@ -319,7 +337,7 @@ function resolveRedirectTo(routePath: string, redirectTo: string): string {
319337
}
320338

321339
// Resolve relative redirectTo based on the current route path.
322-
const segments = routePath.split('/');
340+
const segments = routePath.replace(URL_PARAMETER_REGEXP, '*').split('/');
323341
segments.pop(); // Remove the last segment to make it relative.
324342

325343
return joinUrlParts(...segments, redirectTo);
@@ -459,7 +477,6 @@ export async function getRoutesFromAngularRouterConfig(
459477
includePrerenderFallbackRoutes,
460478
});
461479

462-
let seenAppShellRoute: string | undefined;
463480
for await (const result of traverseRoutes) {
464481
if ('error' in result) {
465482
errors.push(result.error);
@@ -549,8 +566,17 @@ export async function extractRoutesAndCreateRouteTree(
549566
metadata.redirectTo = joinUrlParts(baseHref, metadata.redirectTo);
550567
}
551568

569+
// Remove undefined fields
570+
// Helps avoid unnecessary test updates
571+
for (const [key, value] of Object.entries(metadata)) {
572+
if (value === undefined) {
573+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
574+
delete (metadata as any)[key];
575+
}
576+
}
577+
552578
const fullRoute = joinUrlParts(baseHref, route);
553-
routeTree.insert(fullRoute, metadata);
579+
routeTree.insert(fullRoute.replace(URL_PARAMETER_REGEXP, '*'), metadata);
554580
}
555581

556582
return {

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

+68
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,23 @@ export function addLeadingSlash(url: string): string {
6161
return url[0] === '/' ? url : `/${url}`;
6262
}
6363

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+
6481
/**
6582
* Joins URL parts into a single URL string.
6683
*
@@ -128,3 +145,54 @@ export function stripIndexHtmlFromURL(url: URL): URL {
128145

129146
return url;
130147
}
148+
149+
/**
150+
* Resolves `*` placeholders in a path template by mapping them to corresponding segments
151+
* from a base path. This is useful for constructing paths dynamically based on a given base path.
152+
*
153+
* The function processes the `toPath` string, replacing each `*` placeholder with
154+
* the corresponding segment from the `fromPath`. If the `toPath` contains no placeholders,
155+
* it is returned as-is. Invalid `toPath` formats (not starting with `/`) will throw an error.
156+
*
157+
* @param toPath - A path template string that may contain `*` placeholders. Each `*` is replaced
158+
* by the corresponding segment from the `fromPath`. Static paths (e.g., `/static/path`) are returned
159+
* directly without placeholder replacement.
160+
* @param fromPath - A base path string, split into segments, that provides values for
161+
* replacing `*` placeholders in the `toPath`.
162+
* @returns A resolved path string with `*` placeholders replaced by segments from the `fromPath`,
163+
* or the `toPath` returned unchanged if it contains no placeholders.
164+
*
165+
* @throws If the `toPath` does not start with a `/`, indicating an invalid path format.
166+
*
167+
* @example
168+
* ```typescript
169+
* // Example with placeholders resolved
170+
* const resolvedPath = buildPathWithParams('/*\/details', '/123/abc');
171+
* console.log(resolvedPath); // Outputs: '/123/details'
172+
*
173+
* // Example with a static path
174+
* const staticPath = buildPathWithParams('/static/path', '/base/unused');
175+
* console.log(staticPath); // Outputs: '/static/path'
176+
* ```
177+
*/
178+
export function buildPathWithParams(toPath: string, fromPath: string): string {
179+
if (toPath[0] !== '/') {
180+
throw new Error(`Invalid toPath: The string must start with a '/'. Received: '${toPath}'`);
181+
}
182+
183+
if (fromPath[0] !== '/') {
184+
throw new Error(`Invalid fromPath: The string must start with a '/'. Received: '${fromPath}'`);
185+
}
186+
187+
if (!toPath.includes('/*')) {
188+
return toPath;
189+
}
190+
191+
const fromPathParts = fromPath.split('/');
192+
const toPathParts = toPath.split('/');
193+
const resolvedParts = toPathParts.map((part, index) =>
194+
toPathParts[index] === '*' ? fromPathParts[index] : part,
195+
);
196+
197+
return joinUrlParts(...resolvedParts);
198+
}

packages/angular/ssr/test/app_spec.ts

+7
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ describe('AngularServerApp', () => {
3838
{ path: 'page-with-status', component: HomeComponent },
3939
{ path: 'redirect', redirectTo: 'home' },
4040
{ path: 'redirect/relative', redirectTo: 'home' },
41+
{ path: 'redirect/:param/relative', redirectTo: 'home' },
4142
{ path: 'redirect/absolute', redirectTo: '/home' },
4243
],
4344
[
@@ -117,6 +118,12 @@ describe('AngularServerApp', () => {
117118
expect(response?.status).toBe(302);
118119
});
119120

121+
it('should correctly handle relative nested redirects with parameter', async () => {
122+
const response = await app.handle(new Request('http://localhost/redirect/param/relative'));
123+
expect(response?.headers.get('location')).toContain('http://localhost/redirect/param/home');
124+
expect(response?.status).toBe(302);
125+
});
126+
120127
it('should correctly handle absolute nested redirects', async () => {
121128
const response = await app.handle(new Request('http://localhost/redirect/absolute'));
122129
expect(response?.headers.get('location')).toContain('http://localhost/home');

0 commit comments

Comments
 (0)