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 ad79820a..1914f22d 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 c6ed9b18..f02777f3 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 ea34f2e7..16cae895 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 00000000..da00caf8 --- /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 1914f22d..2271d03e 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 16cae895..921209ef 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 da00caf8..00000000 --- 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 921209ef..68284ce7 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 2271d03e..7ab8e019 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 68284ce7..ff8c5c92 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 7ab8e019..2271d03e 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;