Skip to content

Commit 4ee910b

Browse files
sabberwormsabberworm
authored
Selector validation (#162)
Selector validation
2 parents a90142f + a27e301 commit 4ee910b

File tree

9 files changed

+178
-12
lines changed

9 files changed

+178
-12
lines changed

‎lib/Sabberworm/CSS/CSSList/CSSList.php‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -83,16 +83,18 @@ private static function parseListItem(ParserState $oParserState, CSSList $oList)
8383
}
8484
return $oAtRule;
8585
} else if ($oParserState->comes('}')) {
86-
$oParserState->consume('}');
87-
if ($bIsRoot) {
88-
if ($oParserState->getSettings()->bLenientParsing) {
89-
while ($oParserState->comes('}')) $oParserState->consume('}');
90-
return DeclarationBlock::parse($oParserState);
86+
if (!$oParserState->getSettings()->bLenientParsing) {
87+
throw new UnexpectedTokenException('CSS selector', '}', 'identifier', $oParserState->currentLine());
88+
} else {
89+
if ($bIsRoot) {
90+
if ($oParserState->getSettings()->bLenientParsing) {
91+
return DeclarationBlock::parse($oParserState);
92+
} else {
93+
throw new SourceException("Unopened {", $oParserState->currentLine());
94+
}
9195
} else {
92-
throw new SourceException("Unopened {", $oParserState->currentLine());
96+
return null;
9397
}
94-
} else {
95-
return null;
9698
}
9799
} else {
98100
return DeclarationBlock::parse($oParserState);
@@ -123,6 +125,9 @@ private static function parseAtRule(ParserState $oParserState) {
123125
$oResult->setVendorKeyFrame($sIdentifier);
124126
$oResult->setAnimationName(trim($oParserState->consumeUntil('{', false, true)));
125127
CSSList::parseList($oParserState, $oResult);
128+
if ($oParserState->comes('}')) {
129+
$oParserState->consume('}');
130+
}
126131
return $oResult;
127132
} else if ($sIdentifier === 'namespace') {
128133
$sPrefix = null;
@@ -162,6 +167,9 @@ private static function parseAtRule(ParserState $oParserState) {
162167
} else {
163168
$oAtRule = new AtRuleBlockList($sIdentifier, $sArgs, $iIdentifierLineNum);
164169
CSSList::parseList($oParserState, $oAtRule);
170+
if ($oParserState->comes('}')) {
171+
$oParserState->consume('}');
172+
}
165173
}
166174
return $oAtRule;
167175
}
@@ -264,6 +272,9 @@ public function removeDeclarationBlockBySelector($mSelector, $bRemoveAll = false
264272
}
265273
foreach ($mSelector as $iKey => &$mSel) {
266274
if (!($mSel instanceof Selector)) {
275+
if (!Selector::isValid($mSel)) {
276+
throw new UnexpectedTokenException("Selector did not match '" . Selector::SELECTOR_VALIDATION_RX . "'.", $mSel, "custom");
277+
}
267278
$mSel = new Selector($mSel);
268279
}
269280
}

‎lib/Sabberworm/CSS/Property/Selector.php‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
namespace Sabberworm\CSS\Property;
44

5+
use Sabberworm\CSS\Parsing\UnexpectedTokenException;
6+
57
/**
68
* Class representing a single CSS selector. Selectors have to be split by the comma prior to being passed into this class.
79
*/
@@ -35,9 +37,23 @@ class Selector {
3537
))
3638
/ix';
3739

40+
const SELECTOR_VALIDATION_RX = '/
41+
^(
42+
(?:
43+
[a-zA-Z0-9\x{00A0}-\x{FFFF}_^$|*="\'~\[\]()\-\s\.:#+>]* # any sequence of valid unescaped characters
44+
(?:\\\\.)? # a single escaped character
45+
(?:([\'"]).*?(?<!\\\\)\2)? # a quoted text like [id="example"]
46+
)*
47+
)$
48+
/ux';
49+
3850
private $sSelector;
3951
private $iSpecificity;
4052

53+
public static function isValid($sSelector) {
54+
return preg_match(self::SELECTOR_VALIDATION_RX, $sSelector);
55+
}
56+
4157
public function __construct($sSelector, $bCalculateSpecificity = false) {
4258
$this->setSelector($sSelector);
4359
if ($bCalculateSpecificity) {

‎lib/Sabberworm/CSS/RuleSet/DeclarationBlock.php‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use Sabberworm\CSS\Parsing\ParserState;
66
use Sabberworm\CSS\Parsing\OutputException;
7+
use Sabberworm\CSS\Parsing\UnexpectedTokenException;
78
use Sabberworm\CSS\Property\Selector;
89
use Sabberworm\CSS\Rule\Rule;
910
use Sabberworm\CSS\Value\RuleValueList;
@@ -28,7 +29,33 @@ public function __construct($iLineNo = 0) {
2829
public static function parse(ParserState $oParserState) {
2930
$aComments = array();
3031
$oResult = new DeclarationBlock($oParserState->currentLine());
31-
$oResult->setSelector($oParserState->consumeUntil('{', false, true, $aComments));
32+
try {
33+
$aSelectorParts = array();
34+
$sStringWrapperChar = false;
35+
do {
36+
$aSelectorParts[] = $oParserState->consume(1) . $oParserState->consumeUntil(array('{', '}', '\'', '"'), false, false, $aComments);
37+
if ( in_array($oParserState->peek(), array('\'', '"')) && substr(end($aSelectorParts), -1) != "\\" ) {
38+
if ( $sStringWrapperChar === false ) {
39+
$sStringWrapperChar = $oParserState->peek();
40+
} else if ($sStringWrapperChar == $oParserState->peek()) {
41+
$sStringWrapperChar = false;
42+
}
43+
}
44+
} while (!in_array($oParserState->peek(), array('{', '}')) || $sStringWrapperChar !== false);
45+
$oResult->setSelector(implode('', $aSelectorParts));
46+
if ($oParserState->comes('{')) {
47+
$oParserState->consume(1);
48+
}
49+
} catch (UnexpectedTokenException $e) {
50+
if($oParserState->getSettings()->bLenientParsing) {
51+
if(!$oParserState->comes('}')) {
52+
$oParserState->consumeUntil('}', false, true);
53+
}
54+
return false;
55+
} else {
56+
throw $e;
57+
}
58+
}
3259
$oResult->setComments($aComments);
3360
RuleSet::parseRuleSet($oParserState, $oResult);
3461
return $oResult;
@@ -43,6 +70,9 @@ public function setSelectors($mSelector) {
4370
}
4471
foreach ($this->aSelectors as $iKey => $mSelector) {
4572
if (!($mSelector instanceof Selector)) {
73+
if (!Selector::isValid($mSelector)) {
74+
throw new UnexpectedTokenException("Selector did not match '" . Selector::SELECTOR_VALIDATION_RX . "'.", $mSelector, "custom");
75+
}
4676
$this->aSelectors[$iKey] = new Selector($mSelector);
4777
}
4878
}

‎tests/Sabberworm/CSS/ParserTest.php‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,47 @@ function testUnmatchedBracesInFile() {
432432
$this->assertSame($sExpected, $oDoc->render());
433433
}
434434

435+
function testInvalidSelectorsInFile() {
436+
$oDoc = $this->parsedStructureForFile('invalid-selectors', Settings::create()->withMultibyteSupport(true));
437+
$sExpected = '@keyframes mymove {from {top: 0px;}}
438+
#test {color: white;background: green;}
439+
#test {display: block;background: white;color: black;}';
440+
$this->assertSame($sExpected, $oDoc->render());
441+
442+
$oDoc = $this->parsedStructureForFile('invalid-selectors-2', Settings::create()->withMultibyteSupport(true));
443+
$sExpected = '@media only screen and (max-width: 1215px) {.breadcrumb {padding-left: 10px;}
444+
.super-menu > li:first-of-type {border-left-width: 0;}
445+
.super-menu > li:last-of-type {border-right-width: 0;}
446+
html[dir="rtl"] .super-menu > li:first-of-type {border-left-width: 1px;border-right-width: 0;}
447+
html[dir="rtl"] .super-menu > li:last-of-type {border-left-width: 0;}}
448+
body {background-color: red;}';
449+
$this->assertSame($sExpected, $oDoc->render());
450+
}
451+
452+
function testSelectorEscapesInFile() {
453+
$oDoc = $this->parsedStructureForFile('selector-escapes', Settings::create()->withMultibyteSupport(true));
454+
$sExpected = '#\# {color: red;}
455+
.col-sm-1\/5 {width: 20%;}';
456+
$this->assertSame($sExpected, $oDoc->render());
457+
458+
$oDoc = $this->parsedStructureForFile('invalid-selectors-2', Settings::create()->withMultibyteSupport(true));
459+
$sExpected = '@media only screen and (max-width: 1215px) {.breadcrumb {padding-left: 10px;}
460+
.super-menu > li:first-of-type {border-left-width: 0;}
461+
.super-menu > li:last-of-type {border-right-width: 0;}
462+
html[dir="rtl"] .super-menu > li:first-of-type {border-left-width: 1px;border-right-width: 0;}
463+
html[dir="rtl"] .super-menu > li:last-of-type {border-left-width: 0;}}
464+
body {background-color: red;}';
465+
$this->assertSame($sExpected, $oDoc->render());
466+
}
467+
468+
function testSelectorIgnoresInFile() {
469+
$oDoc = $this->parsedStructureForFile('selector-ignores', Settings::create()->withMultibyteSupport(true));
470+
$sExpected = '.some[selectors-may=\'contain-a-{\'] {}
471+
.this-selector .valid {width: 100px;}
472+
@media only screen and (min-width: 200px) {.test {prop: val;}}';
473+
$this->assertSame($sExpected, $oDoc->render());
474+
}
475+
435476
/**
436477
* @expectedException Sabberworm\CSS\Parsing\UnexpectedTokenException
437478
*/
@@ -501,7 +542,7 @@ function testCharsetFailure2() {
501542
* @expectedException \Sabberworm\CSS\Parsing\SourceException
502543
*/
503544
function testUnopenedClosingBracketFailure() {
504-
$this->parsedStructureForFile('unopened-close-brackets', Settings::create()->withLenientParsing(false));
545+
$this->parsedStructureForFile('-unopened-close-brackets', Settings::create()->withLenientParsing(false));
505546
}
506547

507548
/**

tests/files/unopened-close-brackets.css renamed to tests/files/-unopened-close-brackets.css

File renamed without changes.

‎tests/files/invalid-selectors-2.css‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
@media only screen and (max-width: 1215px) {
2+
.breadcrumb{
3+
padding-left:10px;
4+
}
5+
.super-menu > li:first-of-type{
6+
border-left-width:0;
7+
}
8+
.super-menu > li:last-of-type{
9+
border-right-width:0;
10+
}
11+
html[dir="rtl"] .super-menu > li:first-of-type{
12+
border-left-width:1px;
13+
border-right-width:0;
14+
}
15+
html[dir="rtl"] .super-menu > li:last-of-type{
16+
border-left-width:0;
17+
}
18+
html[dir="rtl"] .super-menu.menu-floated > li:first-of-type
19+
border-right-width:0;
20+
}
21+
}
22+
23+
24+
.super-menu.menu-floated{
25+
border-right-width:1px;
26+
border-left-width:1px;
27+
border-color:rgb(90, 66, 66);
28+
border-style:dotted;
29+
}
30+
31+
body {
32+
background-color: red;
33+
}

‎tests/files/invalid-selectors.css‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
@keyframes mymove {
2+
from { top: 0px; }
3+
}
4+
5+
#test {
6+
color: white;
7+
background: green;
8+
}
9+
10+
body
11+
background: black;
12+
}
13+
14+
#test {
15+
display: block;
16+
background: red;
17+
color: white;
18+
}
19+
#test {
20+
display: block;
21+
background: white;
22+
color: black;
23+
}
24+

‎tests/files/selector-escapes.css‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
#\# {
2+
color: red;
3+
}
4+
5+
.col-sm-1\/5 {
6+
width: 20%;
7+
}

tests/files/-tobedone.css renamed to tests/files/selector-ignores.css

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
11
.some[selectors-may='contain-a-{'] {
2-
2+
3+
}
4+
5+
.this-selector /* should remain-} */ .valid {
6+
width:100px;
37
}
48

59
@media only screen and (min-width: 200px) {
610
.test {
711
prop: val;
812
}
9-
}
13+
}

0 commit comments

Comments
 (0)