From 928280e927ede3ff088699f90e7774a083c727a8 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Fri, 19 Apr 2024 13:56:00 -0400 Subject: [PATCH 1/4] Revert "fix: Incorrect parsing of functional pseudo class css selector (#169)" This reverts commit 810b39f6fa62d17f2389467121ddd11ae6aa1033. --- packages/rrweb-snapshot/src/css.ts | 87 +----------------------- packages/rrweb-snapshot/test/css.test.ts | 81 ---------------------- 2 files changed, 3 insertions(+), 165 deletions(-) diff --git a/packages/rrweb-snapshot/src/css.ts b/packages/rrweb-snapshot/src/css.ts index 41a66f8724..d7a413eb67 100644 --- a/packages/rrweb-snapshot/src/css.ts +++ b/packages/rrweb-snapshot/src/css.ts @@ -437,96 +437,15 @@ export function parse(css: string, options: ParserOptions = {}) { } /* @fix Remove all comments from selectors * http://ostermiller.org/findcomment.html */ - const splitSelectors = trim(m[0]) + return trim(m[0]) .replace(/\/\*([^*]|[\r\n]|(\*+([^*/]|[\r\n])))*\*\/+/g, '') .replace(/"(?:\\"|[^"])*"|'(?:\\'|[^'])*'/g, (m) => { return m.replace(/,/g, '\u200C'); }) - .split(/\s*(?![^(]*\)),\s*/); - - if (splitSelectors.length <= 1) { - return splitSelectors.map((s) => { + .split(/\s*(?![^(]*\)),\s*/) + .map((s) => { return s.replace(/\u200C/g, ','); }); - } - - // For each selector, need to check if we properly split on `,` - // Example case where selector is: - // .bar:has(input:is(:disabled), button:is(:disabled)) - let i = 0; - let j = 0; - const len = splitSelectors.length; - const finalSelectors = []; - while (i < len) { - // Look for selectors with opening parens - `(` and search rest of - // selectors for the first one with matching number of closing - // parens `)` - const openingParensCount = (splitSelectors[i].match(/\(/g) || []).length; - const closingParensCount = (splitSelectors[i].match(/\)/g) || []).length; - let unbalancedParens = openingParensCount - closingParensCount; - - if (unbalancedParens >= 1) { - // At least one opening parens was found, prepare to look through - // rest of selectors - let foundClosingSelector = false; - - // Loop starting with next item in array, until we find matching - // number of ending parens - j = i + 1; - while (j < len) { - // peek into next item to count the number of closing brackets - const nextOpeningParensCount = (splitSelectors[j].match(/\(/g) || []) - .length; - const nextClosingParensCount = (splitSelectors[j].match(/\)/g) || []) - .length; - const nextUnbalancedParens = - nextClosingParensCount - nextOpeningParensCount; - - if (nextUnbalancedParens === unbalancedParens) { - // Matching # of closing parens was found, join all elements - // from i to j - finalSelectors.push(splitSelectors.slice(i, j + 1).join(',')); - - // we will want to skip the items that we have joined together - i = j + 1; - - // Use to continue the outer loop - foundClosingSelector = true; - - // break out of inner loop so we found matching closing parens - break; - } - - // No matching closing parens found, keep moving through index, but - // update the # of unbalanced parents still outstanding - j++; - unbalancedParens -= nextUnbalancedParens; - } - - if (foundClosingSelector) { - // Matching closing selector was found, move to next selector - continue; - } - - // No matching closing selector was found, either invalid CSS, - // or unbalanced number of opening parens were used as CSS - // selectors. Assume that rest of the list of selectors are - // selectors and break to avoid iterating through the list of - // selectors again. - splitSelectors - .slice(i, len) - .forEach((selector) => selector && finalSelectors.push(selector)); - break; - } - - // No opening parens found, contiue looking through list - splitSelectors[i] && finalSelectors.push(splitSelectors[i]); - i++; - } - - return finalSelectors.map((s) => { - return s.replace(/\u200C/g, ','); - }); } /** diff --git a/packages/rrweb-snapshot/test/css.test.ts b/packages/rrweb-snapshot/test/css.test.ts index d6731db131..2818386071 100644 --- a/packages/rrweb-snapshot/test/css.test.ts +++ b/packages/rrweb-snapshot/test/css.test.ts @@ -119,87 +119,6 @@ describe('css parser', () => { expect(out3).toEqual('[data-aa\\:other] { color: red; }'); }); - it.each([ - ['.foo,.bar {}', ['.foo', '.bar']], - ['.bar:has(:disabled) {}', ['.bar:has(:disabled)']], - ['.bar:has(input, button) {}', ['.bar:has(input, button)']], - [ - '.bar:has(input:is(:disabled),button:has(:disabled)) {}', - ['.bar:has(input:is(:disabled),button:has(:disabled))'], - ], - [ - '.bar:has(div, input:is(:disabled), button) {}', - ['.bar:has(div,input:is(:disabled), button)'], - ], - [ - '.bar:has(div, input:is(:disabled),button:has(:disabled,.baz)) {}', - ['.bar:has(div,input:is(:disabled),button:has(:disabled,.baz))'], - ], - [ - '.bar:has(input), .foo:has(input, button), .baz {}', - ['.bar:has(input)', '.foo:has(input, button)', '.baz'], - ], - [ - '.bar:has(input:is(:disabled),button:has(:disabled,.baz), div:has(:disabled,.baz)){color: red;}', - [ - '.bar:has(input:is(:disabled),button:has(:disabled,.baz),div:has(:disabled,.baz))', - ], - ], - ['.bar((( {}', ['.bar(((']], - [ - '.bar:has(:has(:has(a), :has(:has(:has(b, :has(a), c), e))), input:is(:disabled), button) {}', - [ - '.bar:has(:has(:has(a),:has(:has(:has(b,:has(a), c), e))),input:is(:disabled), button)', - ], - ], - ['.foo,.bar(((,.baz {}', ['.foo', '.bar(((', '.baz']], - [ - '.foo,.bar:has(input:is(:disabled)){color: red;}', - ['.foo', '.bar:has(input:is(:disabled))'], - ], - [ - '.foo,.bar:has(input:is(:disabled),button:has(:disabled,.baz)){color: red;}', - ['.foo', '.bar:has(input:is(:disabled),button:has(:disabled,.baz))'], - ], - [ - '.foo,.bar:has(input:is(:disabled),button:has(:disabled), div:has(:disabled,.baz)){color: red;}', - [ - '.foo', - '.bar:has(input:is(:disabled),button:has(:disabled),div:has(:disabled,.baz))', - ], - ], - [ - '.foo,.bar:has(input:is(:disabled),button:has(:disabled,.baz), div:has(:disabled,.baz)){color: red;}', - [ - '.foo', - '.bar:has(input:is(:disabled),button:has(:disabled,.baz),div:has(:disabled,.baz))', - ], - ], - ['.bar:has(:disabled), .foo {}', ['.bar:has(:disabled)', '.foo']], - [ - '.bar:has(input:is(:disabled),.foo,button:is(:disabled)), .foo {}', - ['.bar:has(input:is(:disabled),.foo,button:is(:disabled))', '.foo'], - ], - [ - '.bar:has(input:is(:disabled),.foo,button:is(:disabled)), .foo:has(input, button), .baz, {}', - [ - '.bar:has(input:is(:disabled),.foo,button:is(:disabled))', - '.foo:has(input, button)', - '.baz', - ], - ], - ])( - 'can parse selector(s) with functional pseudo classes: %s', - (cssText, expected) => { - expect( - parse( - cssText, - // @ts-ignore - ).stylesheet?.rules[0].selectors, - ).toEqual(expected); - }, - ); - it('parses imports with quotes correctly', () => { const out1 = escapeImportStatement({ cssText: `@import url("/foo.css;900;800"");`, From 6a1362bfac788ea2910cc4608ec49e5c038a599e Mon Sep 17 00:00:00 2001 From: Daniel Engelke Date: Thu, 18 Apr 2024 16:50:20 +0800 Subject: [PATCH 2/4] Fix for test cases mentioned in #1379 (#1401) * Fix known issues * Run format * Fix linting errors * Add comment in code for source of match logic * Add changeset --- .changeset/rich-dots-lay.md | 5 ++ packages/rrweb-snapshot/src/css.ts | 62 +++++++++++++++++++++--- packages/rrweb-snapshot/test/css.test.ts | 29 +++++++++++ 3 files changed, 90 insertions(+), 6 deletions(-) create mode 100644 .changeset/rich-dots-lay.md diff --git a/.changeset/rich-dots-lay.md b/.changeset/rich-dots-lay.md new file mode 100644 index 0000000000..ba889e446e --- /dev/null +++ b/.changeset/rich-dots-lay.md @@ -0,0 +1,5 @@ +--- +'rrweb-snapshot': patch +--- + +Fix css parsing errors diff --git a/packages/rrweb-snapshot/src/css.ts b/packages/rrweb-snapshot/src/css.ts index d7a413eb67..82b4c41f96 100644 --- a/packages/rrweb-snapshot/src/css.ts +++ b/packages/rrweb-snapshot/src/css.ts @@ -431,21 +431,71 @@ export function parse(css: string, options: ParserOptions = {}) { */ function selector() { - const m = match(/^([^{]+)/); + whitespace(); + while (css[0] == '}') { + error('extra closing bracket'); + css = css.slice(1); + whitespace(); + } + + // Use match logic from https://github.com/NxtChg/pieces/blob/3eb39c8287a97632e9347a24f333d52d916bc816/js/css_parser/css_parse.js#L46C1-L47C1 + const m = match(/^(("(?:\\"|[^"])*"|'(?:\\'|[^'])*'|[^{])+)/); if (!m) { return; } /* @fix Remove all comments from selectors * http://ostermiller.org/findcomment.html */ - return trim(m[0]) + const cleanedInput = m[0] + .trim() .replace(/\/\*([^*]|[\r\n]|(\*+([^*/]|[\r\n])))*\*\/+/g, '') + + // Handle strings by replacing commas inside them .replace(/"(?:\\"|[^"])*"|'(?:\\'|[^'])*'/g, (m) => { return m.replace(/,/g, '\u200C'); - }) - .split(/\s*(?![^(]*\)),\s*/) - .map((s) => { - return s.replace(/\u200C/g, ','); }); + + // Split using a custom function and restore commas in strings + return customSplit(cleanedInput).map((s) => + s.replace(/\u200C/g, ',').trim(), + ); + } + + /** + * Split selector correctly, ensuring not to split on comma if inside (). + */ + + function customSplit(input: string) { + const result = []; + let currentSegment = ''; + let depthParentheses = 0; // Track depth of parentheses + let depthBrackets = 0; // Track depth of square brackets + + for (const char of input) { + if (char === '(') { + depthParentheses++; + } else if (char === ')') { + depthParentheses--; + } else if (char === '[') { + depthBrackets++; + } else if (char === ']') { + depthBrackets--; + } + + // Split point is a comma that is not inside parentheses or square brackets + if (char === ',' && depthParentheses === 0 && depthBrackets === 0) { + result.push(currentSegment); + currentSegment = ''; + } else { + currentSegment += char; + } + } + + // Add the last segment + if (currentSegment) { + result.push(currentSegment); + } + + return result; } /** diff --git a/packages/rrweb-snapshot/test/css.test.ts b/packages/rrweb-snapshot/test/css.test.ts index 2818386071..d3c9482a90 100644 --- a/packages/rrweb-snapshot/test/css.test.ts +++ b/packages/rrweb-snapshot/test/css.test.ts @@ -78,6 +78,35 @@ describe('css parser', () => { expect(errors[0].filename).toEqual('foo.css'); }); + it('should parse selector with comma nested inside ()', () => { + const result = parse( + '[_nghost-ng-c4172599085]:not(.fit-content).aim-select:hover:not(:disabled, [_nghost-ng-c4172599085]:not(.fit-content).aim-select--disabled, [_nghost-ng-c4172599085]:not(.fit-content).aim-select--invalid, [_nghost-ng-c4172599085]:not(.fit-content).aim-select--active) { border-color: rgb(84, 84, 84); }', + ); + + expect(result.parent).toEqual(null); + + const rules = result.stylesheet!.rules; + expect(rules.length).toEqual(1); + + let rule = rules[0] as Rule; + expect(rule.parent).toEqual(result); + expect(rule.selectors?.length).toEqual(1); + + let decl = rule.declarations![0]; + expect(decl.parent).toEqual(rule); + }); + + it('parses { and } in attribute selectors correctly', () => { + const result = parse('foo[someAttr~="{someId}"] { color: red; }'); + const rules = result.stylesheet!.rules; + + expect(rules.length).toEqual(1); + + const rule = rules[0] as Rule; + + expect(rule.selectors![0]).toEqual('foo[someAttr~="{someId}"]'); + }); + it('should set parent property', () => { const result = parse( 'thing { test: value; }\n' + From 020cfd9204598ea4a64b81a20f03673b8fb5561e Mon Sep 17 00:00:00 2001 From: David Newell Date: Thu, 18 Apr 2024 23:46:17 +0100 Subject: [PATCH 3/4] better splitting of selectors (#1440) * better splitting of selectors - overlapping with #1401 * Add test from example at https://github.com/PostHog/posthog/pull/21427 * ignore brackets inside selector strings * Add another test as noticed that it's possible to escape strings * Ensure we are ignoring commas within strings Co-authored-by: Eoghan Murray --- .changeset/modern-doors-watch.md | 5 +++ packages/rrweb-snapshot/src/css.ts | 12 ++++++- packages/rrweb-snapshot/test/css.test.ts | 44 ++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 .changeset/modern-doors-watch.md diff --git a/.changeset/modern-doors-watch.md b/.changeset/modern-doors-watch.md new file mode 100644 index 0000000000..b74bfbad45 --- /dev/null +++ b/.changeset/modern-doors-watch.md @@ -0,0 +1,5 @@ +--- +'rrweb-snapshot': patch +--- + +better nested css selector splitting when commas or brackets happen to be in quoted text diff --git a/packages/rrweb-snapshot/src/css.ts b/packages/rrweb-snapshot/src/css.ts index 82b4c41f96..ee36a4771e 100644 --- a/packages/rrweb-snapshot/src/css.ts +++ b/packages/rrweb-snapshot/src/css.ts @@ -443,6 +443,7 @@ export function parse(css: string, options: ParserOptions = {}) { if (!m) { return; } + /* @fix Remove all comments from selectors * http://ostermiller.org/findcomment.html */ const cleanedInput = m[0] @@ -469,9 +470,16 @@ export function parse(css: string, options: ParserOptions = {}) { let currentSegment = ''; let depthParentheses = 0; // Track depth of parentheses let depthBrackets = 0; // Track depth of square brackets + let currentStringChar = null; for (const char of input) { - if (char === '(') { + const hasStringEscape = currentSegment.endsWith('\\'); + + if (currentStringChar) { + if (currentStringChar === char && !hasStringEscape) { + currentStringChar = null; + } + } else if (char === '(') { depthParentheses++; } else if (char === ')') { depthParentheses--; @@ -479,6 +487,8 @@ export function parse(css: string, options: ParserOptions = {}) { depthBrackets++; } else if (char === ']') { depthBrackets--; + } else if ('\'"'.includes(char)) { + currentStringChar = char; } // Split point is a comma that is not inside parentheses or square brackets diff --git a/packages/rrweb-snapshot/test/css.test.ts b/packages/rrweb-snapshot/test/css.test.ts index d3c9482a90..6f10f6e569 100644 --- a/packages/rrweb-snapshot/test/css.test.ts +++ b/packages/rrweb-snapshot/test/css.test.ts @@ -148,6 +148,50 @@ describe('css parser', () => { expect(out3).toEqual('[data-aa\\:other] { color: red; }'); }); + it('parses nested commas in selectors correctly', () => { + const result = parse( + ` +body > ul :is(li:not(:first-of-type) a:hover, li:not(:first-of-type).active a) { + background: red; +} +`, + ); + expect((result.stylesheet!.rules[0] as Rule)!.selectors!.length).toEqual(1); + + const trickresult = parse( + ` +li[attr="weirdly("] a:hover, li[attr="weirdly)"] a { + background-color: red; +} +`, + ); + expect( + (trickresult.stylesheet!.rules[0] as Rule)!.selectors!.length, + ).toEqual(2); + + const weirderresult = parse( + ` +li[attr="weirder\\"("] a:hover, li[attr="weirder\\")"] a { + background-color: red; +} +`, + ); + expect( + (weirderresult.stylesheet!.rules[0] as Rule)!.selectors!.length, + ).toEqual(2); + + const commainstrresult = parse( + ` +li[attr="has,comma"] a:hover { + background-color: red; +} +`, + ); + expect( + (commainstrresult.stylesheet!.rules[0] as Rule)!.selectors!.length, + ).toEqual(1); + }); + it('parses imports with quotes correctly', () => { const out1 = escapeImportStatement({ cssText: `@import url("/foo.css;900;800"");`, From 4c76050b6bc9e5fdfc475d133da3ea45bf094e9f Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Fri, 19 Apr 2024 14:21:43 -0400 Subject: [PATCH 4/4] sentry: add existing sentry tests --- packages/rrweb-snapshot/test/css.test.ts | 79 ++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/packages/rrweb-snapshot/test/css.test.ts b/packages/rrweb-snapshot/test/css.test.ts index 6f10f6e569..ebd6586132 100644 --- a/packages/rrweb-snapshot/test/css.test.ts +++ b/packages/rrweb-snapshot/test/css.test.ts @@ -192,6 +192,85 @@ li[attr="has,comma"] a:hover { ).toEqual(1); }); + it.each([ + ['.foo,.bar {}', ['.foo', '.bar']], + ['.bar:has(:disabled) {}', ['.bar:has(:disabled)']], + ['.bar:has(input, button) {}', ['.bar:has(input, button)']], + [ + '.bar:has(input:is(:disabled),button:has(:disabled)) {}', + ['.bar:has(input:is(:disabled),button:has(:disabled))'], + ], + [ + '.bar:has(div, input:is(:disabled), button) {}', + ['.bar:has(div, input:is(:disabled), button)'], + ], + [ + '.bar:has(div, input:is(:disabled),button:has(:disabled,.baz)) {}', + ['.bar:has(div, input:is(:disabled),button:has(:disabled,.baz))'], + ], + [ + '.bar:has(input), .foo:has(input, button), .baz {}', + ['.bar:has(input)', '.foo:has(input, button)', '.baz'], + ], + [ + '.bar:has(input:is(:disabled),button:has(:disabled,.baz), div:has(:disabled,.baz)){color: red;}', + [ + '.bar:has(input:is(:disabled),button:has(:disabled,.baz), div:has(:disabled,.baz))', + ], + ], + [ + '.bar:has(:has(:has(a), :has(:has(:has(b, :has(a), c), e))), input:is(:disabled), button) {}', + [ + '.bar:has(:has(:has(a), :has(:has(:has(b, :has(a), c), e))), input:is(:disabled), button)', + ], + ], + [ + '.foo,.bar:has(input:is(:disabled)){color: red;}', + ['.foo', '.bar:has(input:is(:disabled))'], + ], + [ + '.foo,.bar:has(input:is(:disabled),button:has(:disabled,.baz)){color: red;}', + ['.foo', '.bar:has(input:is(:disabled),button:has(:disabled,.baz))'], + ], + [ + '.foo,.bar:has(input:is(:disabled),button:has(:disabled), div:has(:disabled,.baz)){color: red;}', + [ + '.foo', + '.bar:has(input:is(:disabled),button:has(:disabled), div:has(:disabled,.baz))', + ], + ], + [ + '.foo,.bar:has(input:is(:disabled),button:has(:disabled,.baz), div:has(:disabled,.baz)){color: red;}', + [ + '.foo', + '.bar:has(input:is(:disabled),button:has(:disabled,.baz), div:has(:disabled,.baz))', + ], + ], + ['.bar:has(:disabled), .foo {}', ['.bar:has(:disabled)', '.foo']], + [ + '.bar:has(input:is(:disabled),.foo,button:is(:disabled)), .foo {}', + ['.bar:has(input:is(:disabled),.foo,button:is(:disabled))', '.foo'], + ], + [ + '.bar:has(input:is(:disabled),.foo,button:is(:disabled)), .foo:has(input, button), .baz, {}', + [ + '.bar:has(input:is(:disabled),.foo,button:is(:disabled))', + '.foo:has(input, button)', + '.baz', + ], + ], + ])( + 'can parse selector(s) with functional pseudo classes: %s', + (cssText, expected) => { + expect( + parse( + cssText, + // @ts-ignore + ).stylesheet?.rules[0].selectors, + ).toEqual(expected); + }, + ); + it('parses imports with quotes correctly', () => { const out1 = escapeImportStatement({ cssText: `@import url("/foo.css;900;800"");`,