Skip to content

Commit 4df24ec

Browse files
committed
fixup! fix(@angular/ssr): resolve route parameters in redirect targets
1 parent 1f12518 commit 4df24ec

4 files changed

Lines changed: 84 additions & 32 deletions

File tree

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

Lines changed: 61 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ async function* handleRoute(options: {
147147
metadata: ServerConfigRouteTreeNodeMetadata;
148148
currentRoutePath: string;
149149
parentRoute: string;
150+
baseHref: string;
150151
route: Route;
151152
compiler: Compiler;
152153
parentInjector: Injector;
@@ -160,6 +161,7 @@ async function* handleRoute(options: {
160161
metadata,
161162
currentRoutePath,
162163
parentRoute,
164+
baseHref,
163165
route,
164166
compiler,
165167
parentInjector,
@@ -179,8 +181,9 @@ async function* handleRoute(options: {
179181
if (metadata.renderMode === RenderMode.Prerender) {
180182
for await (const result of handleSSGRoute(
181183
serverConfigRouteTree,
182-
typeof redirectTo === 'string' ? redirectTo : undefined,
184+
route,
183185
parentRoute,
186+
baseHref,
184187
metadata,
185188
parentInjector,
186189
invokeGetPrerenderParams,
@@ -200,9 +203,18 @@ async function* handleRoute(options: {
200203
`Please use one of the following redirect response codes: ${[...VALID_REDIRECT_RESPONSE_CODES.values()].join(', ')}.`,
201204
};
202205
} else if (typeof redirectTo === 'string') {
206+
// Reference the route parameters by their position in the URL (see `buildPathWithParams`).
207+
const routeSegments = joinUrlParts(baseHref, currentRoutePath).split('/');
203208
parentRoutes.set(metadata.route, {
204209
...metadata,
205-
redirectTo: resolveRedirectTo(parentRoute, redirectTo),
210+
redirectTo: resolveRedirectTo(
211+
parentRoute,
212+
replaceRedirectParams(
213+
route,
214+
redirectTo,
215+
(parameter) => `*${routeSegments.lastIndexOf(parameter)}`,
216+
),
217+
),
206218
relativeRedirect: redirectTo[0] !== '/' ? true : undefined,
207219
});
208220
} else {
@@ -290,6 +302,7 @@ async function* traverseRoutesConfig(options: {
290302
compiler: Compiler;
291303
parentInjector: Injector;
292304
parentRoute: string;
305+
baseHref: string;
293306
serverConfigRouteTree?: RouteTree<ServerConfigRouteTreeAdditionalMetadata>;
294307
invokeGetPrerenderParams: boolean;
295308
includePrerenderFallbackRoutes: boolean;
@@ -421,8 +434,9 @@ function appendPreloadToMetadata(
421434
* all parameterized paths, returning any errors encountered.
422435
*
423436
* @param serverConfigRouteTree - The tree representing the server's routing setup.
424-
* @param redirectTo - Optional path to redirect to, if specified.
437+
* @param route - The route configuration.
425438
* @param parentRoute - The path of the parent route.
439+
* @param baseHref - The base href of the application.
426440
* @param metadata - The metadata associated with the route tree node.
427441
* @param parentInjector - The dependency injection container for the parent route.
428442
* @param invokeGetPrerenderParams - A flag indicating whether to invoke the `getPrerenderParams` function.
@@ -431,8 +445,9 @@ function appendPreloadToMetadata(
431445
*/
432446
async function* handleSSGRoute(
433447
serverConfigRouteTree: RouteTree<ServerConfigRouteTreeAdditionalMetadata> | undefined,
434-
redirectTo: string | undefined,
448+
route: Route,
435449
parentRoute: string,
450+
baseHref: string,
436451
metadata: ServerConfigRouteTreeNodeMetadata,
437452
parentInjector: Injector,
438453
invokeGetPrerenderParams: boolean,
@@ -451,8 +466,18 @@ async function* handleSSGRoute(
451466
delete meta['getPrerenderParams'];
452467
}
453468

469+
const redirectTo = typeof route.redirectTo === 'string' ? route.redirectTo : undefined;
454470
if (redirectTo !== undefined) {
455-
meta.redirectTo = resolveRedirectTo(parentRoute, redirectTo);
471+
// Reference the route parameters by their position in the URL (see `buildPathWithParams`).
472+
const routeSegments = joinUrlParts(baseHref, currentRoutePath).split('/');
473+
meta.redirectTo = resolveRedirectTo(
474+
parentRoute,
475+
replaceRedirectParams(
476+
route,
477+
redirectTo,
478+
(parameter) => `*${routeSegments.lastIndexOf(parameter)}`,
479+
),
480+
);
456481
meta.relativeRedirect = redirectTo[0] !== '/' ? true : undefined;
457482
}
458483

@@ -511,12 +536,13 @@ async function* handleSSGRoute(
511536
redirectTo:
512537
redirectTo === undefined
513538
? undefined
514-
: replaceRedirectParams(
515-
resolveRedirectTo(
516-
parentRoute.replace(URL_PARAMETER_GLOBAL_REGEXP, replacer),
539+
: resolveRedirectTo(
540+
parentRoute.replace(URL_PARAMETER_GLOBAL_REGEXP, replacer),
541+
replaceRedirectParams(
542+
route,
517543
redirectTo,
544+
(parameter) => params[parameter.slice(1)],
518545
),
519-
(parameter) => params[parameter.slice(1)],
520546
),
521547
};
522548
}
@@ -595,21 +621,40 @@ function resolveRedirectTo(parentRoute: string, redirectTo: string): string {
595621
/**
596622
* Replaces the route parameters, such as `:id`, in the path of `redirectTo`.
597623
*
598-
* Like in the Angular router, only whole path segments starting with `:` are parameters.
599-
* Parameters for which `resolve` returns `undefined` are kept as is.
624+
* Like in the Angular router, only whole path segments starting with `:` are parameters, and only
625+
* the parameters defined in the path of the redirecting route itself can be used.
600626
*
627+
* @param route - The redirecting route.
601628
* @param redirectTo - The target path for redirection.
602629
* @param resolve - A function that returns the replacement for a parameter, such as `:id`.
603630
* @returns The redirect path with its route parameters replaced.
631+
* @throws If `redirectTo` uses a parameter that is not defined in the path of the route.
604632
*/
605633
function replaceRedirectParams(
634+
route: Route,
606635
redirectTo: string,
607-
resolve: (parameter: string) => string | undefined,
636+
resolve: (parameter: string) => string,
608637
): string {
638+
if (route.matcher) {
639+
// The parameters of a custom matcher are only known at runtime.
640+
return redirectTo;
641+
}
642+
643+
const routeSegments = route.path?.split('/') ?? [];
609644
const [path] = redirectTo.split(/[?#]/, 1);
610645
const resolvedPath = path
611646
.split('/')
612-
.map((segment) => (segment[0] === ':' ? (resolve(segment) ?? segment) : segment))
647+
.map((segment) => {
648+
if (segment[0] !== ':') {
649+
return segment;
650+
}
651+
652+
if (!routeSegments.includes(segment)) {
653+
throw new Error(`Cannot redirect to '${redirectTo}'. Cannot find '${segment}'.`);
654+
}
655+
656+
return resolve(segment);
657+
})
613658
.join('/');
614659

615660
return resolvedPath + redirectTo.slice(path.length);
@@ -768,6 +813,7 @@ export async function getRoutesFromAngularRouterConfig(
768813
compiler,
769814
parentInjector: injector,
770815
parentRoute: '',
816+
baseHref,
771817
serverConfigRouteTree,
772818
invokeGetPrerenderParams,
773819
includePrerenderFallbackRoutes,
@@ -888,19 +934,8 @@ export function extractRoutesAndCreateRouteTree(options: {
888934
);
889935

890936
for (const { route, ...metadata } of routes) {
891-
const fullRoute = joinUrlParts(baseHref, route);
892-
893937
if (metadata.redirectTo !== undefined) {
894-
// Reference the route parameters by their position in the URL (see `buildPathWithParams`).
895-
const routeSegments = fullRoute.split('/');
896-
metadata.redirectTo = replaceRedirectParams(
897-
joinUrlParts(baseHref, metadata.redirectTo),
898-
(parameter) => {
899-
const position = routeSegments.lastIndexOf(parameter);
900-
901-
return position === -1 ? undefined : `*${position}`;
902-
},
903-
);
938+
metadata.redirectTo = joinUrlParts(baseHref, metadata.redirectTo);
904939
}
905940

906941
// Remove undefined fields
@@ -912,6 +947,7 @@ export function extractRoutesAndCreateRouteTree(options: {
912947
}
913948
}
914949

950+
const fullRoute = joinUrlParts(baseHref, route);
915951
routeTree.insert(fullRoute, metadata);
916952
}
917953

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,9 @@ export function stripIndexHtmlFromURL(url: URL): URL {
153153
}
154154

155155
/**
156-
* Matches a `*<n>` placeholder that makes up a whole path segment, which may be followed by
157-
* the query or fragment of the URL.
156+
* Matches a `*<n>` placeholder that makes up a whole path segment.
158157
*/
159-
const POSITIONAL_PLACEHOLDER_REGEXP = /^\*(\d+)(?=[?#]|$)/;
158+
const POSITIONAL_PLACEHOLDER_REGEXP = /^\*(\d+)$/;
160159

161160
/**
162161
* Resolves `*` placeholders in a path template by mapping them to corresponding segments
@@ -201,19 +200,20 @@ export function buildPathWithParams(toPath: string, fromPath: string): string {
201200
throw new Error(`Invalid fromPath: The string must start with a '/'. Received: '${fromPath}'`);
202201
}
203202

204-
if (!toPath.includes('/*')) {
203+
const [path] = toPath.split(/[?#]/, 1);
204+
if (!path.includes('/*')) {
205205
return toPath;
206206
}
207207

208208
const fromPathParts = fromPath.split('/');
209-
const toPathParts = toPath.split('/');
209+
const toPathParts = path.split('/');
210210
const resolvedParts = toPathParts.map((part, index) =>
211211
part === '*'
212212
? fromPathParts[index]
213213
: part.replace(POSITIONAL_PLACEHOLDER_REGEXP, (_, position) => fromPathParts[position]),
214214
);
215215

216-
return joinUrlParts(...resolvedParts);
216+
return joinUrlParts(...resolvedParts) + toPath.slice(path.length);
217217
}
218218

219219
const MATRIX_PARAMS_REGEX = /;[^/]+/g;

‎packages/angular/ssr/test/routes/ng-routes_spec.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -714,6 +714,22 @@ describe('extractRoutesAndCreateRouteTree', () => {
714714
]);
715715
});
716716

717+
it('should error when a redirect target uses a parameter not defined by the route', async () => {
718+
setAngularAppTestingManifest(
719+
[
720+
{
721+
path: ':lang',
722+
children: [{ path: 'user/:id', redirectTo: 'profile/:lang' }],
723+
},
724+
],
725+
[{ path: '**', renderMode: RenderMode.Server }],
726+
);
727+
728+
const { errors } = await extractRoutesAndCreateRouteTree({ url });
729+
expect(errors).toHaveSize(1);
730+
expect(errors[0]).toContain(`Cannot redirect to 'profile/:lang'. Cannot find ':lang'.`);
731+
});
732+
717733
it('should resolve multi-segment relative redirects against the parent route', async () => {
718734
setAngularAppTestingManifest(
719735
[

‎packages/angular/ssr/test/utils/url_spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ describe('URL Utils', () => {
189189
});
190190

191191
it('should replace a positional placeholder followed by a query or fragment', () => {
192-
expect(buildPathWithParams('/user/*1?tab=1', '/a')).toBe('/user/a?tab=1');
192+
expect(buildPathWithParams('/user/*1?returnTo=/a//b/', '/a')).toBe('/user/a?returnTo=/a//b/');
193193
expect(buildPathWithParams('/user/*1#top', '/a')).toBe('/user/a#top');
194194
});
195195

0 commit comments

Comments
 (0)