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

Lines changed: 102 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,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‎

Lines changed: 23 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,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)

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