Skip to content

Commit 9566d2b

Browse files
committed
fix(@angular/build): serialize server manifest asset paths
Serialize route-derived asset keys, base paths, hashes, and App Engine entry points before embedding them in executable ESM manifests. Generate bounded ASCII asset chunk names with a path digest so filesystem-sensitive and URL-significant characters remain filename data without creating chunk-name collisions. Add focused manifest coverage and exercise an apostrophe-bearing prerender route end to end.
1 parent 49584a2 commit 9566d2b

3 files changed

Lines changed: 142 additions & 11 deletions

File tree

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

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
BuildOutputFileType,
1919
createOutputFile,
2020
} from '../../tools/esbuild/bundler-files';
21+
import { calculateHash } from '../hash';
2122
import { findNonce } from '../index-file/nonce';
2223
import { joinUrlParts } from '../url';
2324

@@ -60,6 +61,37 @@ function escapeUnsafeChars(str: string): string {
6061
return str.replace(/[$`\\]/g, (c) => UNSAFE_CHAR_MAP[c]);
6162
}
6263

64+
/**
65+
* Matches every character which is not safe in the name of a generated server asset chunk.
66+
*/
67+
const UNSAFE_CHUNK_NAME_CHARACTER_REGEXP = /[^a-zA-Z0-9_-]/g;
68+
69+
/**
70+
* The maximum number of characters of an asset path kept in the name of its generated chunk.
71+
* The appended digest is what makes the name unique, so the readable part can be truncated to
72+
* stay well within the file name length limits of all supported platforms.
73+
*/
74+
const MAX_CHUNK_NAME_LENGTH = 128;
75+
76+
/**
77+
* Builds the path of the generated chunk which holds the content of a server asset.
78+
*
79+
* Asset paths are derived from route paths and can therefore contain characters which are unusable
80+
* in a file name (`?`, `:` and `*` are invalid on Windows) or which change how the generated
81+
* dynamic import is resolved (`?`, `#` and `%` are URL syntax). Those characters are replaced, and
82+
* a digest of the asset path is appended so that two asset paths never share a chunk.
83+
*
84+
* @param assetPath - The path of the asset, for example `store/summer sale/index.html`.
85+
* @returns The path of the chunk to generate for the asset.
86+
*/
87+
function generateServerAssetChunkPath(assetPath: string): string {
88+
const name = assetPath
89+
.replace(UNSAFE_CHUNK_NAME_CHARACTER_REGEXP, '_')
90+
.slice(0, MAX_CHUNK_NAME_LENGTH);
91+
92+
return `assets-chunks/${name}-${calculateHash(assetPath)}.mjs`;
93+
}
94+
6395
/**
6496
* Generates the server manifest for the App Engine environment.
6597
*
@@ -85,7 +117,7 @@ export function generateAngularServerAppEngineManifest(
85117
for (const locale of i18nOptions.inlineLocales) {
86118
const { subPath } = i18nOptions.locales[locale];
87119
const importPath = `${subPath ? `${subPath}/` : ''}${MAIN_SERVER_OUTPUT_FILENAME}`;
88-
entryPoints[subPath] = `() => import('./${importPath}')`;
120+
entryPoints[subPath] = `() => import(${JSON.stringify(`./${importPath}`)})`;
89121
supportedLocales[locale] = subPath;
90122
}
91123
} else {
@@ -101,12 +133,12 @@ export function generateAngularServerAppEngineManifest(
101133

102134
const manifestContent = `
103135
export default {
104-
basePath: '${basePath}',
136+
basePath: ${JSON.stringify(basePath)},
105137
allowedHosts: ${JSON.stringify(allowedHosts, undefined, 2)},
106138
supportedLocales: ${JSON.stringify(supportedLocales, undefined, 2)},
107139
entryPoints: {
108140
${Object.entries(entryPoints)
109-
.map(([key, value]) => `'${key}': ${value}`)
141+
.map(([key, value]) => `${JSON.stringify(key)}: ${value}`)
110142
.join(',\n ')}
111143
},
112144
};
@@ -170,7 +202,7 @@ export async function generateAngularServerAppManifest(
170202
for (const file of [...additionalHtmlOutputFiles.values(), ...outputFiles]) {
171203
const extension = extname(file.path);
172204
if (extension === '.html') {
173-
const jsChunkFilePath = `assets-chunks/${file.path.replace(/[./]/g, '_')}.mjs`;
205+
const jsChunkFilePath = generateServerAssetChunkPath(file.path);
174206
const escapedContent = escapeUnsafeChars(file.text);
175207

176208
serverAssetsChunks.push(
@@ -190,8 +222,11 @@ export async function generateAngularServerAppManifest(
190222
pos = file.text.indexOf('\r\n', pos + 2);
191223
}
192224

225+
// Asset paths are derived from route paths and can contain arbitrary characters, so they are
226+
// serialized rather than interpolated into the generated executable manifest.
193227
serverAssets[file.path] =
194-
`{size: ${size}, hash: '${file.hash}', text: () => import('./${jsChunkFilePath}').then(m => m.default)}`;
228+
`{size: ${size}, hash: ${JSON.stringify(file.hash)}, ` +
229+
`text: () => import(${JSON.stringify(`./${jsChunkFilePath}`)}).then(m => m.default)}`;
195230
} else if (inlineCriticalCss && extension === '.css') {
196231
const sheet = compileSheet(file.text, {
197232
href: joinUrlParts(publicPath ?? '', file.path),
@@ -213,15 +248,15 @@ export async function generateAngularServerAppManifest(
213248
const manifestContent = `
214249
export default {
215250
bootstrap: () => import('./main.server.mjs').then(m => m.default),
216-
baseHref: '${baseHref}',
251+
baseHref: ${JSON.stringify(baseHref)},
217252
${criticalCssPlans.length ? ` criticalCssPlans: ${JSON.stringify(criticalCssPlans)},\n` : ''}${
218253
nonce ? ` nonce: ${JSON.stringify(nonce)},\n` : ''
219254
} locale: ${JSON.stringify(locale)},
220255
routes: ${JSON.stringify(routes, undefined, 2)},
221256
entryPointToBrowserMapping: ${JSON.stringify(entryPointToBrowserMapping, undefined, 2)},
222257
assets: {
223258
${Object.entries(serverAssets)
224-
.map(([key, value]) => `'${key}': ${value}`)
259+
.map(([key, value]) => `${JSON.stringify(key)}: ${value}`)
225260
.join(',\n ')}
226261
},
227262
};

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

Lines changed: 98 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,48 @@ import { BuildOutputFileType, createOutputFile } from '../../tools/esbuild/bundl
1111
import { initializeHash } from '../hash';
1212
import { generateAngularServerAppManifest } from './manifest';
1313

14+
/**
15+
* Evaluates a generated manifest, which both asserts that it is syntactically valid JavaScript and
16+
* gives access to the values it declares. The dynamic imports it contains are never invoked.
17+
*/
18+
function evaluateManifest(manifestContent: string): Record<string, unknown> {
19+
return new Function(manifestContent.replace('export default', 'return'))() as Record<
20+
string,
21+
unknown
22+
>;
23+
}
24+
25+
const dummyMetafile = { inputs: {}, outputs: {} } as unknown as Metafile;
26+
27+
function generateManifest(
28+
htmlOutputFiles: Record<string, string>,
29+
baseHref = '/',
30+
): ReturnType<typeof generateAngularServerAppManifest> {
31+
const additionalHtmlOutputFiles = new Map(
32+
Object.entries(htmlOutputFiles).map(([path, content]) => [
33+
path,
34+
createOutputFile(path, content, BuildOutputFileType.Browser),
35+
]),
36+
);
37+
38+
return generateAngularServerAppManifest(
39+
additionalHtmlOutputFiles,
40+
[],
41+
false,
42+
undefined,
43+
undefined,
44+
baseHref,
45+
new Set(),
46+
dummyMetafile,
47+
undefined,
48+
);
49+
}
50+
1451
describe('generateAngularServerAppManifest', () => {
1552
beforeAll(async () => {
1653
await initializeHash();
1754
});
1855

19-
const dummyMetafile = { inputs: {}, outputs: {} } as unknown as Metafile;
20-
2156
it('should include criticalCssPlans when inlineCriticalCss is true', async () => {
2257
const additionalHtml = new Map([
2358
[
@@ -138,6 +173,66 @@ describe('generateAngularServerAppManifest', () => {
138173
);
139174

140175
expect(serverAssetsChunks.some((chunk) => chunk.path.includes('styles'))).toBeFalse();
141-
expect(manifestContent).not.toContain("'styles.css':");
176+
expect(manifestContent).not.toContain('"styles.css":');
177+
});
178+
179+
it('serializes asset paths which contain JavaScript string delimiters', async () => {
180+
const assetPath = "catalog/customer's-choice/index.html";
181+
const { manifestContent } = await generateManifest({
182+
[assetPath]: '<main>Featured</main>',
183+
});
184+
185+
const assets = evaluateManifest(manifestContent)['assets'] as Record<string, unknown>;
186+
expect(Object.keys(assets)).toEqual([assetPath]);
187+
});
188+
189+
it('serializes a base href which contains JavaScript string delimiters', async () => {
190+
const { manifestContent } = await generateManifest(
191+
{ 'index.html': '<main></main>' },
192+
"/o'brien/",
193+
);
194+
195+
expect(evaluateManifest(manifestContent)['baseHref']).toBe("/o'brien/");
196+
});
197+
198+
it('generates chunk names which are usable as a file name and as a module specifier', async () => {
199+
const assetPath = "catalog/customer's#featured?ratio=50%/index.html";
200+
const { serverAssetsChunks } = await generateManifest({
201+
[assetPath]: '<main>Featured</main>',
202+
});
203+
204+
expect(serverAssetsChunks).toHaveSize(1);
205+
expect(serverAssetsChunks[0].path).toMatch(/^assets-chunks\/[a-zA-Z0-9_-]+\.mjs$/);
206+
});
207+
208+
it('generates a dynamic import which resolves back to the emitted chunk', async () => {
209+
// The in-memory ESM loader used while prerendering resolves the specifier as a URL and looks the
210+
// result up by output file path, so the two have to match exactly.
211+
const assetPath = "catalog/customer's#featured?ratio=50%/index.html";
212+
const { manifestContent, serverAssetsChunks } = await generateManifest({
213+
[assetPath]: '<main>Featured</main>',
214+
});
215+
216+
const assets = evaluateManifest(manifestContent)['assets'] as Record<
217+
string,
218+
{ text: () => Promise<string> }
219+
>;
220+
const specifier = /import\("(.+?)"\)/.exec(assets[assetPath].text.toString())?.[1];
221+
222+
const root = 'file:///virtual/root/';
223+
expect(specifier).toBeDefined();
224+
expect(new URL(specifier as string, root).href.slice(root.length)).toBe(
225+
serverAssetsChunks[0].path,
226+
);
227+
});
228+
229+
it('generates a distinct chunk for asset paths which map to the same name', async () => {
230+
const { serverAssetsChunks } = await generateManifest({
231+
'foo/bar/index.html': '<main>nested</main>',
232+
'foo_bar/index.html': '<main>flat</main>',
233+
});
234+
235+
expect(serverAssetsChunks).toHaveSize(2);
236+
expect(serverAssetsChunks[0].path).not.toBe(serverAssetsChunks[1].path);
142237
});
143238
});

tests/e2e/tests/build/server-rendering/server-routes-output-mode-server.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ export default async function () {
7777
path: 'ssg/:id',
7878
renderMode: RenderMode.Prerender,
7979
headers: { 'x-custom': 'ssg-with-params' },
80-
getPrerenderParams: async() => [{id: 'one'}, {id: 'two'}],
80+
getPrerenderParams: async() => [{id: 'one'}, {id: 'two'}, {id: "customer's-choice"}],
8181
},
8282
{
8383
path: 'ssr',
@@ -115,6 +115,7 @@ export default async function () {
115115
'ssg/index.html': 'ssg works!',
116116
'ssg/one/index.html': 'ssg-with-params works!',
117117
'ssg/two/index.html': 'ssg-with-params works!',
118+
"ssg/customer's-choice/index.html": 'ssg-with-params works!',
118119
};
119120

120121
for (const [filePath, fileMatch] of Object.entries(expects)) {

0 commit comments

Comments
 (0)