Skip to content

Commit db45eed

Browse files
SkyZeroZxalan-agius4
authored andcommitted
fix(@angular/build): serialize server manifest asset paths
Serialize the values embedded in the generated server manifests with `JSON.stringify`, so a prerendered route such as `ssg/customer's-choice` no longer produces an invalid manifest. (cherry picked from commit dd09534)
1 parent 7b9c8d0 commit db45eed

3 files changed

Lines changed: 76 additions & 8 deletions

File tree

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ export function generateAngularServerAppEngineManifest(
8585
for (const locale of i18nOptions.inlineLocales) {
8686
const { subPath } = i18nOptions.locales[locale];
8787
const importPath = `${subPath ? `${subPath}/` : ''}${MAIN_SERVER_OUTPUT_FILENAME}`;
88-
entryPoints[subPath] = `() => import('./${importPath}')`;
88+
entryPoints[subPath] = `() => import(${JSON.stringify(`./${importPath}`)})`;
8989
supportedLocales[locale] = subPath;
9090
}
9191
} else {
@@ -101,12 +101,12 @@ export function generateAngularServerAppEngineManifest(
101101

102102
const manifestContent = `
103103
export default {
104-
basePath: '${basePath}',
104+
basePath: ${JSON.stringify(basePath)},
105105
allowedHosts: ${JSON.stringify(allowedHosts, undefined, 2)},
106106
supportedLocales: ${JSON.stringify(supportedLocales, undefined, 2)},
107107
entryPoints: {
108108
${Object.entries(entryPoints)
109-
.map(([key, value]) => `'${key}': ${value}`)
109+
.map(([key, value]) => `${JSON.stringify(key)}: ${value}`)
110110
.join(',\n ')}
111111
},
112112
};
@@ -190,8 +190,11 @@ export async function generateAngularServerAppManifest(
190190
pos = file.text.indexOf('\r\n', pos + 2);
191191
}
192192

193+
// Asset paths are derived from route paths and can contain arbitrary characters, so they are
194+
// serialized rather than interpolated into the generated executable manifest.
193195
serverAssets[file.path] =
194-
`{size: ${size}, hash: '${file.hash}', text: () => import('./${jsChunkFilePath}').then(m => m.default)}`;
196+
`{size: ${size}, hash: ${JSON.stringify(file.hash)}, ` +
197+
`text: () => import(${JSON.stringify(`./${jsChunkFilePath}`)}).then(m => m.default)}`;
195198
} else if (inlineCriticalCss && extension === '.css') {
196199
const sheet = compileSheet(file.text, {
197200
href: joinUrlParts(publicPath ?? '', file.path),
@@ -213,15 +216,15 @@ export async function generateAngularServerAppManifest(
213216
const manifestContent = `
214217
export default {
215218
bootstrap: () => import('./main.server.mjs').then(m => m.default),
216-
baseHref: '${baseHref}',
219+
baseHref: ${JSON.stringify(baseHref)},
217220
${criticalCssPlans.length ? ` criticalCssPlans: ${JSON.stringify(criticalCssPlans)},\n` : ''}${
218221
nonce ? ` nonce: ${JSON.stringify(nonce)},\n` : ''
219222
} locale: ${JSON.stringify(locale)},
220223
routes: ${JSON.stringify(routes, undefined, 2)},
221224
entryPointToBrowserMapping: ${JSON.stringify(entryPointToBrowserMapping, undefined, 2)},
222225
assets: {
223226
${Object.entries(serverAssets)
224-
.map(([key, value]) => `'${key}': ${value}`)
227+
.map(([key, value]) => `${JSON.stringify(key)}: ${value}`)
225228
.join(',\n ')}
226229
},
227230
};

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

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,38 @@ describe('generateAngularServerAppManifest', () => {
1818

1919
const dummyMetafile = { inputs: {}, outputs: {} } as unknown as Metafile;
2020

21+
/**
22+
* Evaluates a generated manifest, which both asserts that it is syntactically valid JavaScript and
23+
* gives access to the values it declares. The dynamic imports it contains are never invoked.
24+
*/
25+
function evaluateManifest(manifestContent: string): unknown {
26+
return new Function(manifestContent.replace('export default', 'return'))();
27+
}
28+
29+
function generateManifest(
30+
htmlOutputFiles: Record<string, string>,
31+
baseHref = '/',
32+
): ReturnType<typeof generateAngularServerAppManifest> {
33+
const additionalHtmlOutputFiles = new Map(
34+
Object.entries(htmlOutputFiles).map(([path, content]) => [
35+
path,
36+
createOutputFile(path, content, BuildOutputFileType.Browser),
37+
]),
38+
);
39+
40+
return generateAngularServerAppManifest(
41+
additionalHtmlOutputFiles,
42+
[],
43+
false,
44+
undefined,
45+
undefined,
46+
baseHref,
47+
new Set(),
48+
dummyMetafile,
49+
undefined,
50+
);
51+
}
52+
2153
it('should include criticalCssPlans when inlineCriticalCss is true', async () => {
2254
const additionalHtml = new Map([
2355
[
@@ -138,6 +170,38 @@ describe('generateAngularServerAppManifest', () => {
138170
);
139171

140172
expect(serverAssetsChunks.some((chunk) => chunk.path.includes('styles'))).toBeFalse();
141-
expect(manifestContent).not.toContain("'styles.css':");
173+
expect(manifestContent).not.toContain('"styles.css":');
174+
});
175+
176+
it('should serialize asset paths which contain JavaScript string delimiters', async () => {
177+
const assetPath = "catalog/customer's-choice/index.html";
178+
const { manifestContent } = await generateManifest({
179+
[assetPath]: '<main>Featured</main>',
180+
});
181+
182+
expect(evaluateManifest(manifestContent)).toEqual(
183+
jasmine.objectContaining({
184+
assets: {
185+
[assetPath]: jasmine.objectContaining({
186+
size: 21,
187+
hash: jasmine.any(String),
188+
text: jasmine.any(Function),
189+
}),
190+
},
191+
}),
192+
);
193+
});
194+
195+
it('should serialize a base href which contains JavaScript string delimiters', async () => {
196+
const { manifestContent } = await generateManifest(
197+
{ 'index.html': '<main></main>' },
198+
"/o'brien/",
199+
);
200+
201+
expect(evaluateManifest(manifestContent)).toEqual(
202+
jasmine.objectContaining({
203+
baseHref: "/o'brien/",
204+
}),
205+
);
142206
});
143207
});

‎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)