diff --git a/Gruntfile.js b/Gruntfile.js index 5fcdaace420..a2b2be79ffb 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -27,6 +27,7 @@ module.exports = function( grunt ) { // ⚠️ Warning: These paths are passed straight to rm command in the shell, without any escaping. const productionVendorExcludedFilePatterns = [ 'composer.*', + 'patches', 'vendor/*/*/.editorconfig', 'vendor/*/*/.gitignore', 'vendor/*/*/composer.*', @@ -150,6 +151,7 @@ module.exports = function( grunt ) { paths.push( 'assets/js/*.js' ); // @todo Also include *.map files? paths.push( 'assets/js/*.asset.php' ); paths.push( 'assets/css/*.css' ); + paths.push( 'patches/*.patch' ); grunt.config.set( 'copy', { build: { diff --git a/composer.json b/composer.json index 2f8a153b631..dc14159440d 100644 --- a/composer.json +++ b/composer.json @@ -41,7 +41,8 @@ "extra": { "patches": { "sabberworm/php-css-parser": { - "PHP-CSS-Parser: Fix parsing CSS selectors which contain commas ": "https://github.com/sabberworm/PHP-CSS-Parser/commit/fa139f65c5b098ae652c970b25e6eb03fc495eb4.diff" + "Fix parsing CSS selectors which contain commas ": "https://github.com/sabberworm/PHP-CSS-Parser/commit/fa139f65c5b098ae652c970b25e6eb03fc495eb4.diff", + "Validate name-start code points for identifier ": "patches/php-css-parser-pull-185.patch" } } }, diff --git a/includes/sanitizers/class-amp-style-sanitizer.php b/includes/sanitizers/class-amp-style-sanitizer.php index 203b142ae7a..388c0633f58 100644 --- a/includes/sanitizers/class-amp-style-sanitizer.php +++ b/includes/sanitizers/class-amp-style-sanitizer.php @@ -1441,7 +1441,7 @@ private function get_parsed_stylesheet( $stylesheet, $options = [] ) { $parsed = null; $cache_key = null; $cached = true; - $cache_group = 'amp-parsed-stylesheet-v24'; // This should be bumped whenever the PHP-CSS-Parser is updated or parsed format is updated. + $cache_group = 'amp-parsed-stylesheet-v25'; // This should be bumped whenever the PHP-CSS-Parser is updated or parsed format is updated. $cache_impacting_options = array_merge( wp_array_slice_assoc( diff --git a/patches/php-css-parser-pull-185.patch b/patches/php-css-parser-pull-185.patch new file mode 100644 index 00000000000..4f14fae4a1d --- /dev/null +++ b/patches/php-css-parser-pull-185.patch @@ -0,0 +1,364 @@ +From d42b64793f2edaffeb663c63e9de79069cdc0831 Mon Sep 17 00:00:00 2001 +From: Pierre Gordon +Date: Wed, 22 Jan 2020 01:00:18 -0500 +Subject: [PATCH 1/5] Validate name-start code points for identifier + +--- + lib/Sabberworm/CSS/Parsing/ParserState.php | 40 +++++++++++++++++----- + lib/Sabberworm/CSS/Value/Color.php | 2 +- + tests/Sabberworm/CSS/ParserTest.php | 13 +++++-- + tests/files/-invalid-identifier.css | 6 ++++ + 4 files changed, 48 insertions(+), 13 deletions(-) + create mode 100644 tests/files/-invalid-identifier.css + +diff --git a/lib/Sabberworm/CSS/Parsing/ParserState.php b/lib/Sabberworm/CSS/Parsing/ParserState.php +index ad79820..1914f22 100644 +--- a/lib/Sabberworm/CSS/Parsing/ParserState.php ++++ b/lib/Sabberworm/CSS/Parsing/ParserState.php +@@ -48,8 +48,30 @@ public function getSettings() { + return $this->oParserSettings; + } + +- public function parseIdentifier($bIgnoreCase = true) { +- $sResult = $this->parseCharacter(true); ++ public function parseIdentifier($bIgnoreCase = true, $bNameStartCodePoint = true) { ++ $sResult = null; ++ $bCanParseCharacter = true; ++ ++ if ( $bNameStartCodePoint ) { ++ // Check if 3 code points would start an identifier. See . ++ $sNameStartCodePoint = '[a-zA-Z_]|[\x80-\xFF}]'; ++ $sEscapeCode = '\\[^\r\n\f]'; ++ ++ if ( ++ ! ( ++ preg_match("/-([-${sNameStartCodePoint}]|${sEscapeCode})/isSu", $this->peek(3)) || ++ preg_match("/${sNameStartCodePoint}/isSu", $this->peek()) || ++ preg_match("/${sEscapeCode}/isS", $this->peek(2)) ++ ) ++ ) { ++ $bCanParseCharacter = false; ++ } ++ } ++ ++ if ( $bCanParseCharacter ) { ++ $sResult = $this->parseCharacter(true); ++ } ++ + if ($sResult === null) { + throw new UnexpectedTokenException($sResult, $this->peek(5), 'identifier', $this->iLineNo); + } +@@ -97,13 +119,13 @@ public function parseCharacter($bIsForIdentifier) { + } + if ($bIsForIdentifier) { + $peek = ord($this->peek()); +- // Ranges: a-z A-Z 0-9 - _ ++ // Matches a name code point. See . + if (($peek >= 97 && $peek <= 122) || + ($peek >= 65 && $peek <= 90) || + ($peek >= 48 && $peek <= 57) || + ($peek === 45) || + ($peek === 95) || +- ($peek > 0xa1)) { ++ ($peek > 0x81)) { + return $this->consume(1); + } + } else { +@@ -261,22 +283,22 @@ public function strlen($sString) { + return mb_strlen($sString, $this->sCharset); + } else { + return strlen($sString); +- } +- } ++ } ++ } + + private function substr($iStart, $iLength) { + if ($iLength < 0) { + $iLength = $this->iLength - $iStart + $iLength; +- } ++ } + if ($iStart + $iLength > $this->iLength) { + $iLength = $this->iLength - $iStart; +- } ++ } + $sResult = ''; + while ($iLength > 0) { + $sResult .= $this->aText[$iStart]; + $iStart++; + $iLength--; +- } ++ } + return $sResult; + } + +diff --git a/lib/Sabberworm/CSS/Value/Color.php b/lib/Sabberworm/CSS/Value/Color.php +index c6ed9b1..f02777f 100644 +--- a/lib/Sabberworm/CSS/Value/Color.php ++++ b/lib/Sabberworm/CSS/Value/Color.php +@@ -14,7 +14,7 @@ public static function parse(ParserState $oParserState) { + $aColor = array(); + if ($oParserState->comes('#')) { + $oParserState->consume('#'); +- $sValue = $oParserState->parseIdentifier(false); ++ $sValue = $oParserState->parseIdentifier(false, false); + if ($oParserState->strlen($sValue) === 3) { + $sValue = $sValue[0] . $sValue[0] . $sValue[1] . $sValue[1] . $sValue[2] . $sValue[2]; + } else if ($oParserState->strlen($sValue) === 4) { +diff --git a/tests/Sabberworm/CSS/ParserTest.php b/tests/Sabberworm/CSS/ParserTest.php +index ea34f2e..16cae89 100644 +--- a/tests/Sabberworm/CSS/ParserTest.php ++++ b/tests/Sabberworm/CSS/ParserTest.php +@@ -214,7 +214,7 @@ function testManipulation() { + $this->assertSame('#header {margin: 10px 2em 1cm 2%;color: red !important;frequency: 30Hz;} + body {color: green;}', $oDoc->render()); + } +- ++ + function testRuleGetters() { + $oDoc = $this->parsedStructureForFile('values'); + $aBlocks = $oDoc->getAllDeclarationBlocks(); +@@ -319,7 +319,7 @@ function testNamespaces() { + |test {gaga: 2;}'; + $this->assertSame($sExpected, $oDoc->render()); + } +- ++ + function testInnerColors() { + $oDoc = $this->parsedStructureForFile('inner-color'); + $sExpected = 'test {background: -webkit-gradient(linear,0 0,0 bottom,from(#006cad),to(hsl(202,100%,49%)));}'; +@@ -359,7 +359,7 @@ function testListValueRemoval() { + $this->assertSame('@media screen {html {some: -test(val2);}} + #unrelated {other: yes;}', $oDoc->render()); + } +- ++ + /** + * @expectedException Sabberworm\CSS\Parsing\OutputException + */ +@@ -766,4 +766,11 @@ function testLonelyImport() { + $sExpected = "@import url(\"example.css\") only screen and (max-width: 600px);"; + $this->assertSame($sExpected, $oDoc->render()); + } ++ ++ /** ++ * @expectedException \Sabberworm\CSS\Parsing\UnexpectedTokenException ++ */ ++ function testInvalidIdentifier() { ++ $this->parsedStructureForFile('-invalid-identifier', Settings::create()->withLenientParsing(false)); ++ } + } +diff --git a/tests/files/-invalid-identifier.css b/tests/files/-invalid-identifier.css +new file mode 100644 +index 0000000..da00caf +--- /dev/null ++++ b/tests/files/-invalid-identifier.css +@@ -0,0 +1,6 @@ ++body { ++ transition: all .3s ease-in-out; ++ -webkit-transition: all .3s ease-in-out; ++ -moz-transition: all .3s ease-in-out; ++ -0-transition: all .3s ease-in-out; ++} + +From e031394fe3fc4448ed7e625e0c2b4ab334ad4ba2 Mon Sep 17 00:00:00 2001 +From: Pierre Gordon +Date: Sun, 26 Jan 2020 00:56:31 -0500 +Subject: [PATCH 2/5] Make validation of identifier more strict + +--- + lib/Sabberworm/CSS/Parsing/ParserState.php | 8 +++---- + tests/Sabberworm/CSS/ParserTest.php | 26 +++++++++++++++++++--- + tests/files/-invalid-identifier.css | 6 ----- + 3 files changed, 27 insertions(+), 13 deletions(-) + delete mode 100644 tests/files/-invalid-identifier.css + +diff --git a/lib/Sabberworm/CSS/Parsing/ParserState.php b/lib/Sabberworm/CSS/Parsing/ParserState.php +index 1914f22..2271d03 100644 +--- a/lib/Sabberworm/CSS/Parsing/ParserState.php ++++ b/lib/Sabberworm/CSS/Parsing/ParserState.php +@@ -54,14 +54,14 @@ public function parseIdentifier($bIgnoreCase = true, $bNameStartCodePoint = true + + if ( $bNameStartCodePoint ) { + // Check if 3 code points would start an identifier. See . +- $sNameStartCodePoint = '[a-zA-Z_]|[\x80-\xFF}]'; ++ $sNameStartCodePoint = '[a-zA-Z_]|[\x80-\xFF]'; + $sEscapeCode = '\\[^\r\n\f]'; + + if ( + ! ( +- preg_match("/-([-${sNameStartCodePoint}]|${sEscapeCode})/isSu", $this->peek(3)) || +- preg_match("/${sNameStartCodePoint}/isSu", $this->peek()) || +- preg_match("/${sEscapeCode}/isS", $this->peek(2)) ++ preg_match("/^-([-${sNameStartCodePoint}]|${sEscapeCode})/isSu", $this->peek(3)) || ++ preg_match("/^${sNameStartCodePoint}/isSu", $this->peek()) || ++ preg_match("/^${sEscapeCode}/isS", $this->peek(2)) + ) + ) { + $bCanParseCharacter = false; +diff --git a/tests/Sabberworm/CSS/ParserTest.php b/tests/Sabberworm/CSS/ParserTest.php +index 16cae89..921209e 100644 +--- a/tests/Sabberworm/CSS/ParserTest.php ++++ b/tests/Sabberworm/CSS/ParserTest.php +@@ -767,10 +767,30 @@ function testLonelyImport() { + $this->assertSame($sExpected, $oDoc->render()); + } + ++ function getInvalidIdentifiers() { ++ return array( ++ array( ++ 'body { -0-transition: all .3s ease-in-out; }', ++ 'Identifier expected. Got “-0-tr” [line no: 1]' ++ ), ++ array( ++ 'body { 4-o-transition: all .3s ease-in-out; }', ++ 'Identifier expected. Got “4-o-t” [line no: 1]' ++ ) ++ ); ++ } ++ + /** +- * @expectedException \Sabberworm\CSS\Parsing\UnexpectedTokenException ++ * @dataProvider getInvalidIdentifiers + */ +- function testInvalidIdentifier() { +- $this->parsedStructureForFile('-invalid-identifier', Settings::create()->withLenientParsing(false)); ++ function testInvalidIdentifier($css, $errorMessage) { ++ try { ++ $settings = Settings::create()->withLenientParsing(false); ++ $parser = new Parser($css, $settings); ++ $parser->parse(); ++ $this->fail( 'UnexpectedTokenException not thrown' ); ++ } catch ( UnexpectedTokenException $e ) { ++ $this->assertEquals( $errorMessage, $e->getMessage() ); ++ } + } + } +diff --git a/tests/files/-invalid-identifier.css b/tests/files/-invalid-identifier.css +deleted file mode 100644 +index da00caf..0000000 +--- a/tests/files/-invalid-identifier.css ++++ /dev/null +@@ -1,6 +0,0 @@ +-body { +- transition: all .3s ease-in-out; +- -webkit-transition: all .3s ease-in-out; +- -moz-transition: all .3s ease-in-out; +- -0-transition: all .3s ease-in-out; +-} + +From 8fbd0fe82aa08ad2650def1b44f2f77154211e30 Mon Sep 17 00:00:00 2001 +From: Pierre Gordon +Date: Sun, 26 Jan 2020 01:08:21 -0500 +Subject: [PATCH 3/5] Refactor `testInvalidIdentifier` test + +--- + tests/Sabberworm/CSS/ParserTest.php | 27 ++++++++++----------------- + 1 file changed, 10 insertions(+), 17 deletions(-) + +diff --git a/tests/Sabberworm/CSS/ParserTest.php b/tests/Sabberworm/CSS/ParserTest.php +index 921209e..68284ce 100644 +--- a/tests/Sabberworm/CSS/ParserTest.php ++++ b/tests/Sabberworm/CSS/ParserTest.php +@@ -769,28 +769,21 @@ function testLonelyImport() { + + function getInvalidIdentifiers() { + return array( +- array( +- 'body { -0-transition: all .3s ease-in-out; }', +- 'Identifier expected. Got “-0-tr” [line no: 1]' +- ), +- array( +- 'body { 4-o-transition: all .3s ease-in-out; }', +- 'Identifier expected. Got “4-o-t” [line no: 1]' +- ) ++ array('body { -0-transition: all .3s ease-in-out; }' ), ++ array('body { 4-o-transition: all .3s ease-in-out; }' ), + ); + } + + /** + * @dataProvider getInvalidIdentifiers ++ * ++ * @param string $css CSS text. + */ +- function testInvalidIdentifier($css, $errorMessage) { +- try { +- $settings = Settings::create()->withLenientParsing(false); +- $parser = new Parser($css, $settings); +- $parser->parse(); +- $this->fail( 'UnexpectedTokenException not thrown' ); +- } catch ( UnexpectedTokenException $e ) { +- $this->assertEquals( $errorMessage, $e->getMessage() ); +- } ++ function testInvalidIdentifier($css) { ++ $this->setExpectedException( 'Sabberworm\CSS\Parsing\UnexpectedTokenException' ); ++ ++ $oSettings = Settings::create()->withLenientParsing(false); ++ $oParser = new Parser($css, $oSettings); ++ $oParser->parse(); + } + } + +From 586c684a990458d70af55b47f584b619ad5c3a41 Mon Sep 17 00:00:00 2001 +From: Pierre Gordon +Date: Fri, 31 Jan 2020 15:55:54 -0500 +Subject: [PATCH 4/5] Recover from invalid identifier if in lenient mode + +--- + lib/Sabberworm/CSS/Parsing/ParserState.php | 2 +- + tests/Sabberworm/CSS/ParserTest.php | 4 ++-- + 2 files changed, 3 insertions(+), 3 deletions(-) + +diff --git a/lib/Sabberworm/CSS/Parsing/ParserState.php b/lib/Sabberworm/CSS/Parsing/ParserState.php +index 2271d03..7ab8e01 100644 +--- a/lib/Sabberworm/CSS/Parsing/ParserState.php ++++ b/lib/Sabberworm/CSS/Parsing/ParserState.php +@@ -72,7 +72,7 @@ public function parseIdentifier($bIgnoreCase = true, $bNameStartCodePoint = true + $sResult = $this->parseCharacter(true); + } + +- if ($sResult === null) { ++ if (!$this->oParserSettings->bLenientParsing && $sResult === null) { + throw new UnexpectedTokenException($sResult, $this->peek(5), 'identifier', $this->iLineNo); + } + $sCharacter = null; +diff --git a/tests/Sabberworm/CSS/ParserTest.php b/tests/Sabberworm/CSS/ParserTest.php +index 68284ce..ff8c5c9 100644 +--- a/tests/Sabberworm/CSS/ParserTest.php ++++ b/tests/Sabberworm/CSS/ParserTest.php +@@ -769,8 +769,8 @@ function testLonelyImport() { + + function getInvalidIdentifiers() { + return array( +- array('body { -0-transition: all .3s ease-in-out; }' ), +- array('body { 4-o-transition: all .3s ease-in-out; }' ), ++ array('body { -0-transition: all .3s ease-in-out; }'), ++ array('body { 4-o-transition: all .3s ease-in-out; }'), + ); + } + + +From 113df5d55e94e21c6402021dfa959924941d4c29 Mon Sep 17 00:00:00 2001 +From: Pierre Gordon +Date: Fri, 14 Feb 2020 04:20:16 -0500 +Subject: [PATCH 5/5] Remove check of lenient parsing + +The thrown exception will be caught when in lenient mode +--- + lib/Sabberworm/CSS/Parsing/ParserState.php | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/lib/Sabberworm/CSS/Parsing/ParserState.php b/lib/Sabberworm/CSS/Parsing/ParserState.php +index 7ab8e01..2271d03 100644 +--- a/lib/Sabberworm/CSS/Parsing/ParserState.php ++++ b/lib/Sabberworm/CSS/Parsing/ParserState.php +@@ -72,7 +72,7 @@ public function parseIdentifier($bIgnoreCase = true, $bNameStartCodePoint = true + $sResult = $this->parseCharacter(true); + } + +- if (!$this->oParserSettings->bLenientParsing && $sResult === null) { ++ if ($sResult === null) { + throw new UnexpectedTokenException($sResult, $this->peek(5), 'identifier', $this->iLineNo); + } + $sCharacter = null; diff --git a/tests/php/test-amp-style-sanitizer.php b/tests/php/test-amp-style-sanitizer.php index 4a9de4c131d..cd834d08989 100644 --- a/tests/php/test-amp-style-sanitizer.php +++ b/tests/php/test-amp-style-sanitizer.php @@ -1055,6 +1055,12 @@ public function get_amp_css_hacks_data() { [ '.selector:not([attr*=\'\']) {}', ], + [ + 'body { -0-transition: all .3s ease-in-out; }', + ], + [ + 'body { 4-o-transition: all .3s ease-in-out; }', + ], ]; }