Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 79aad28

Browse files
committed
fix(@angular-devkit/build-angular): support CSP on critical CSS link tags.
Based on #24880 (review). Critters can generate `link` tags with inline `onload` handlers which breaks CSP. These changes update the style nonce processor to remove the `onload` handlers and replicate the behavior with an inline `script` tag that gets the proper nonce. Note that earlier we talked about doing this through Critters which while possible, would still require a custom HTML processor, because we need to both add and remove attributes from an element.
1 parent 0ac5f27 commit 79aad28

File tree

3 files changed

+192
-2
lines changed

3 files changed

+192
-2
lines changed

‎packages/angular_devkit/build_angular/src/builders/app-shell/app-shell_spec.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,4 +163,30 @@ describe('AppShell Builder', () => {
163163
/<linkrel="stylesheet"href="styles\.[a-z0-9]+\.css"media="print"onload="this\.media='all'">/,
164164
);
165165
});
166+
167+
it('applies CSP nonce to critical CSS', async () => {
168+
host.writeMultipleFiles(appShellRouteFiles);
169+
host.replaceInFile('src/index.html', /<app-root/g, '<app-root ngCspNonce="{% nonce %}" ');
170+
const overrides = {
171+
route: 'shell',
172+
browserTarget: 'app:build:production,inline-critical-css',
173+
};
174+
175+
const run = await architect.scheduleTarget(target, overrides);
176+
const output = await run.result;
177+
await run.stop();
178+
179+
expect(output.success).toBe(true);
180+
const fileName = 'dist/index.html';
181+
const content = virtualFs.fileBufferToString(host.scopedSync().read(normalize(fileName)));
182+
183+
expect(content).toContain('app-shell works!');
184+
expect(content).toContain('<style nonce="{% nonce %}">p{color:#000}</style>');
185+
expect(content).toContain('<style ng-app-id="ng" nonce="{% nonce %}">');
186+
expect(content).toContain('<app-root ngcspnonce="{% nonce %}"');
187+
expect(content).toContain('<script nonce="{% nonce %}">');
188+
expect(content).toMatch(
189+
/<linkrel="stylesheet"href="styles\.[a-z0-9]+\.css"media="print"ngCspMedia="all">/,
190+
);
191+
});
166192
});

‎packages/angular_devkit/build_angular/src/utils/index-file/inline-critical-css.ts‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,16 @@ import * as fs from 'fs';
1010

1111
const Critters: typeof import('critters').default = require('critters');
1212

13+
/**
14+
* Pattern used to extract the media query set by Critters in an `onload` handler.
15+
*/
16+
const MEDIA_SET_HANDLER_PATTERN = /^this\.media=["'](.*)["'];?$/;
17+
18+
/**
19+
* Name of the attribute used to save the Critters media query so it can be re-assigned on load.
20+
*/
21+
const CSP_MEDIA_ATTR = 'ngCspMedia';
22+
1323
export interface InlineCriticalCssProcessOptions {
1424
outputPath: string;
1525
}
@@ -20,9 +30,40 @@ export interface InlineCriticalCssProcessorOptions {
2030
readAsset?: (path: string) => Promise<string>;
2131
}
2232

33+
/** Partial representation of an `HTMLElement`. */
34+
interface PartialHTMLElement {
35+
getAttribute(name: string): string | null;
36+
setAttribute(name: string, value: string): void;
37+
hasAttribute(name: string): boolean;
38+
removeAttribute(name: string): void;
39+
appendChild(child: PartialHTMLElement): void;
40+
textContent: string;
41+
tagName: string | null;
42+
children: PartialHTMLElement[];
43+
}
44+
45+
/** Partial representation of an HTML `Document`. */
46+
interface PartialDocument {
47+
head: PartialHTMLElement;
48+
createElement(tagName: string): PartialHTMLElement;
49+
querySelector(selector: string): PartialHTMLElement | null;
50+
}
51+
52+
/** Signature of the `Critters.embedLinkedStylesheet` method. */
53+
type EmbedLinkedStylesheetFn = (
54+
link: PartialHTMLElement,
55+
document: PartialDocument,
56+
) => Promise<unknown>;
57+
2358
class CrittersExtended extends Critters {
2459
readonly warnings: string[] = [];
2560
readonly errors: string[] = [];
61+
private initialEmbedLinkedStylesheet: EmbedLinkedStylesheetFn;
62+
private addedCspScriptsDocuments = new WeakSet<PartialDocument>();
63+
private documentNonces = new WeakMap<PartialDocument, string | null>();
64+
65+
// Inherited from `Critters`, but not exposed in the typings.
66+
protected embedLinkedStylesheet!: EmbedLinkedStylesheetFn;
2667

2768
constructor(
2869
private readonly optionsExtended: InlineCriticalCssProcessorOptions &
@@ -41,17 +82,112 @@ class CrittersExtended extends Critters {
4182
pruneSource: false,
4283
reduceInlineStyles: false,
4384
mergeStylesheets: false,
85+
// Note: if `preload` changes to anything other than `media`, the logic in
86+
// `embedLinkedStylesheetOverride` will have to be updated.
4487
preload: 'media',
4588
noscriptFallback: true,
4689
inlineFonts: true,
4790
});
91+
92+
// We can't use inheritance to override `embedLinkedStylesheet`, because it's not declared in
93+
// the `Critters` .d.ts which means that we can't call the `super` implementation. TS doesn't
94+
// allow for `super` to be cast to a different type.
95+
this.initialEmbedLinkedStylesheet = this.embedLinkedStylesheet;
96+
this.embedLinkedStylesheet = this.embedLinkedStylesheetOverride;
4897
}
4998

5099
public override readFile(path: string): Promise<string> {
51100
const readAsset = this.optionsExtended.readAsset;
52101

53102
return readAsset ? readAsset(path) : fs.promises.readFile(path, 'utf-8');
54103
}
104+
105+
/**
106+
* Override of the Critters `embedLinkedStylesheet` method
107+
* that makes it work with Angular's CSP APIs.
108+
*/
109+
private embedLinkedStylesheetOverride: EmbedLinkedStylesheetFn = async (link, document) => {
110+
const returnValue = await this.initialEmbedLinkedStylesheet(link, document);
111+
const cspNonce = this.findCspNonce(document);
112+
113+
if (cspNonce) {
114+
const crittersMedia = link.getAttribute('onload')?.match(MEDIA_SET_HANDLER_PATTERN);
115+
116+
if (crittersMedia) {
117+
// If there's a Critters-generated `onload` handler and the file has an Angular CSP nonce,
118+
// we have to remove the handler, because it's incompatible with CSP. We save the value
119+
// in a different attribute and we generate a script tag with the nonce that uses
120+
// `addEventListener` to apply the media query instead.
121+
link.removeAttribute('onload');
122+
link.setAttribute(CSP_MEDIA_ATTR, crittersMedia[1]);
123+
this.conditionallyInsertCspLoadingScript(document, cspNonce);
124+
}
125+
126+
// Ideally we would hook in at the time Critters inserts the `style` tags, but there isn't
127+
// a way of doing that at the moment so we fall back to doing it any time a `link` tag is
128+
// inserted. We mitigate it by only iterating the direct children of the `<head>` which
129+
// should be pretty shallow.
130+
document.head.children.forEach((child) => {
131+
if (child.tagName === 'style' && !child.hasAttribute('nonce')) {
132+
child.setAttribute('nonce', cspNonce);
133+
}
134+
});
135+
}
136+
137+
return returnValue;
138+
};
139+
140+
/**
141+
* Finds the CSP nonce for a specific document.
142+
*/
143+
private findCspNonce(document: PartialDocument): string | null {
144+
if (this.documentNonces.has(document)) {
145+
return this.documentNonces.get(document) ?? null;
146+
}
147+
148+
// HTML attribute are case-insensitive, but the parser used by Critters is case-sensitive.
149+
const nonceElement = document.querySelector('[ngCspNonce], [ngcspnonce]');
150+
const cspNonce =
151+
nonceElement?.getAttribute('ngCspNonce') || nonceElement?.getAttribute('ngcspnonce') || null;
152+
153+
this.documentNonces.set(document, cspNonce);
154+
155+
return cspNonce;
156+
}
157+
158+
/**
159+
* Inserts the `script` tag that swaps the critical CSS at runtime,
160+
* if one hasn't been inserted into the document already.
161+
*/
162+
private conditionallyInsertCspLoadingScript(document: PartialDocument, nonce: string) {
163+
if (this.addedCspScriptsDocuments.has(document)) {
164+
return;
165+
}
166+
167+
const script = document.createElement('script');
168+
script.setAttribute('nonce', nonce);
169+
script.textContent = [
170+
`(() => {`,
171+
// Save the `children` in a variable since they're a live DOM node collection.
172+
// We iterate over the direct descendants, instead of going through a `querySelectorAll`,
173+
// because we know that the tags will be directly inside the `head`.
174+
` const children = document.head.children;`,
175+
// Declare `onLoad` outside the loop to avoid leaking memory.
176+
// Can't be an arrow function, because we need `this` to refer to the DOM node.
177+
` function onLoad() {this.media = this.getAttribute('${CSP_MEDIA_ATTR}');}`,
178+
// Has to use a plain for loop, because some browsers don't support
179+
// `forEach` on `children` which is a `HTMLCollection`.
180+
` for (let i = 0; i < children.length; i++) {`,
181+
` const child = children[i];`,
182+
` child.hasAttribute('${CSP_MEDIA_ATTR}') && child.addEventListener('load', onLoad);`,
183+
` }`,
184+
`})();`,
185+
].join('\n');
186+
// Append the script to the head since it needs to
187+
// run as early as possible, after the `link` tags.
188+
document.head.appendChild(script);
189+
this.addedCspScriptsDocuments.add(document);
190+
}
55191
}
56192

57193
export class InlineCriticalCssProcessor {

‎packages/angular_devkit/build_angular/src/utils/index-file/inline-critical-css_spec.ts‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,14 @@ describe('InlineCriticalCssProcessor', () => {
3030
throw new Error('Cannot read asset.');
3131
};
3232

33-
const getContent = (deployUrl: string): string => {
33+
const getContent = (deployUrl: string,bodyContent=''): string => {
3434
return `
3535
<html>
3636
<head>
3737
<link href="${deployUrl}styles.css" rel="stylesheet">
3838
<link href="${deployUrl}theme.css" rel="stylesheet">
3939
</head>
40-
<body></body>
40+
<body>${bodyContent}</body>
4141
</html>`;
4242
};
4343

@@ -105,4 +105,32 @@ describe('InlineCriticalCssProcessor', () => {
105105
);
106106
expect(content).toContain('<style>body{margin:0}html{color:white}</style>');
107107
});
108+
109+
it('should process the inline `onload` handlers if a CSP nonce is specified', async () => {
110+
const inlineCssProcessor = new InlineCriticalCssProcessor({
111+
readAsset,
112+
});
113+
114+
const { content } = await inlineCssProcessor.process(
115+
getContent('', '<app ngCspNonce="{% nonce %}"></app>'),
116+
{
117+
outputPath: '/dist/',
118+
},
119+
);
120+
121+
expect(content).toContain(
122+
'<link href="styles.css" rel="stylesheet" media="print" ngCspMedia="all">',
123+
);
124+
expect(content).toContain(
125+
'<link href="theme.css" rel="stylesheet" media="print" ngCspMedia="all">',
126+
);
127+
// Nonces shouldn't be added inside the `noscript` tags.
128+
expect(content).toContain('<noscript><link rel="stylesheet" href="theme.css"></noscript>');
129+
expect(content).toContain('<script nonce="{% nonce %}">');
130+
expect(tags.stripIndents`${content}`).toContain(tags.stripIndents`
131+
<style nonce="{% nonce %}">
132+
body { margin: 0; }
133+
html { color: white; }
134+
</style>`);
135+
});
108136
});

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /