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 angular#28903

Author: alan-agius4

packages/angular/ssr/src/app.ts

@@ -24,7 +24,12 @@ import { sha256 } from './utils/crypto';
 import { InlineCriticalCssProcessor } from './utils/inline-critical-css';
 import { LRUCache } from './utils/lru-cache';
 import { AngularBootstrap, renderAngular } from './utils/ng';
-import { joinUrlParts, stripIndexHtmlFromURL, stripLeadingSlash } from './utils/url';
+import {
+  buildPathWithParams,
+  joinUrlParts,
+  stripIndexHtmlFromURL,
+  stripLeadingSlash,
+} from './utils/url';
 
 /**
  * Maximum number of critical CSS entries the cache can store.
@@ -160,11 +165,14 @@ export class AngularServerApp {
 
     const { redirectTo, status, renderMode } = matchedRoute;
     if (redirectTo !== undefined) {
-      // Note: The status code is validated during route extraction.
-      // 302 Found is used by default for redirections
-      // See: https://developer.mozilla.org/en-US/docs/Web/API/Response/redirect_static#status
-      // eslint-disable-next-line @typescript-eslint/no-explicit-any
-      return Response.redirect(new URL(redirectTo, url), (status as any) ?? 302);
+      return Response.redirect(
+        new URL(buildPathWithParams(redirectTo, url.pathname), url),
+        // Note: The status code is validated during route extraction.
+        // 302 Found is used by default for redirections
+        // See: https://developer.mozilla.org/en-US/docs/Web/API/Response/redirect_static#status
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        (status as any) ?? 302,
+      );
     }
 
     if (renderMode === RenderMode.Prerender) {
@@ -241,17 +249,8 @@ export class AngularServerApp {
     matchedRoute: RouteTreeNodeMetadata,
     requestContext?: unknown,
   ): Promise<Response | null> {
-    const { redirectTo, status } = matchedRoute;
-
-    if (redirectTo !== undefined) {
-      // Note: The status code is validated during route extraction.
-      // 302 Found is used by default for redirections
-      // See: https://developer.mozilla.org/en-US/docs/Web/API/Response/redirect_static#status
-      // eslint-disable-next-line @typescript-eslint/no-explicit-any
-      return Response.redirect(new URL(redirectTo, new URL(request.url)), (status as any) ?? 302);
-    }
+    const { renderMode, headers, status } = matchedRoute;
 
-    const { renderMode, headers } = matchedRoute;
     if (!this.allowStaticRouteRender && renderMode === RenderMode.Prerender) {
       return null;
     }

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

@@ -21,7 +21,7 @@ import { ServerAssets } from '../assets';
 import { Console } from '../console';
 import { AngularAppManifest, getAngularAppManifest } from '../manifest';
 import { AngularBootstrap, isNgModule } from '../utils/ng';
-import { joinUrlParts, stripLeadingSlash } from '../utils/url';
+import { addTrailingSlash, joinUrlParts, stripLeadingSlash } from '../utils/url';
 import {
   PrerenderFallback,
   RenderMode,
@@ -146,31 +146,36 @@ async function* traverseRoutesConfig(options: {
       const metadata: ServerConfigRouteTreeNodeMetadata = {
         renderMode: RenderMode.Prerender,
         ...matchedMetaData,
-        route: currentRoutePath,
+        // Match Angular router behavior
+        // ['one', 'two', ''] -> 'one/two/'
+        // ['one', 'two', 'three'] -> 'one/two/three'
+        route: path === '' ? addTrailingSlash(currentRoutePath) : currentRoutePath,
       };
 
       delete metadata.presentInClientRouter;
 
-      // Handle redirects
-      if (typeof redirectTo === 'string') {
-        const redirectToResolved = resolveRedirectTo(currentRoutePath, redirectTo);
+      if (metadata.renderMode === RenderMode.Prerender) {
+        // Handle SSG routes
+        yield* handleSSGRoute(
+          typeof redirectTo === 'string' ? redirectTo : undefined,
+          metadata,
+          parentInjector,
+          invokeGetPrerenderParams,
+          includePrerenderFallbackRoutes,
+        );
+      } else if (typeof redirectTo === 'string') {
+        // Handle redirects
         if (metadata.status && !VALID_REDIRECT_RESPONSE_CODES.has(metadata.status)) {
           yield {
             error:
               `The '${metadata.status}' status code is not a valid redirect response code. ` +
               `Please use one of the following redirect response codes: ${[...VALID_REDIRECT_RESPONSE_CODES.values()].join(', ')}.`,
           };
+
           continue;
         }
-        yield { ...metadata, redirectTo: redirectToResolved };
-      } else if (metadata.renderMode === RenderMode.Prerender) {
-        // Handle SSG routes
-        yield* handleSSGRoute(
-          metadata,
-          parentInjector,
-          invokeGetPrerenderParams,
-          includePrerenderFallbackRoutes,
-        );
+
+        yield { ...metadata, redirectTo: resolveRedirectTo(metadata.route, redirectTo) };
       } else {
         yield metadata;
       }
@@ -214,13 +219,15 @@ async function* traverseRoutesConfig(options: {
  * Handles SSG (Static Site Generation) routes by invoking `getPrerenderParams` and yielding
  * all parameterized paths, returning any errors encountered.
  *
+ * @param redirectTo - Optional path to redirect to, if specified.
  * @param metadata - The metadata associated with the route tree node.
  * @param parentInjector - The dependency injection container for the parent route.
  * @param invokeGetPrerenderParams - A flag indicating whether to invoke the `getPrerenderParams` function.
  * @param includePrerenderFallbackRoutes - A flag indicating whether to include fallback routes in the result.
  * @returns An async iterable iterator that yields route tree node metadata for each SSG path or errors.
  */
 async function* handleSSGRoute(
+  redirectTo: string | undefined,
   metadata: ServerConfigRouteTreeNodeMetadata,
   parentInjector: Injector,
   invokeGetPrerenderParams: boolean,
@@ -239,6 +246,10 @@ async function* handleSSGRoute(
     delete meta['getPrerenderParams'];
   }
 
+  if (redirectTo !== undefined) {
+    meta.redirectTo = resolveRedirectTo(currentRoutePath, redirectTo);
+  }
+
   if (!URL_PARAMETER_REGEXP.test(currentRoutePath)) {
     // Route has no parameters
     yield {
@@ -279,7 +290,14 @@ async function* handleSSGRoute(
           return value;
         });
 
-        yield { ...meta, route: routeWithResolvedParams };
+        yield {
+          ...meta,
+          route: routeWithResolvedParams,
+          redirectTo:
+            redirectTo === undefined
+              ? undefined
+              : resolveRedirectTo(routeWithResolvedParams, redirectTo),
+        };
       }
     } catch (error) {
       yield { error: `${(error as Error).message}` };
@@ -319,7 +337,7 @@ function resolveRedirectTo(routePath: string, redirectTo: string): string {
   }
 
   // Resolve relative redirectTo based on the current route path.
-  const segments = routePath.split('/');
+  const segments = routePath.replace(URL_PARAMETER_REGEXP, '*').split('/');
   segments.pop(); // Remove the last segment to make it relative.
 
   return joinUrlParts(...segments, redirectTo);
@@ -459,7 +477,6 @@ export async function getRoutesFromAngularRouterConfig(
         includePrerenderFallbackRoutes,
       });
 
-      let seenAppShellRoute: string | undefined;
       for await (const result of traverseRoutes) {
         if ('error' in result) {
           errors.push(result.error);
@@ -549,8 +566,17 @@ export async function extractRoutesAndCreateRouteTree(
       metadata.redirectTo = joinUrlParts(baseHref, metadata.redirectTo);
     }
 
+    // Remove undefined fields
+    // Helps avoid unnecessary test updates
+    for (const [key, value] of Object.entries(metadata)) {
+      if (value === undefined) {
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        delete (metadata as any)[key];
+      }
+    }
+
     const fullRoute = joinUrlParts(baseHref, route);
-    routeTree.insert(fullRoute, metadata);
+    routeTree.insert(fullRoute.replace(URL_PARAMETER_REGEXP, '*'), metadata);
   }
 
   return {

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

@@ -61,6 +61,23 @@ export function addLeadingSlash(url: string): string {
   return url[0] === '/' ? url : `/${url}`;
 }
 
+/**
+ * Adds a trailing slash to a URL if it does not already have one.
+ *
+ * @param url - The URL string to which the trailing slash will be added.
+ * @returns The URL string with a trailing slash.
+ *
+ * @example
+ * ```js
+ * addTrailingSlash('path'); // 'path/'
+ * addTrailingSlash('path/'); // 'path/'
+ * ```
+ */
+export function addTrailingSlash(url: string): string {
+  // Check if the URL already end with a slash
+  return url[url.length - 1] === '/' ? url : `${url}/`;
+}
+
 /**
  * Joins URL parts into a single URL string.
  *
@@ -128,3 +145,54 @@ export function stripIndexHtmlFromURL(url: URL): URL {
 
   return url;
 }
+
+/**
+ * Resolves `*` placeholders in a path template by mapping them to corresponding segments
+ * from a base path. This is useful for constructing paths dynamically based on a given base path.
+ *
+ * The function processes the `toPath` string, replacing each `*` placeholder with
+ * the corresponding segment from the `fromPath`. If the `toPath` contains no placeholders,
+ * it is returned as-is. Invalid `toPath` formats (not starting with `/`) will throw an error.
+ *
+ * @param toPath - A path template string that may contain `*` placeholders. Each `*` is replaced
+ * by the corresponding segment from the `fromPath`. Static paths (e.g., `/static/path`) are returned
+ * directly without placeholder replacement.
+ * @param fromPath - A base path string, split into segments, that provides values for
+ * replacing `*` placeholders in the `toPath`.
+ * @returns A resolved path string with `*` placeholders replaced by segments from the `fromPath`,
+ * or the `toPath` returned unchanged if it contains no placeholders.
+ *
+ * @throws If the `toPath` does not start with a `/`, indicating an invalid path format.
+ *
+ * @example
+ * ```typescript
+ * // Example with placeholders resolved
+ * const resolvedPath = buildPathWithParams('/*\/details', '/123/abc');
+ * console.log(resolvedPath); // Outputs: '/123/details'
+ *
+ * // Example with a static path
+ * const staticPath = buildPathWithParams('/static/path', '/base/unused');
+ * console.log(staticPath); // Outputs: '/static/path'
+ * ```
+ */
+export function buildPathWithParams(toPath: string, fromPath: string): string {
+  if (toPath[0] !== '/') {
+    throw new Error(`Invalid toPath: The string must start with a '/'. Received: '${toPath}'`);
+  }
+
+  if (fromPath[0] !== '/') {
+    throw new Error(`Invalid fromPath: The string must start with a '/'. Received: '${fromPath}'`);
+  }
+
+  if (!toPath.includes('/*')) {
+    return toPath;
+  }
+
+  const fromPathParts = fromPath.split('/');
+  const toPathParts = toPath.split('/');
+  const resolvedParts = toPathParts.map((part, index) =>
+    toPathParts[index] === '*' ? fromPathParts[index] : part,
+  );
+
+  return joinUrlParts(...resolvedParts);
+}

packages/angular/ssr/test/app_spec.ts

@@ -38,6 +38,7 @@ describe('AngularServerApp', () => {
         { path: 'page-with-status', component: HomeComponent },
         { path: 'redirect', redirectTo: 'home' },
         { path: 'redirect/relative', redirectTo: 'home' },
+        { path: 'redirect/:param/relative', redirectTo: 'home' },
         { path: 'redirect/absolute', redirectTo: '/home' },
       ],
       [
@@ -117,6 +118,12 @@ describe('AngularServerApp', () => {
         expect(response?.status).toBe(302);
       });
 
+      it('should correctly handle relative nested redirects with parameter', async () => {
+        const response = await app.handle(new Request('http://localhost/redirect/param/relative'));
+        expect(response?.headers.get('location')).toContain('http://localhost/redirect/param/home');
+        expect(response?.status).toBe(302);
+      });
+
       it('should correctly handle absolute nested redirects', async () => {
         const response = await app.handle(new Request('http://localhost/redirect/absolute'));
         expect(response?.headers.get('location')).toContain('http://localhost/home');