From 21bd501440454614dd0436b18e1049e0a581d205 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Sat, 9 Jun 2018 15:56:03 +0100 Subject: [PATCH] Fix whitelist case sensitiveness (#222) Closes #215 --- .gitignore | 3 +- .../NodeVisitor/ConstStmtReplacer.php | 2 +- .../NodeVisitor/NameStmtPrefixer.php | 2 +- .../NodeVisitor/StringScalarPrefixer.php | 24 ++++++- src/Whitelist.php | 68 +++++++++++++++---- tests/WhitelistTest.php | 60 +++++++++++++--- 6 files changed, 131 insertions(+), 28 deletions(-) diff --git a/.gitignore b/.gitignore index 48c9ddf9..d608d0c5 100644 --- a/.gitignore +++ b/.gitignore @@ -7,7 +7,8 @@ /clover.xml /dist/ /fixtures/set004/scoper.inc.php -/fixtures/*/vendor +/fixtures/*/.box_dump/ +/fixtures/*/vendor/ /vendor/ /vendor-bin/*/vendor/ /vendor-bin/*/bin/ diff --git a/src/PhpParser/NodeVisitor/ConstStmtReplacer.php b/src/PhpParser/NodeVisitor/ConstStmtReplacer.php index 11cef991..25d6fcbd 100644 --- a/src/PhpParser/NodeVisitor/ConstStmtReplacer.php +++ b/src/PhpParser/NodeVisitor/ConstStmtReplacer.php @@ -72,7 +72,7 @@ public function enterNode(Node $node): Node ) )->getName(); - if (false === $this->whitelist->isClassWhitelisted((string) $resolvedConstantName)) { + if (false === $this->whitelist->isConstantWhitelisted((string) $resolvedConstantName)) { continue; } diff --git a/src/PhpParser/NodeVisitor/NameStmtPrefixer.php b/src/PhpParser/NodeVisitor/NameStmtPrefixer.php index 62d496c5..7df40ee9 100644 --- a/src/PhpParser/NodeVisitor/NameStmtPrefixer.php +++ b/src/PhpParser/NodeVisitor/NameStmtPrefixer.php @@ -160,7 +160,7 @@ private function prefixName(Name $name): Node } if ($parentNode instanceof ConstFetch) { - if ($this->whitelist->isClassWhitelisted($resolvedName->toString())) { + if ($this->whitelist->isConstantWhitelisted($resolvedName->toString())) { return $resolvedName; } diff --git a/src/PhpParser/NodeVisitor/StringScalarPrefixer.php b/src/PhpParser/NodeVisitor/StringScalarPrefixer.php index 71924850..2859e3e5 100644 --- a/src/PhpParser/NodeVisitor/StringScalarPrefixer.php +++ b/src/PhpParser/NodeVisitor/StringScalarPrefixer.php @@ -109,6 +109,8 @@ private function prefixStringScalar(String_ $string): Node $string->getAttributes() ); + $isConstantNode = $this->isConstantNode($string); + // Skip if is already prefixed if ($this->prefix === $stringName->getFirst()) { $newStringName = $stringName; @@ -117,7 +119,8 @@ private function prefixStringScalar(String_ $string): Node } elseif ( 1 === count($stringName->parts) || $this->reflector->isClassInternal($stringName->toString()) - || $this->whitelist->isClassWhitelisted((string) $stringName) + || (false === $isConstantNode && $this->whitelist->isClassWhitelisted((string) $stringName)) + || ($isConstantNode && $this->whitelist->isConstantWhitelisted((string) $stringName)) || $this->whitelist->isNamespaceWhitelisted((string) $stringName) ) { $newStringName = $stringName; @@ -127,4 +130,23 @@ private function prefixStringScalar(String_ $string): Node return new String_($newStringName->toString(), $string->getAttributes()); } + + private function isConstantNode(String_ $node): bool + { + $parent = AppendParentNode::getParent($node); + + if (false === ($parent instanceof Arg)) { + return false; + } + + /** @var Arg $parent */ + $argParent = AppendParentNode::getParent($parent); + + if (false === ($argParent instanceof FuncCall)) { + return false; + } + + /* @var FuncCall $argParent */ + return 'define' === (string) $argParent->name; + } } diff --git a/src/Whitelist.php b/src/Whitelist.php index 3fbd8a72..ea8f9b27 100644 --- a/src/Whitelist.php +++ b/src/Whitelist.php @@ -16,25 +16,33 @@ use Countable; use InvalidArgumentException; +use function array_filter; use function array_map; -use function array_merge; +use function array_pop; use function array_unique; use function count; +use function explode; +use function implode; use function in_array; use function sprintf; +use function strtolower; use function substr; use function trim; final class Whitelist implements Countable { + private $original; private $classes; + private $constants; private $namespaces; private $whitelistGlobalConstants; public static function create(bool $whitelistGlobalConstants, string ...$elements): self { $classes = []; + $constants = []; $namespaces = []; + $original = []; foreach ($elements as $element) { if (isset($element[0]) && '\\' === $element[0]) { @@ -50,30 +58,44 @@ public static function create(bool $whitelistGlobalConstants, string ...$element ); } + $original[] = $element; + if ('\*' === substr($element, -2)) { - $namespaces[] = substr($element, 0, -2); + $namespaces[] = strtolower(substr($element, 0, -2)); } elseif ('*' === $element) { $namespaces[] = ''; } else { - $classes[] = $element; + $classes[] = strtolower($element); + $constants[] = self::lowerConstantName($element); } } return new self( $whitelistGlobalConstants, + array_unique($original), array_unique($classes), + array_unique($constants), array_unique($namespaces) ); } /** + * @param string[] $original * @param string[] $classes + * @param string[] $constants * @param string[] $namespaces */ - private function __construct(bool $whitelistGlobalConstants, array $classes, array $namespaces) - { + private function __construct( + bool $whitelistGlobalConstants, + array $original, + array $classes, + array $constants, + array $namespaces + ) { $this->whitelistGlobalConstants = $whitelistGlobalConstants; + $this->original = $original; $this->classes = $classes; + $this->constants = $constants; $this->namespaces = $namespaces; } @@ -84,7 +106,12 @@ public function whitelistGlobalConstants(): bool public function isClassWhitelisted(string $name): bool { - return in_array($name, $this->classes, true); + return in_array(strtolower($name), $this->classes, true); + } + + public function isConstantWhitelisted(string $name): bool + { + return in_array(self::lowerConstantName($name), $this->constants, true); } /** @@ -92,11 +119,18 @@ public function isClassWhitelisted(string $name): bool */ public function getClassWhitelistArray(): array { - return $this->classes; + return array_filter( + $this->original, + function (string $name): bool { + return '*' !== $name && '\*' !== substr($name, -2); + } + ); } public function isNamespaceWhitelisted(string $name): bool { + $name = strtolower($name); + foreach ($this->namespaces as $namespace) { if ('' === $namespace || 0 === strpos($name, $namespace)) { return true; @@ -116,13 +150,19 @@ public function count(): int public function toArray(): array { - $namespaces = array_map( - function (string $namespace): string { - return '' === $namespace ? '*' : $namespace.'\*'; - }, - $this->namespaces - ); + return $this->original; + } + + private static function lowerConstantName(string $name): string + { + $parts = explode('\\', $name); + + $lastPart = array_pop($parts); + + $parts = array_map('strtolower', $parts); + + $parts[] = $lastPart; - return array_merge($this->classes, $namespaces); + return implode('\\', $parts); } } diff --git a/tests/WhitelistTest.php b/tests/WhitelistTest.php index 7b4a947d..d903576c 100644 --- a/tests/WhitelistTest.php +++ b/tests/WhitelistTest.php @@ -27,8 +27,9 @@ class WhitelistTest extends TestCase */ public function test_it_can_be_created_from_a_list_of_strings( array $whitelist, + array $expectedNamespaces, array $expectedClasses, - array $expectedNamespaces + array $expectedConstants ) { $whitelistObject = Whitelist::create(true, ...$whitelist); @@ -40,15 +41,21 @@ public function test_it_can_be_created_from_a_list_of_strings( $whitelistNamespaceReflection->setAccessible(true); $actualNamespaces = $whitelistNamespaceReflection->getValue($whitelistObject); + $whitelistConstantReflection = $whitelistReflection->getProperty('constants'); + $whitelistConstantReflection->setAccessible(true); + $actualConstants = $whitelistConstantReflection->getValue($whitelistObject); + $this->assertTrue($whitelistObject->whitelistGlobalConstants()); - $this->assertSame($expectedClasses, $actualClasses); $this->assertSame($expectedNamespaces, $actualNamespaces); + $this->assertSame($expectedClasses, $actualClasses); + $this->assertSame($expectedConstants, $actualConstants); $whitelistObject = Whitelist::create(false, ...$whitelist); $this->assertFalse($whitelistObject->whitelistGlobalConstants()); $this->assertSame($expectedClasses, $actualClasses); $this->assertSame($expectedNamespaces, $actualNamespaces); + $this->assertSame($expectedConstants, $actualConstants); } /** @@ -83,19 +90,22 @@ public function test_it_can_be_converted_back_into_an_array(Whitelist $whitelist public function provideWhitelists() { - yield [[], [], []]; - - yield [['Acme\Foo'], ['Acme\Foo'], []]; + yield [[], [], [], []]; - yield [['\Acme\Foo'], ['Acme\Foo'], []]; + yield [['Acme\Foo'], [], ['Acme\Foo'], ['acme\Foo']]; - yield [['Acme\Foo\*'], [], ['Acme\Foo']]; + yield [['Acme\Foo\*'], ['acme\foo'], [], []]; - yield [['\*'], [], ['']]; + yield [['\*'], [''], [], []]; - yield [['*'], [], ['']]; + yield [['*'], [''], [], []]; - yield [['Acme\Foo', 'Acme\Foo\*', '\*'], ['Acme\Foo'], ['Acme\Foo', '']]; + yield [ + ['Acme\Foo', 'Acme\Foo\*', '\*'], + ['acme\foo', ''], + ['Acme\Foo'], + ['acme\Foo'], + ]; } public function provideClassWhitelists() @@ -151,29 +161,59 @@ public function provideNamespaceWhitelists() true, ]; + yield [ + Whitelist::create(true, 'Acme\Foo\*'), + 'acme\foo', + true, + ]; + yield [ Whitelist::create(true, 'Acme\*'), 'Acme\Foo', true, ]; + yield [ + Whitelist::create(true, 'Acme\*'), + 'acme\foo', + true, + ]; + yield [ Whitelist::create(true, 'Acme\Foo\*'), 'Acme\Foo\Bar', true, ]; + yield [ + Whitelist::create(true, 'Acme\Foo\*'), + 'acme\foo\bar', + true, + ]; + yield [ Whitelist::create(true, '\*'), 'Acme', true, ]; + yield [ + Whitelist::create(true, '\*'), + 'acme', + true, + ]; + yield [ Whitelist::create(true, '\*'), 'Acme\Foo', true, ]; + + yield [ + Whitelist::create(true, '\*'), + 'acme\foo', + true, + ]; } public function provideWhitelistToConvert()