From 7f689a291a42de8327d4ff4e02a30969fb0a89c4 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 5 Sep 2020 11:30:54 +0200 Subject: [PATCH] fix(compiler): incorrectly encapsulating @import containing colons and semicolons (#38716) At a high level, the current shadow DOM shim logic works by escaping the content of a CSS rule (e.g. `div {color: red;}` becomes `div {%BLOCK%}`), using a regex to parse out things like the selector and the rule body, and then re-adding the content after the selector has been modified. The problem is that the regex has to be very broad in order capture all of the different use cases, which can cause it to match strings suffixed with a semi-colon in some places where it shouldn't, like this URL from Google Fonts `https://fonts.googleapis.com/css2?family=Roboto:wght@400;500&display=swap`. Most of the time this is fine, because the logic that escapes the rule content to `%BLOCK%` will have converted it to something that won't be matched by the regex. However, it breaks down for rules like `@import` which don't have a body, but can still have quoted content with characters that can match the regex. These changes resolve the issue by making a second pass over the escaped string and replacing all of the remaining quoted content with `%QUOTED%` before parsing it with the regex. Once everything has been processed, we make a final pass where we restore the quoted content. In a previous iteration of this PR, I went with a shorter approach which narrowed down the regex so that it doesn't capture rules without a body. It fixed the issue, but it also ended up breaking some of the more contrived unit test cases. I decided not to pursue it further, because we would've ended up with a very long and brittle regex that likely would've broken in even weirder ways. Fixes #38587. PR Close #38716 --- packages/compiler/src/shadow_css.ts | 99 +++++++++++++---------- packages/compiler/test/shadow_css_spec.ts | 40 +++++++++ 2 files changed, 98 insertions(+), 41 deletions(-) diff --git a/packages/compiler/src/shadow_css.ts b/packages/compiler/src/shadow_css.ts index 554add3039..72463ca817 100644 --- a/packages/compiler/src/shadow_css.ts +++ b/packages/compiler/src/shadow_css.ts @@ -555,66 +555,83 @@ function extractCommentsWithHash(input: string): string[] { return input.match(_commentWithHashRe) || []; } -const _ruleRe = /(\s*)([^;\{\}]+?)(\s*)((?:{%BLOCK%}?\s*;?)|(?:\s*;))/g; -const _curlyRe = /([{}])/g; -const OPEN_CURLY = '{'; -const CLOSE_CURLY = '}'; const BLOCK_PLACEHOLDER = '%BLOCK%'; +const QUOTE_PLACEHOLDER = '%QUOTED%'; +const _ruleRe = /(\s*)([^;\{\}]+?)(\s*)((?:{%BLOCK%}?\s*;?)|(?:\s*;))/g; +const _quotedRe = /%QUOTED%/g; +const CONTENT_PAIRS = new Map([['{', '}']]); +const QUOTE_PAIRS = new Map([[`"`, `"`], [`'`, `'`]]); export class CssRule { constructor(public selector: string, public content: string) {} } export function processRules(input: string, ruleCallback: (rule: CssRule) => CssRule): string { - const inputWithEscapedBlocks = escapeBlocks(input); + const inputWithEscapedQuotes = escapeBlocks(input, QUOTE_PAIRS, QUOTE_PLACEHOLDER); + const inputWithEscapedBlocks = + escapeBlocks(inputWithEscapedQuotes.escapedString, CONTENT_PAIRS, BLOCK_PLACEHOLDER); let nextBlockIndex = 0; - return inputWithEscapedBlocks.escapedString.replace(_ruleRe, function(...m: string[]) { - const selector = m[2]; - let content = ''; - let suffix = m[4]; - let contentPrefix = ''; - if (suffix && suffix.startsWith('{' + BLOCK_PLACEHOLDER)) { - content = inputWithEscapedBlocks.blocks[nextBlockIndex++]; - suffix = suffix.substring(BLOCK_PLACEHOLDER.length + 1); - contentPrefix = '{'; - } - const rule = ruleCallback(new CssRule(selector, content)); - return `${m[1]}${rule.selector}${m[3]}${contentPrefix}${rule.content}${suffix}`; - }); + let nextQuoteIndex = 0; + return inputWithEscapedBlocks.escapedString + .replace( + _ruleRe, + (...m: string[]) => { + const selector = m[2]; + let content = ''; + let suffix = m[4]; + let contentPrefix = ''; + if (suffix && suffix.startsWith('{' + BLOCK_PLACEHOLDER)) { + content = inputWithEscapedBlocks.blocks[nextBlockIndex++]; + suffix = suffix.substring(BLOCK_PLACEHOLDER.length + 1); + contentPrefix = '{'; + } + const rule = ruleCallback(new CssRule(selector, content)); + return `${m[1]}${rule.selector}${m[3]}${contentPrefix}${rule.content}${suffix}`; + }) + .replace(_quotedRe, () => inputWithEscapedQuotes.blocks[nextQuoteIndex++]); } class StringWithEscapedBlocks { constructor(public escapedString: string, public blocks: string[]) {} } -function escapeBlocks(input: string): StringWithEscapedBlocks { - const inputParts = input.split(_curlyRe); +function escapeBlocks( + input: string, charPairs: Map, placeholder: string): StringWithEscapedBlocks { const resultParts: string[] = []; const escapedBlocks: string[] = []; - let bracketCount = 0; - let currentBlockParts: string[] = []; - for (let partIndex = 0; partIndex < inputParts.length; partIndex++) { - const part = inputParts[partIndex]; - if (part == CLOSE_CURLY) { - bracketCount--; - } - if (bracketCount > 0) { - currentBlockParts.push(part); - } else { - if (currentBlockParts.length > 0) { - escapedBlocks.push(currentBlockParts.join('')); - resultParts.push(BLOCK_PLACEHOLDER); - currentBlockParts = []; + let openCharCount = 0; + let nonBlockStartIndex = 0; + let blockStartIndex = -1; + let openChar: string|undefined; + let closeChar: string|undefined; + for (let i = 0; i < input.length; i++) { + const char = input[i]; + if (char === '\\') { + i++; + } else if (char === closeChar) { + openCharCount--; + if (openCharCount === 0) { + escapedBlocks.push(input.substring(blockStartIndex, i)); + resultParts.push(placeholder); + nonBlockStartIndex = i; + blockStartIndex = -1; + openChar = closeChar = undefined; } - resultParts.push(part); - } - if (part == OPEN_CURLY) { - bracketCount++; + } else if (char === openChar) { + openCharCount++; + } else if (openCharCount === 0 && charPairs.has(char)) { + openChar = char; + closeChar = charPairs.get(char); + openCharCount = 1; + blockStartIndex = i + 1; + resultParts.push(input.substring(nonBlockStartIndex, blockStartIndex)); } } - if (currentBlockParts.length > 0) { - escapedBlocks.push(currentBlockParts.join('')); - resultParts.push(BLOCK_PLACEHOLDER); + if (blockStartIndex !== -1) { + escapedBlocks.push(input.substring(blockStartIndex)); + resultParts.push(placeholder); + } else { + resultParts.push(input.substring(nonBlockStartIndex)); } return new StringWithEscapedBlocks(resultParts.join(''), escapedBlocks); } diff --git a/packages/compiler/test/shadow_css_spec.ts b/packages/compiler/test/shadow_css_spec.ts index 67d525cfbb..ea9c771972 100644 --- a/packages/compiler/test/shadow_css_spec.ts +++ b/packages/compiler/test/shadow_css_spec.ts @@ -283,6 +283,28 @@ import {normalizeCSS} from '@angular/platform-browser/testing/src/browser_util'; expect(css).toEqual('@import url("a"); div[contenta] {}'); }); + it('should shim rules with quoted content after @import', () => { + const styleStr = '@import url("a"); div {background-image: url("a.jpg"); color: red;}'; + const css = s(styleStr, 'contenta'); + expect(css).toEqual( + '@import url("a"); div[contenta] {background-image:url("a.jpg"); color:red;}'); + }); + + it('should pass through @import directives whose URL contains colons and semicolons', () => { + const styleStr = + '@import url("https://fonts.googleapis.com/css2?family=Roboto:wght@400;500&display=swap");'; + const css = s(styleStr, 'contenta'); + expect(css).toEqual(styleStr); + }); + + it('should shim rules after @import with colons and semicolons', () => { + const styleStr = + '@import url("https://fonts.googleapis.com/css2?family=Roboto:wght@400;500&display=swap"); div {}'; + const css = s(styleStr, 'contenta'); + expect(css).toEqual( + '@import url("https://fonts.googleapis.com/css2?family=Roboto:wght@400;500&display=swap"); div[contenta] {}'); + }); + it('should leave calc() unchanged', () => { const styleStr = 'div {height:calc(100% - 55px);}'; const css = s(styleStr, 'contenta'); @@ -312,6 +334,24 @@ import {normalizeCSS} from '@angular/platform-browser/testing/src/browser_util'; expect(s('/*# sourceMappingURL=data:x */b {c}/*# sourceURL=xxx */', 'contenta')) .toEqual('b[contenta] {c}/*# sourceMappingURL=data:x *//*# sourceURL=xxx */'); }); + + it('should shim rules with quoted content', () => { + const styleStr = 'div {background-image: url("a.jpg"); color: red;}'; + const css = s(styleStr, 'contenta'); + expect(css).toEqual('div[contenta] {background-image:url("a.jpg"); color:red;}'); + }); + + it('should shim rules with an escaped quote inside quoted content', () => { + const styleStr = 'div::after { content: "\\"" }'; + const css = s(styleStr, 'contenta'); + expect(css).toEqual('div[contenta]::after { content:"\\""}'); + }); + + it('should shim rules with curly braces inside quoted content', () => { + const styleStr = 'div::after { content: "{}" }'; + const css = s(styleStr, 'contenta'); + expect(css).toEqual('div[contenta]::after { content:"{}"}'); + }); }); describe('processRules', () => {