fix(@angular/build): escape prerender redirect URLs

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

@@ -15,6 +15,7 @@ import {
   BuildOutputFileType,
   createOutputFile,
 } from '../../tools/esbuild/bundler-files';
+import { calculateHash } from '../hash';
 
 export const SERVER_APP_MANIFEST_FILENAME = 'angular-app-manifest.mjs';
 export const SERVER_APP_ENGINE_MANIFEST_FILENAME = 'angular-app-engine-manifest.mjs';
@@ -60,6 +61,37 @@ function escapeUnsafeChars(str: string): string {
   return str.replace(/[$`\\]/g, (c) => UNSAFE_CHAR_MAP[c]);
 }
 
+/**
+ * Matches every character which is not safe in the name of a generated server asset chunk.
+ */
+const UNSAFE_CHUNK_NAME_CHARACTER_REGEXP = /[^a-zA-Z0-9_-]/g;
+
+/**
+ * The maximum number of characters of an asset path kept in the name of its generated chunk.
+ * The appended digest is what makes the name unique, so the readable part can be truncated to
+ * stay well within the file name length limits of all supported platforms.
+ */
+const MAX_CHUNK_NAME_LENGTH = 128;
+
+/**
+ * Builds the path of the generated chunk which holds the content of a server asset.
+ *
+ * Asset paths are derived from route paths and can therefore contain characters which are unusable
+ * in a file name (`?`, `:` and `*` are invalid on Windows) or which change how the generated
+ * dynamic import is resolved (`?`, `#` and `%` are URL syntax). Those characters are replaced, and
+ * a digest of the asset path is appended so that two asset paths never share a chunk.
+ *
+ * @param assetPath - The path of the asset, for example `store/summer sale/index.html`.
+ * @returns The path of the chunk to generate for the asset.
+ */
+function generateServerAssetChunkPath(assetPath: string): string {
+  const name = assetPath
+    .replace(UNSAFE_CHUNK_NAME_CHARACTER_REGEXP, '_')
+    .slice(0, MAX_CHUNK_NAME_LENGTH);
+
+  return `assets-chunks/${name}-${calculateHash(assetPath)}.mjs`;
+}
+
 /**
  * Generates the server manifest for the App Engine environment.
  *
@@ -85,7 +117,7 @@ export function generateAngularServerAppEngineManifest(
     for (const locale of i18nOptions.inlineLocales) {
       const { subPath } = i18nOptions.locales[locale];
       const importPath = `${subPath ? `${subPath}/` : ''}${MAIN_SERVER_OUTPUT_FILENAME}`;
-      entryPoints[subPath] = `() => import('./${importPath}')`;
+      entryPoints[subPath] = `() => import(${JSON.stringify(`./${importPath}`)})`;
       supportedLocales[locale] = subPath;
     }
   } else {
@@ -101,12 +133,12 @@ export function generateAngularServerAppEngineManifest(
 
   const manifestContent = `
 export default {
-  basePath: '${basePath}',
+  basePath: ${JSON.stringify(basePath)},
   allowedHosts: ${JSON.stringify(allowedHosts, undefined, 2)},
   supportedLocales: ${JSON.stringify(supportedLocales, undefined, 2)},
   entryPoints: {
     ${Object.entries(entryPoints)
-      .map(([key, value]) => `'${key}': ${value}`)
+      .map(([key, value]) => `${JSON.stringify(key)}: ${value}`)
       .join(',\n    ')}
   },
 };
@@ -163,7 +195,7 @@ export function generateAngularServerAppManifest(
   for (const file of [...additionalHtmlOutputFiles.values(), ...outputFiles]) {
     const extension = extname(file.path);
     if (extension === '.html' || (inlineCriticalCss && extension === '.css')) {
-      const jsChunkFilePath = `assets-chunks/${file.path.replace(/[./]/g, '_')}.mjs`;
+      const jsChunkFilePath = generateServerAssetChunkPath(file.path);
       const escapedContent = escapeUnsafeChars(file.text);
 
       serverAssetsChunks.push(
@@ -183,8 +215,11 @@ export function generateAngularServerAppManifest(
         pos = file.text.indexOf('\r\n', pos + 2);
       }
 
+      // Asset paths are derived from route paths and can contain arbitrary characters, so they are
+      // serialized rather than interpolated into the generated executable manifest.
       serverAssets[file.path] =
-        `{size: ${size}, hash: '${file.hash}', text: () => import('./${jsChunkFilePath}').then(m => m.default)}`;
+        `{size: ${size}, hash: ${JSON.stringify(file.hash)}, ` +
+        `text: () => import(${JSON.stringify(`./${jsChunkFilePath}`)}).then(m => m.default)}`;
     }
   }
 
@@ -197,13 +232,13 @@ export function generateAngularServerAppManifest(
 export default {
   bootstrap: () => import('./main.server.mjs').then(m => m.default),
   inlineCriticalCss: ${inlineCriticalCss},
-  baseHref: '${baseHref}',
+  baseHref: ${JSON.stringify(baseHref)},
   locale: ${JSON.stringify(locale)},
   routes: ${JSON.stringify(routes, undefined, 2)},
   entryPointToBrowserMapping: ${JSON.stringify(entryPointToBrowserMapping, undefined, 2)},
   assets: {
     ${Object.entries(serverAssets)
-      .map(([key, value]) => `'${key}': ${value}`)
+      .map(([key, value]) => `${JSON.stringify(key)}: ${value}`)
       .join(',\n    ')}
   },
 };

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

@@ -0,0 +1,105 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.dev/license
+ */
+
+import { BuildOutputFileType, createOutputFile } from '../../tools/esbuild/bundler-files';
+import { initializeHash } from '../hash';
+import { generateAngularServerAppManifest } from './manifest';
+
+/**
+ * Evaluates a generated manifest, which both asserts that it is syntactically valid JavaScript and
+ * gives access to the values it declares. The dynamic imports it contains are never invoked.
+ */
+function evaluateManifest(manifestContent: string): Record<string, unknown> {
+  return new Function(manifestContent.replace('export default', 'return'))() as Record<
+    string,
+    unknown
+  >;
+}
+
+function generateManifest(
+  htmlOutputFiles: Record<string, string>,
+  baseHref = '/',
+): ReturnType<typeof generateAngularServerAppManifest> {
+  const additionalHtmlOutputFiles = new Map(
+    Object.entries(htmlOutputFiles).map(([path, content]) => [
+      path,
+      createOutputFile(path, content, BuildOutputFileType.Browser),
+    ]),
+  );
+
+  return generateAngularServerAppManifest(
+    additionalHtmlOutputFiles,
+    [],
+    false,
+    undefined,
+    undefined,
+    baseHref,
+    new Set(),
+    { inputs: {}, outputs: {} },
+    undefined,
+  );
+}
+
+describe('generateAngularServerAppManifest', () => {
+  beforeAll(async () => {
+    await initializeHash();
+  });
+
+  it('serializes asset paths which contain JavaScript string delimiters', () => {
+    const assetPath = "catalog/customer's-choice/index.html";
+    const { manifestContent } = generateManifest({ [assetPath]: '<main>Featured</main>' });
+
+    const assets = evaluateManifest(manifestContent)['assets'] as Record<string, unknown>;
+    expect(Object.keys(assets)).toEqual([assetPath]);
+  });
+
+  it('serializes a base href which contains JavaScript string delimiters', () => {
+    const { manifestContent } = generateManifest({ 'index.html': '<main></main>' }, "/o'brien/");
+
+    expect(evaluateManifest(manifestContent)['baseHref']).toBe("/o'brien/");
+  });
+
+  it('generates chunk names which are usable as a file name and as a module specifier', () => {
+    const assetPath = "catalog/customer's#featured?ratio=50%/index.html";
+    const { serverAssetsChunks } = generateManifest({ [assetPath]: '<main>Featured</main>' });
+
+    expect(serverAssetsChunks).toHaveSize(1);
+    expect(serverAssetsChunks[0].path).toMatch(/^assets-chunks\/[a-zA-Z0-9_-]+\.mjs$/);
+  });
+
+  it('generates a dynamic import which resolves back to the emitted chunk', () => {
+    // The in-memory ESM loader used while prerendering resolves the specifier as a URL and looks the
+    // result up by output file path, so the two have to match exactly.
+    const assetPath = "catalog/customer's#featured?ratio=50%/index.html";
+    const { manifestContent, serverAssetsChunks } = generateManifest({
+      [assetPath]: '<main>Featured</main>',
+    });
+
+    const assets = evaluateManifest(manifestContent)['assets'] as Record<
+      string,
+      { text: () => Promise<string> }
+    >;
+    const specifier = /import\("(.+?)"\)/.exec(assets[assetPath].text.toString())?.[1];
+
+    const root = 'file:///virtual/root/';
+    expect(specifier).toBeDefined();
+    expect(new URL(specifier as string, root).href.slice(root.length)).toBe(
+      serverAssetsChunks[0].path,
+    );
+  });
+
+  it('generates a distinct chunk for asset paths which map to the same name', () => {
+    const { serverAssetsChunks } = generateManifest({
+      'foo/bar/index.html': '<main>nested</main>',
+      'foo_bar/index.html': '<main>flat</main>',
+    });
+
+    expect(serverAssetsChunks).toHaveSize(2);
+    expect(serverAssetsChunks[0].path).not.toBe(serverAssetsChunks[1].path);
+  });
+});

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

@@ -22,26 +22,53 @@ export function isSsrRequestHandler(
   return typeof value === 'function' && '__ng_request_handler__' in value;
 }
 
+/**
+ * A mapping of the characters which have to be escaped to be interpolated into HTML,
+ * to their entity equivalents.
+ */
+const HTML_ESCAPE_CHARACTER_MAP: Record<string, string> = {
+  '&': '&amp;',
+  '<': '&lt;',
+  '>': '&gt;',
+  '"': '&quot;',
+  "'": '&#39;',
+};
+
+/**
+ * Escapes the characters of a value which is interpolated into HTML text or into a quoted
+ * attribute value.
+ *
+ * @param text - The value to escape.
+ * @returns The escaped value.
+ */
+function escapeHtml(text: string): string {
+  return text.replace(/[&<>"']/g, (character) => HTML_ESCAPE_CHARACTER_MAP[character]);
+}
+
 /**
  * Generates a static HTML page with a meta refresh tag to redirect the user to a specified URL.
  *
  * This function creates a simple HTML page that performs a redirect using a meta tag.
  * It includes a fallback link in case the meta-refresh doesn't work.
  *
+ * The provided URL is HTML-escaped before being interpolated.
+ *
  * @param url - The URL to which the page should redirect.
  * @returns The HTML content of the static redirect page.
  */
 export function generateRedirectStaticPage(url: string): string {
+  const escapedUrl = escapeHtml(url);
+
   return `
 <!DOCTYPE html>
 <html>
   <head>
     <meta charset="utf-8">
     <title>Redirecting</title>
-    <meta http-equiv="refresh" content="0; url=${url}">
+    <meta http-equiv="refresh" content="0; url=${escapedUrl}">
   </head>
   <body>
-    <pre>Redirecting to <a href="${url}">${url}</a></pre>
+    <pre>Redirecting to <a href="${escapedUrl}">${escapedUrl}</a></pre>
   </body>
 </html>
 `.trim();

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

@@ -0,0 +1,34 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.dev/license
+ */
+
+import { generateRedirectStaticPage } from './utils';
+
+describe('generateRedirectStaticPage', () => {
+  it('escapes the ampersands of a redirect target', () => {
+    const page = generateRedirectStaticPage('https://example.com/docs?from=ssg&next=/ssg');
+
+    expect(page).toContain(
+      '<meta http-equiv="refresh" content="0; url=https://example.com/docs?from=ssg&amp;next=/ssg">',
+    );
+    expect(page).toContain(
+      '<a href="https://example.com/docs?from=ssg&amp;next=/ssg">' +
+        'https://example.com/docs?from=ssg&amp;next=/ssg</a>',
+    );
+  });
+
+  it('escapes characters which would break out of the attribute or the tag', () => {
+    const page = generateRedirectStaticPage(`/"><script>alert('1')</script>`);
+
+    expect(page).not.toContain('<script>');
+    expect(page).toContain(
+      '<meta http-equiv="refresh" content="0; url=' +
+        '/&quot;&gt;&lt;script&gt;alert(&#39;1&#39;)&lt;/script&gt;">',
+    );
+    expect(page).toContain('<a href="/&quot;&gt;&lt;script&gt;alert(&#39;1&#39;)&lt;/script&gt;">');
+  });
+});