From e4e817cc1c4dfe3b5fabbb9ebfb9f21ad93fbbe5 Mon Sep 17 00:00:00 2001 From: raxbg Date: Tue, 12 Nov 2019 13:12:43 +0200 Subject: [PATCH 1/9] Improve EOF handling + handle lonely import statements --- lib/Sabberworm/CSS/CSSList/CSSList.php | 36 +++++++++++++++---- lib/Sabberworm/CSS/Parsing/ParserState.php | 33 ++++++++++------- .../CSS/Parsing/UnexpectedEOFException.php | 9 +++++ tests/Sabberworm/CSS/ParserTest.php | 6 ++++ tests/files/lonely-import.css | 1 + 5 files changed, 66 insertions(+), 19 deletions(-) create mode 100644 lib/Sabberworm/CSS/Parsing/UnexpectedEOFException.php create mode 100644 tests/files/lonely-import.css diff --git a/lib/Sabberworm/CSS/CSSList/CSSList.php b/lib/Sabberworm/CSS/CSSList/CSSList.php index 5def89a0..6931adfb 100644 --- a/lib/Sabberworm/CSS/CSSList/CSSList.php +++ b/lib/Sabberworm/CSS/CSSList/CSSList.php @@ -6,6 +6,7 @@ use Sabberworm\CSS\Parsing\ParserState; use Sabberworm\CSS\Parsing\SourceException; use Sabberworm\CSS\Parsing\UnexpectedTokenException; +use Sabberworm\CSS\Parsing\UnexpectedEOFException; use Sabberworm\CSS\Property\AtRule; use Sabberworm\CSS\Property\Charset; use Sabberworm\CSS\Property\CSSNamespace; @@ -110,15 +111,34 @@ private static function parseAtRule(ParserState $oParserState) { $oLocation = URL::parse($oParserState); $oParserState->consumeWhiteSpace(); $sMediaQuery = null; - if (!$oParserState->comes(';')) { - $sMediaQuery = $oParserState->consumeUntil(';'); + try { + if (!$oParserState->comes(';')) { + $sMediaQuery = $oParserState->consumeUntil(';'); + } + $oParserState->consume(';'); + } catch (UnexpectedEOFException $e) { + // Save the media query if present + $sMediaQuery = ''; + try { + while (!$oParserState->isEnd()) { + $sMediaQuery .= $oParserState->consume(1); + } + } catch (UnexpectedEOFException $e) {} + + $sMediaQuery = trim($sMediaQuery); + if ($sMediaQuery === '') { + $sMediaQuery = null; + } } - $oParserState->consume(';'); return new Import($oLocation, $sMediaQuery, $iIdentifierLineNum); } else if ($sIdentifier === 'charset') { $sCharset = CSSString::parse($oParserState); - $oParserState->consumeWhiteSpace(); - $oParserState->consume(';'); + try { + $oParserState->consumeWhiteSpace(); + $oParserState->consume(';'); + } catch (UnexpectedEOFException $e) { + // Nothing fatal, file ended before ; was found + } return new Charset($sCharset, $iIdentifierLineNum); } else if (self::identifierIs($sIdentifier, 'keyframes')) { $oResult = new KeyFrame($iIdentifierLineNum); @@ -136,7 +156,11 @@ private static function parseAtRule(ParserState $oParserState) { $sPrefix = $mUrl; $mUrl = Value::parsePrimitiveValue($oParserState); } - $oParserState->consume(';'); + try { + $oParserState->consume(';'); + } catch (UnexpectedEOFException $e) { + // Nothing fatal, file ended before ; was found + } if ($sPrefix !== null && !is_string($sPrefix)) { throw new UnexpectedTokenException('Wrong namespace prefix', $sPrefix, 'custom', $iIdentifierLineNum); } diff --git a/lib/Sabberworm/CSS/Parsing/ParserState.php b/lib/Sabberworm/CSS/Parsing/ParserState.php index 4305c9a0..23d6c27a 100644 --- a/lib/Sabberworm/CSS/Parsing/ParserState.php +++ b/lib/Sabberworm/CSS/Parsing/ParserState.php @@ -3,6 +3,7 @@ use Sabberworm\CSS\Comment\Comment; use Sabberworm\CSS\Parsing\UnexpectedTokenException; +use Sabberworm\CSS\Parsing\UnexpectedEOFException; use Sabberworm\CSS\Settings; class ParserState { @@ -158,7 +159,7 @@ public function consume($mValue = 1) { return $mValue; } else { if ($this->iCurrentPosition + $mValue > $this->iLength) { - throw new UnexpectedTokenException($mValue, $this->peek(5), 'count', $this->iLineNo); + throw new UnexpectedEOFException($mValue, $this->peek(5), 'count', $this->iLineNo); } $sResult = $this->substr($this->iCurrentPosition, $mValue); $iLineCount = substr_count($sResult, "\n"); @@ -212,19 +213,25 @@ public function consumeUntil($aEnd, $bIncludeEnd = false, $consumeEnd = false, a $out = ''; $start = $this->iCurrentPosition; - while (($char = $this->consume(1)) !== '') { - if (in_array($char, $aEnd)) { - if ($bIncludeEnd) { - $out .= $char; - } elseif (!$consumeEnd) { - $this->iCurrentPosition -= $this->strlen($char); + try { + while (($char = $this->consume(1)) !== '') { + if (in_array($char, $aEnd)) { + if ($bIncludeEnd) { + $out .= $char; + } elseif (!$consumeEnd) { + $this->iCurrentPosition -= $this->strlen($char); + } + return $out; + } + $out .= $char; + if ($comment = $this->consumeComment()) { + $comments[] = $comment; } - return $out; - } - $out .= $char; - if ($comment = $this->consumeComment()) { - $comments[] = $comment; } + } catch (UnexpectedEOFException $e) { + // Reset the position and forward the EOF exception, so the caller can distinguish between EOF and the standard unexpected token error + $this->iCurrentPosition = $start; + throw $e; } $this->iCurrentPosition = $start; @@ -307,4 +314,4 @@ private function strpos($sString, $sNeedle, $iOffset) { return strpos($sString, $sNeedle, $iOffset); } } -} \ No newline at end of file +} diff --git a/lib/Sabberworm/CSS/Parsing/UnexpectedEOFException.php b/lib/Sabberworm/CSS/Parsing/UnexpectedEOFException.php new file mode 100644 index 00000000..e3d89489 --- /dev/null +++ b/lib/Sabberworm/CSS/Parsing/UnexpectedEOFException.php @@ -0,0 +1,9 @@ +assertSame($sExpected, $oDoc->render()); } + + function testLonelyImport() { + $oDoc = $this->parsedStructureForFile('lonely-import'); + $sExpected = "@import url(\"example.css\") only screen and (max-width: 600px);"; + $this->assertSame($sExpected, $oDoc->render()); + } } diff --git a/tests/files/lonely-import.css b/tests/files/lonely-import.css new file mode 100644 index 00000000..c21f0899 --- /dev/null +++ b/tests/files/lonely-import.css @@ -0,0 +1 @@ +@import "example.css" only screen and (max-width: 600px) From 48e99f65fd5abd79d353ed8462d19541adf99678 Mon Sep 17 00:00:00 2001 From: raxbg Date: Tue, 12 Nov 2019 19:33:41 +0200 Subject: [PATCH 2/9] Replace assumption with certainty for EOF when consuming white space --- lib/Sabberworm/CSS/Parsing/ParserState.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/Sabberworm/CSS/Parsing/ParserState.php b/lib/Sabberworm/CSS/Parsing/ParserState.php index 2934eac3..1e33b6fc 100644 --- a/lib/Sabberworm/CSS/Parsing/ParserState.php +++ b/lib/Sabberworm/CSS/Parsing/ParserState.php @@ -119,8 +119,7 @@ public function consumeWhiteSpace() { if($this->oParserSettings->bLenientParsing) { try { $oComment = $this->consumeComment(); - } catch(UnexpectedTokenException $e) { - // When we can’t find the end of a comment, we assume the document is finished. + } catch(UnexpectedEOFException $e) { $this->iCurrentPosition = $this->iLength; return; } From e795c88b7638f83df49d4fd9afe9ad4f3cb39ecc Mon Sep 17 00:00:00 2001 From: raxbg Date: Tue, 12 Nov 2019 20:01:17 +0200 Subject: [PATCH 3/9] Remove trailing newline from the lonely-import test file --- tests/files/lonely-import.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/files/lonely-import.css b/tests/files/lonely-import.css index c21f0899..87767bba 100644 --- a/tests/files/lonely-import.css +++ b/tests/files/lonely-import.css @@ -1 +1 @@ -@import "example.css" only screen and (max-width: 600px) +@import "example.css" only screen and (max-width: 600px) \ No newline at end of file From b50f2284698ad735b378a34adfac8c901d70c0a9 Mon Sep 17 00:00:00 2001 From: raxbg Date: Tue, 12 Nov 2019 20:02:16 +0200 Subject: [PATCH 4/9] Add ParserState::EOL and support it in consumeUntil --- lib/Sabberworm/CSS/CSSList/CSSList.php | 21 ++++----------------- lib/Sabberworm/CSS/Parsing/ParserState.php | 12 +++++++++--- 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/lib/Sabberworm/CSS/CSSList/CSSList.php b/lib/Sabberworm/CSS/CSSList/CSSList.php index 51cc892e..431499c5 100644 --- a/lib/Sabberworm/CSS/CSSList/CSSList.php +++ b/lib/Sabberworm/CSS/CSSList/CSSList.php @@ -111,24 +111,11 @@ private static function parseAtRule(ParserState $oParserState) { $oLocation = URL::parse($oParserState); $oParserState->consumeWhiteSpace(); $sMediaQuery = null; - try { - if (!$oParserState->comes(';')) { - $sMediaQuery = $oParserState->consumeUntil(';'); - } + if (!$oParserState->comes(';')) { + $sMediaQuery = $oParserState->consumeUntil(array(';', ParserState::EOF)); + } + if (!$oParserState->isEnd()) { $oParserState->consume(';'); - } catch (UnexpectedEOFException $e) { - // Save the media query if present - $sMediaQuery = ''; - try { - while (!$oParserState->isEnd()) { - $sMediaQuery .= $oParserState->consume(1); - } - } catch (UnexpectedEOFException $e) {} - - $sMediaQuery = trim($sMediaQuery); - if ($sMediaQuery === '') { - $sMediaQuery = null; - } } return new Import($oLocation, $sMediaQuery, $iIdentifierLineNum); } else if ($sIdentifier === 'charset') { diff --git a/lib/Sabberworm/CSS/Parsing/ParserState.php b/lib/Sabberworm/CSS/Parsing/ParserState.php index 1e33b6fc..5a2493eb 100644 --- a/lib/Sabberworm/CSS/Parsing/ParserState.php +++ b/lib/Sabberworm/CSS/Parsing/ParserState.php @@ -7,6 +7,8 @@ use Sabberworm\CSS\Settings; class ParserState { + const EOF = null; + private $oParserSettings; private $sText; @@ -230,9 +232,13 @@ public function consumeUntil($aEnd, $bIncludeEnd = false, $consumeEnd = false, a } } } catch (UnexpectedEOFException $e) { - // Reset the position and forward the EOF exception, so the caller can distinguish between EOF and the standard unexpected token error - $this->iCurrentPosition = $start; - throw $e; + if (in_array(self::EOF, $aEnd)) { + return $out; + } else { + // Reset the position and forward the EOF exception, so the caller can distinguish between EOF and the standard unexpected token error + $this->iCurrentPosition = $start; + throw $e; + } } $this->iCurrentPosition = $start; From 8db468cc78376091d0221488db4d6452efa77bb2 Mon Sep 17 00:00:00 2001 From: raxbg Date: Tue, 12 Nov 2019 20:09:24 +0200 Subject: [PATCH 5/9] Resolve https://github.com/sabberworm/PHP-CSS-Parser/pull/176#discussion_r345279363 --- lib/Sabberworm/CSS/Parsing/ParserState.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Sabberworm/CSS/Parsing/ParserState.php b/lib/Sabberworm/CSS/Parsing/ParserState.php index 5a2493eb..bb750dd1 100644 --- a/lib/Sabberworm/CSS/Parsing/ParserState.php +++ b/lib/Sabberworm/CSS/Parsing/ParserState.php @@ -217,7 +217,8 @@ public function consumeUntil($aEnd, $bIncludeEnd = false, $consumeEnd = false, a $start = $this->iCurrentPosition; try { - while (($char = $this->consume(1)) !== '') { + while (true) { + $char = $this->consume(1); if (in_array($char, $aEnd)) { if ($bIncludeEnd) { $out .= $char; From e0efd9fe8b55dbfa54351abe02ea3dc78e456aec Mon Sep 17 00:00:00 2001 From: raxbg Date: Tue, 12 Nov 2019 20:42:33 +0200 Subject: [PATCH 6/9] Resolve issues with using exceptions for flow control --- lib/Sabberworm/CSS/CSSList/CSSList.php | 12 ++------ lib/Sabberworm/CSS/Parsing/ParserState.php | 36 +++++++++------------- 2 files changed, 18 insertions(+), 30 deletions(-) diff --git a/lib/Sabberworm/CSS/CSSList/CSSList.php b/lib/Sabberworm/CSS/CSSList/CSSList.php index 431499c5..f15fd7b9 100644 --- a/lib/Sabberworm/CSS/CSSList/CSSList.php +++ b/lib/Sabberworm/CSS/CSSList/CSSList.php @@ -120,11 +120,9 @@ private static function parseAtRule(ParserState $oParserState) { return new Import($oLocation, $sMediaQuery, $iIdentifierLineNum); } else if ($sIdentifier === 'charset') { $sCharset = CSSString::parse($oParserState); - try { - $oParserState->consumeWhiteSpace(); + $oParserState->consumeWhiteSpace(); + if (!$oParserState->isEnd()) { $oParserState->consume(';'); - } catch (UnexpectedEOFException $e) { - // Nothing fatal, file ended before ; was found } return new Charset($sCharset, $iIdentifierLineNum); } else if (self::identifierIs($sIdentifier, 'keyframes')) { @@ -143,11 +141,7 @@ private static function parseAtRule(ParserState $oParserState) { $sPrefix = $mUrl; $mUrl = Value::parsePrimitiveValue($oParserState); } - try { - $oParserState->consume(';'); - } catch (UnexpectedEOFException $e) { - // Nothing fatal, file ended before ; was found - } + $oParserState->consume(';'); if ($sPrefix !== null && !is_string($sPrefix)) { throw new UnexpectedTokenException('Wrong namespace prefix', $sPrefix, 'custom', $iIdentifierLineNum); } diff --git a/lib/Sabberworm/CSS/Parsing/ParserState.php b/lib/Sabberworm/CSS/Parsing/ParserState.php index bb750dd1..1561c3b6 100644 --- a/lib/Sabberworm/CSS/Parsing/ParserState.php +++ b/lib/Sabberworm/CSS/Parsing/ParserState.php @@ -216,30 +216,24 @@ public function consumeUntil($aEnd, $bIncludeEnd = false, $consumeEnd = false, a $out = ''; $start = $this->iCurrentPosition; - try { - while (true) { - $char = $this->consume(1); - if (in_array($char, $aEnd)) { - if ($bIncludeEnd) { - $out .= $char; - } elseif (!$consumeEnd) { - $this->iCurrentPosition -= $this->strlen($char); - } - return $out; - } - $out .= $char; - if ($comment = $this->consumeComment()) { - $comments[] = $comment; + while (!$this->isEnd()) { + $char = $this->consume(1); + if (in_array($char, $aEnd)) { + if ($bIncludeEnd) { + $out .= $char; + } elseif (!$consumeEnd) { + $this->iCurrentPosition -= $this->strlen($char); } - } - } catch (UnexpectedEOFException $e) { - if (in_array(self::EOF, $aEnd)) { return $out; - } else { - // Reset the position and forward the EOF exception, so the caller can distinguish between EOF and the standard unexpected token error - $this->iCurrentPosition = $start; - throw $e; } + $out .= $char; + if ($comment = $this->consumeComment()) { + $comments[] = $comment; + } + } + + if (in_array(self::EOF, $aEnd)) { + return $out; } $this->iCurrentPosition = $start; From aad8e56c2eb9cf214264a0659034be61bff8e54f Mon Sep 17 00:00:00 2001 From: raxbg Date: Tue, 12 Nov 2019 20:44:39 +0200 Subject: [PATCH 7/9] Add forgotten check for document end --- lib/Sabberworm/CSS/CSSList/CSSList.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/Sabberworm/CSS/CSSList/CSSList.php b/lib/Sabberworm/CSS/CSSList/CSSList.php index f15fd7b9..3d6c87e3 100644 --- a/lib/Sabberworm/CSS/CSSList/CSSList.php +++ b/lib/Sabberworm/CSS/CSSList/CSSList.php @@ -141,7 +141,9 @@ private static function parseAtRule(ParserState $oParserState) { $sPrefix = $mUrl; $mUrl = Value::parsePrimitiveValue($oParserState); } - $oParserState->consume(';'); + if (!$oParserState->isEnd()) { + $oParserState->consume(';'); + } if ($sPrefix !== null && !is_string($sPrefix)) { throw new UnexpectedTokenException('Wrong namespace prefix', $sPrefix, 'custom', $iIdentifierLineNum); } From 8b62f7fdd88c6c65e8f05c1c6d273be0f905e9b5 Mon Sep 17 00:00:00 2001 From: raxbg Date: Tue, 12 Nov 2019 20:46:50 +0200 Subject: [PATCH 8/9] Trim the Import's media query --- lib/Sabberworm/CSS/CSSList/CSSList.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Sabberworm/CSS/CSSList/CSSList.php b/lib/Sabberworm/CSS/CSSList/CSSList.php index 3d6c87e3..5e99fa65 100644 --- a/lib/Sabberworm/CSS/CSSList/CSSList.php +++ b/lib/Sabberworm/CSS/CSSList/CSSList.php @@ -112,7 +112,7 @@ private static function parseAtRule(ParserState $oParserState) { $oParserState->consumeWhiteSpace(); $sMediaQuery = null; if (!$oParserState->comes(';')) { - $sMediaQuery = $oParserState->consumeUntil(array(';', ParserState::EOF)); + $sMediaQuery = trim($oParserState->consumeUntil(array(';', ParserState::EOF))); } if (!$oParserState->isEnd()) { $oParserState->consume(';'); From 8ba132573cfc751ada35af03862e44b8242cee39 Mon Sep 17 00:00:00 2001 From: raxbg Date: Tue, 12 Nov 2019 21:16:04 +0200 Subject: [PATCH 9/9] Convert some more code to make use of ParserState::EOF and UnexpectedEOFException --- lib/Sabberworm/CSS/CSSList/CSSList.php | 12 +++--------- lib/Sabberworm/CSS/Parsing/ParserState.php | 2 +- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/lib/Sabberworm/CSS/CSSList/CSSList.php b/lib/Sabberworm/CSS/CSSList/CSSList.php index 5e99fa65..debf9122 100644 --- a/lib/Sabberworm/CSS/CSSList/CSSList.php +++ b/lib/Sabberworm/CSS/CSSList/CSSList.php @@ -114,16 +114,12 @@ private static function parseAtRule(ParserState $oParserState) { if (!$oParserState->comes(';')) { $sMediaQuery = trim($oParserState->consumeUntil(array(';', ParserState::EOF))); } - if (!$oParserState->isEnd()) { - $oParserState->consume(';'); - } + $oParserState->consumeUntil(array(';', ParserState::EOF), true, true); return new Import($oLocation, $sMediaQuery, $iIdentifierLineNum); } else if ($sIdentifier === 'charset') { $sCharset = CSSString::parse($oParserState); $oParserState->consumeWhiteSpace(); - if (!$oParserState->isEnd()) { - $oParserState->consume(';'); - } + $oParserState->consumeUntil(array(';', ParserState::EOF), true, true); return new Charset($sCharset, $iIdentifierLineNum); } else if (self::identifierIs($sIdentifier, 'keyframes')) { $oResult = new KeyFrame($iIdentifierLineNum); @@ -141,9 +137,7 @@ private static function parseAtRule(ParserState $oParserState) { $sPrefix = $mUrl; $mUrl = Value::parsePrimitiveValue($oParserState); } - if (!$oParserState->isEnd()) { - $oParserState->consume(';'); - } + $oParserState->consumeUntil(array(';', ParserState::EOF), true, true); if ($sPrefix !== null && !is_string($sPrefix)) { throw new UnexpectedTokenException('Wrong namespace prefix', $sPrefix, 'custom', $iIdentifierLineNum); } diff --git a/lib/Sabberworm/CSS/Parsing/ParserState.php b/lib/Sabberworm/CSS/Parsing/ParserState.php index 1561c3b6..ad79820a 100644 --- a/lib/Sabberworm/CSS/Parsing/ParserState.php +++ b/lib/Sabberworm/CSS/Parsing/ParserState.php @@ -237,7 +237,7 @@ public function consumeUntil($aEnd, $bIncludeEnd = false, $consumeEnd = false, a } $this->iCurrentPosition = $start; - throw new UnexpectedTokenException('One of ("'.implode('","', $aEnd).'")', $this->peek(5), 'search', $this->iLineNo); + throw new UnexpectedEOFException('One of ("'.implode('","', $aEnd).'")', $this->peek(5), 'search', $this->iLineNo); } private function inputLeft() {