Skip to content

Commit 77668b0

Browse files
committed
fixup! fix(@angular/build): escape prerender redirect URLs
1 parent a8c8f77 commit 77668b0

4 files changed

Lines changed: 34 additions & 787 deletions

File tree

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

Lines changed: 33 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,8 @@ import { Console } from '../console';
3030
import { AngularAppManifest, getAngularAppManifest } from '../manifest';
3131
import { AngularBootstrap, isNgModule } from '../utils/ng';
3232
import { promiseWithAbort } from '../utils/promise';
33-
import {
34-
NormalizedUrl,
35-
VALID_REDIRECT_RESPONSE_CODES,
36-
isValidRedirectResponseCode,
37-
normalizeAndValidateRedirect,
38-
normalizeAndValidateRoutePath,
39-
} from '../utils/redirect';
40-
import { addLeadingSlash, joinUrlParts, stripLeadingSlash } from '../utils/url';
33+
import { VALID_REDIRECT_RESPONSE_CODES, isValidRedirectResponseCode } from '../utils/redirect';
34+
import { joinUrlParts, stripLeadingSlash } from '../utils/url';
4135
import {
4236
PrerenderFallback,
4337
RenderMode,
@@ -211,23 +205,18 @@ async function* handleRoute(options: {
211205
} else if (typeof redirectTo === 'string') {
212206
// Reference the route parameters by their position in the URL (see `buildPathWithParams`).
213207
const routeSegments = joinUrlParts(baseHref, currentRoutePath).split('/');
214-
const { url, error } = resolveRedirectTo(
215-
parentRoute,
216-
replaceRedirectParams(
217-
route,
218-
redirectTo,
219-
(parameter) => `*${routeSegments.lastIndexOf(parameter)}`,
208+
parentRoutes.set(metadata.route, {
209+
...metadata,
210+
redirectTo: resolveRedirectTo(
211+
parentRoute,
212+
replaceRedirectParams(
213+
route,
214+
redirectTo,
215+
(parameter) => `*${routeSegments.lastIndexOf(parameter)}`,
216+
),
220217
),
221-
);
222-
if (error !== undefined) {
223-
yield { error: invalidRedirectError(metadata.route, error) };
224-
} else {
225-
parentRoutes.set(metadata.route, {
226-
...metadata,
227-
redirectTo: url,
228-
relativeRedirect: redirectTo[0] !== '/' ? true : undefined,
229-
});
230-
}
218+
relativeRedirect: redirectTo[0] !== '/' ? true : undefined,
219+
});
231220
} else {
232221
parentRoutes.set(metadata.route, metadata);
233222
}
@@ -481,21 +470,14 @@ async function* handleSSGRoute(
481470
if (redirectTo !== undefined) {
482471
// Reference the route parameters by their position in the URL (see `buildPathWithParams`).
483472
const routeSegments = joinUrlParts(baseHref, currentRoutePath).split('/');
484-
const { url, error } = resolveRedirectTo(
473+
meta.redirectTo = resolveRedirectTo(
485474
parentRoute,
486475
replaceRedirectParams(
487476
route,
488477
redirectTo,
489478
(parameter) => `*${routeSegments.lastIndexOf(parameter)}`,
490479
),
491480
);
492-
if (error !== undefined) {
493-
yield { error: invalidRedirectError(currentRoutePath, error) };
494-
495-
return;
496-
}
497-
498-
meta.redirectTo = url;
499481
meta.relativeRedirect = redirectTo[0] !== '/' ? true : undefined;
500482
}
501483

@@ -548,41 +530,20 @@ async function* handleSSGRoute(
548530
.replace(URL_PARAMETER_GLOBAL_REGEXP, replacer)
549531
.replace(CATCH_ALL_REGEXP, replacer);
550532

551-
// A parameter value is only meaningful in the context of the route it is substituted into,
552-
// so the composed path is what gets normalized and validated. This also catches values that
553-
// only become unsafe once combined with the route, such as a protocol-relative `//`.
554-
const { url: normalizedRoute, error: routeError } = normalizeAndValidateRoutePath(
555-
addLeadingSlash(routeWithResolvedParams),
556-
);
557-
if (routeError !== undefined) {
558-
yield {
559-
error:
560-
`The '${stripLeadingSlash(currentRoutePath)}' route produced an invalid ` +
561-
`prerender path: ${routeError}`,
562-
};
563-
564-
continue;
565-
}
566-
567-
let resolvedRedirectTo: string | undefined;
568-
if (redirectTo !== undefined) {
569-
const { url, error } = resolveRedirectTo(
570-
parentRoute.replace(URL_PARAMETER_GLOBAL_REGEXP, replacer),
571-
replaceRedirectParams(route, redirectTo, (parameter) => params[parameter.slice(1)]),
572-
);
573-
if (error !== undefined) {
574-
yield { error: invalidRedirectError(routeWithResolvedParams, error) };
575-
576-
continue;
577-
}
578-
579-
resolvedRedirectTo = url;
580-
}
581-
582533
yield {
583534
...meta,
584-
route: normalizedRoute,
585-
redirectTo: resolvedRedirectTo,
535+
route: routeWithResolvedParams,
536+
redirectTo:
537+
redirectTo === undefined
538+
? undefined
539+
: resolveRedirectTo(
540+
parentRoute.replace(URL_PARAMETER_GLOBAL_REGEXP, replacer),
541+
replaceRedirectParams(
542+
route,
543+
redirectTo,
544+
(parameter) => params[parameter.slice(1)],
545+
),
546+
),
586547
};
587548
}
588549
} catch (error) {
@@ -629,10 +590,7 @@ function handlePrerenderParamsReplacement(
629590
);
630591
}
631592

632-
// The matched placeholder includes the separator, so it has to be re-added. The documented
633-
// value of a `**` parameter is a path, which may already carry it: re-adding it unconditionally
634-
// would compose a root catch-all into a protocol-relative `//foo/bar`.
635-
return parameterName === '**' ? addLeadingSlash(value) : value;
593+
return parameterName === '**' ? `/${value}` : value;
636594
};
637595
}
638596

@@ -644,36 +602,20 @@ function handlePrerenderParamsReplacement(
644602
* it is returned as is. If it is a relative path, it is resolved based on the parent route path,
645603
* and, like in the Angular router, its query and fragment are ignored.
646604
*
647-
* The resolved target is then normalized and validated, so that every `redirectTo` reaching the
648-
* route tree is a safe HTTP(S) URL or same-origin path.
649-
*
650605
* @param parentRoute - The path of the parent route.
651606
* @param redirectTo - The target path for redirection.
652-
* @returns The normalized redirect target, or the reason it was rejected.
607+
* @returns The resolved redirect path as a string.
653608
*/
654-
function resolveRedirectTo(parentRoute: string, redirectTo: string): NormalizedUrl {
609+
function resolveRedirectTo(parentRoute: string, redirectTo: string): string {
655610
if (redirectTo[0] === '/') {
656-
// If the redirectTo path is absolute, use it as is.
657-
return normalizeAndValidateRedirect(redirectTo);
611+
// If the redirectTo path is absolute, return it as is.
612+
return redirectTo;
658613
}
659614

660615
// Match Angular router behavior: replace all segments matched by the route.
661616
const [path] = redirectTo.split(/[?#]/, 1);
662617

663-
return normalizeAndValidateRedirect(
664-
joinUrlParts(parentRoute.replace(URL_PARAMETER_GLOBAL_REGEXP, '*'), path),
665-
);
666-
}
667-
668-
/**
669-
* Builds the diagnostic emitted when a route's `redirectTo` cannot be normalized.
670-
*
671-
* @param routePath - The route the `redirectTo` is defined on.
672-
* @param reason - The reason reported by the redirect normalization.
673-
* @returns The error message to report.
674-
*/
675-
function invalidRedirectError(routePath: string, reason: string): string {
676-
return `The 'redirectTo' value for the '${stripLeadingSlash(routePath)}' route is invalid: ${reason}`;
618+
return joinUrlParts(parentRoute.replace(URL_PARAMETER_GLOBAL_REGEXP, '*'), path);
677619
}
678620

679621
/**
@@ -764,27 +706,6 @@ function buildServerConfigRouteTree({ routes, appShellRoute }: ServerRoutesConfi
764706
return { serverConfigRouteTree, errors };
765707
}
766708

767-
/**
768-
* Builds the key used to de-duplicate an extracted route.
769-
*
770-
* `RouteTree` percent-decodes every segment when a route is inserted, so two route paths which
771-
* differ only in their encoding land on the same node, where the later one overwrites the earlier.
772-
* De-duplication therefore runs on the decoded path rather than on the path as it was written:
773-
* a prerendered path is normalized by the URL parser while the path of every other render mode is
774-
* not, so the same route can be reached in both an encoded and an unencoded form.
775-
*
776-
* @param route - The route path to build a key for.
777-
* @returns The decoded route path.
778-
*/
779-
function routeDeduplicationKey(route: string): string {
780-
try {
781-
return route.split('/').map(decodeURIComponent).join('/');
782-
} catch {
783-
// A malformed percent escape is reported when the route is inserted into the route tree.
784-
return route;
785-
}
786-
}
787-
788709
/**
789710
* Retrieves routes from the given Angular application.
790711
*
@@ -908,7 +829,7 @@ export async function getRoutesFromAngularRouterConfig(
908829

909830
// If a result already exists for the exact same route, subsequent matches should be ignored.
910831
// This aligns with Angular's app router behavior, which prioritizes the first route.
911-
const routePath = routeDeduplicationKey(routeMetadata.route);
832+
const routePath = routeMetadata.route;
912833
if (!seenRoutes.has(routePath)) {
913834
routesResults.push(routeMetadata);
914835
seenRoutes.add(routePath);

0 commit comments

Comments
 (0)