Skip to content

Commit ab1415b

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 659baf7 commit ab1415b

File tree

2 files changed

+125
-2
lines changed

2 files changed

+125
-2
lines changed

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

+102
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,36 @@ 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+
removeAttribute(name: string): void;
38+
appendChild(child: PartialHTMLElement): void;
39+
textContent: string;
40+
}
41+
42+
/** Partial representation of an HTML `Document`. */
43+
interface PartialDocument {
44+
head: PartialHTMLElement;
45+
createElement(tagName: string): PartialHTMLElement;
46+
querySelector(selector: string): PartialHTMLElement | null;
47+
}
48+
49+
/** Signature of the `Critters.embedLinkedStylesheet` method. */
50+
type EmbedLinkedStylesheetFn = (
51+
link: PartialHTMLElement,
52+
document: PartialDocument,
53+
) => Promise<unknown>;
54+
2355
class CrittersExtended extends Critters {
2456
readonly warnings: string[] = [];
2557
readonly errors: string[] = [];
58+
private initialEmbedLinkedStylesheet: EmbedLinkedStylesheetFn;
59+
private addedCspScriptsDocuments = new WeakSet<PartialDocument>();
60+
61+
// Inherited from `Critters`, but not exposed in the typings.
62+
protected embedLinkedStylesheet!: EmbedLinkedStylesheetFn;
2663

2764
constructor(
2865
private readonly optionsExtended: InlineCriticalCssProcessorOptions &
@@ -41,17 +78,82 @@ class CrittersExtended extends Critters {
4178
pruneSource: false,
4279
reduceInlineStyles: false,
4380
mergeStylesheets: false,
81+
// Note: if `preload` changes to anything other than `media`, the logic in
82+
// `embedLinkedStylesheetOverride` will have to be updated.
4483
preload: 'media',
4584
noscriptFallback: true,
4685
inlineFonts: true,
4786
});
87+
88+
// We can't use inheritance to override `embedLinkedStylesheet`, because it's not declared in
89+
// the `Critters` .d.ts which means that we can't call the `super` implementation. TS doesn't
90+
// allow for `super` to be cast to a different type.
91+
this.initialEmbedLinkedStylesheet = this.embedLinkedStylesheet;
92+
this.embedLinkedStylesheet = this.embedLinkedStylesheetOverride;
4893
}
4994

5095
public override readFile(path: string): Promise<string> {
5196
const readAsset = this.optionsExtended.readAsset;
5297

5398
return readAsset ? readAsset(path) : fs.promises.readFile(path, 'utf-8');
5499
}
100+
101+
/**
102+
* Override of the Critters `embedLinkedStylesheet` method
103+
* that makes it work with Angular's CSP APIs.
104+
*/
105+
private embedLinkedStylesheetOverride: EmbedLinkedStylesheetFn = async (link, document) => {
106+
const returnValue = await this.initialEmbedLinkedStylesheet(link, document);
107+
const crittersMedia = link.getAttribute('onload')?.match(MEDIA_SET_HANDLER_PATTERN);
108+
109+
if (crittersMedia) {
110+
// HTML attribute are case-insensitive, but the parser
111+
// used by Critters appears to be case-sensitive.
112+
const nonceElement = document.querySelector('[ngCspNonce], [ngcspnonce]');
113+
const cspNonce =
114+
nonceElement?.getAttribute('ngCspNonce') || nonceElement?.getAttribute('ngcspnonce');
115+
116+
if (cspNonce) {
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+
127+
return returnValue;
128+
};
129+
130+
/**
131+
* Inserts the `script` tag that swaps the critical CSS at runtime,
132+
* if one hasn't been inserted into the document already.
133+
*/
134+
private conditionallyInsertCspLoadingScript(document: PartialDocument, nonce: string) {
135+
if (!this.addedCspScriptsDocuments.has(document)) {
136+
const script = document.createElement('script');
137+
script.setAttribute('nonce', nonce);
138+
script.textContent = [
139+
`(function() {`,
140+
// Save the `children` in a variable since they're a live DOM node collection.
141+
` var children = document.head.children;`,
142+
// Declare `onLoad` outside the loop to avoid leaking memory.
143+
// Can't be an arrow function, because we need `this` to refer to the DOM node.
144+
` function onLoad() {this.media = this.getAttribute('${CSP_MEDIA_ATTR}');}`,
145+
` for (var i = 0; i < children.length; i++) {`,
146+
` var child = children[i];`,
147+
` child.hasAttribute('${CSP_MEDIA_ATTR}') && child.addEventListener('load', onLoad);`,
148+
` }`,
149+
`})();`,
150+
].join('\n');
151+
// Append the script to the head since it needs to
152+
// run as early as possible, after the `link` tags.
153+
document.head.appendChild(script);
154+
this.addedCspScriptsDocuments.add(document);
155+
}
156+
}
55157
}
56158

57159
export class InlineCriticalCssProcessor {

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

+23-2
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,25 @@ 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 inlineFontsProcessor = new InlineCriticalCssProcessor({
111+
readAsset,
112+
});
113+
114+
const { content } = await inlineFontsProcessor.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+
expect(content).toContain('<script nonce="{% nonce %}">');
128+
});
108129
});

0 commit comments

Comments
 (0)