Merge pull request #12 from tgalopin/type-safety

Introduce Psalm, fix type issues and improve Travis config
tgalopin · Nov 26, 2018 · 479a577 · 479a577
2 parents 5a14699 + fba1ee8
commit 479a577
Show file tree

Hide file tree

Showing 28 changed files with 69 additions and 61 deletions.
diff --git a/.gitignore b/.gitignore
@@ -1,2 +1,3 @@
 vendor
 composer.lock
+.php_cs.cache
diff --git a/.php_cs.dist b/.php_cs.dist
@@ -0,0 +1,13 @@
+<?php
+
+$finder = PhpCsFixer\Finder::create()
+    ->in(__DIR__)
+;
+
+return PhpCsFixer\Config::create()
+    ->setRules(array(
+        '@Symfony' => true,
+        'phpdoc_annotation_without_dot' => false,
+    ))
+    ->setFinder($finder)
+;
diff --git a/.travis.yml b/.travis.yml
@@ -2,11 +2,13 @@ language: php
 dist: trusty
 sudo: false
 
-php:
-    - 7.1
-    - 7.2
-
 matrix:
+    include:
+        - php: 7.1
+          env: COMPOSER_FLAGS="--prefer-lowest"
+        - php: 7.2
+          env: CS_FIXER=1
+        - php: 7.3
     fast_finish: true
 
 cache:
@@ -18,7 +20,9 @@ before_install:
 
 before_script:
     - composer self-update
-    - composer install
+    - composer update $COMPOSER_FLAGS --prefer-dist
 
 script:
+    - stty cols 120
+    - if [ "$CS_FIXER" == 1 ]; then wget https://github.com/FriendsOfPHP/PHP-CS-Fixer/releases/download/v2.13.1/php-cs-fixer.phar && php php-cs-fixer.phar fix --dry-run --diff; fi
     - php vendor/bin/phpunit
diff --git a/benchmark/run.php b/benchmark/run.php
@@ -10,7 +10,7 @@
 
 echo "Running...\n";
 
-for ($i = 0; $i < $times; $i++) {
+for ($i = 0; $i < $times; ++$i) {
     $output = $sanitizer->sanitize($input);
 }
 

diff --git a/composer.json b/composer.json
@@ -24,7 +24,7 @@
         "ext-dom": "*",
         "masterminds/html5": "^2.4",
         "psr/log": "^1.0",
-        "league/uri-parser": "^1.4"
+        "league/uri-parser": "^1.4.1"
     },
     "require-dev": {
         "phpunit/phpunit": "^7.4",

diff --git a/src/DomVisitor.php b/src/DomVisitor.php
@@ -33,6 +33,9 @@ class DomVisitor implements DomVisitorInterface
      */
     private $reversedVisitors;
 
+    /**
+     * @param NodeVisitorInterface[] $visitors
+     */
     public function __construct(array $visitors = [])
     {
         $this->visitors = $visitors;
@@ -60,6 +63,7 @@ private function visitNode(\DOMNode $node, Cursor $cursor)
             }
         }
 
+        /** @var \DOMNode $child */
         foreach ($node->childNodes ?? [] as $k => $child) {
             if ('#text' === $child->nodeName) {
                 // Add text in the safe tree without a visitor for performance

diff --git a/src/Extension/Basic/NodeVisitor/ANodeVisitor.php b/src/Extension/Basic/NodeVisitor/ANodeVisitor.php
@@ -27,6 +27,9 @@ class ANodeVisitor extends AbstractNodeVisitor
 {
     use HasChildrenNodeVisitorTrait;
 
+    /**
+     * @var AHrefSanitizer
+     */
     private $sanitizer;
 
     public function __construct(array $config = [])

diff --git a/src/Extension/Basic/Sanitizer/AHrefSanitizer.php b/src/Extension/Basic/Sanitizer/AHrefSanitizer.php
@@ -44,10 +44,12 @@ public function sanitize(?string $input): ?string
             }
         }
 
-        $sanitized = $this->sanitizeUrl($input, $allowedSchemes, $allowedHosts, $this->forceHttps);
+        if (!$sanitized = $this->sanitizeUrl($input, $allowedSchemes, $allowedHosts, $this->forceHttps)) {
+            return null;
+        }
 
         // Basic validation that it's an e-mail
-        if (strpos($sanitized, 'mailto:') === 0 && (strpos($sanitized, '@') === false || strpos($sanitized, '.') === false)) {
+        if (0 === strpos($sanitized, 'mailto:') && (false === strpos($sanitized, '@') || false === strpos($sanitized, '.'))) {
             return null;
         }
 

diff --git a/src/Extension/ExtensionInterface.php b/src/Extension/ExtensionInterface.php
@@ -27,9 +27,7 @@ public function getName(): string;
 
     /**
      * Return a list of node visitors to register in the sanitizer following the format tagName => visitor.
-     * For instance:
-     *
-     *      'strong' => new StrongVisitor($config,
+     * For instance: 'strong' => new StrongVisitor($config).
      *
      * @param array $config The configuration given by the user of the library.
      *

diff --git a/src/Extension/Image/Sanitizer/ImgSrcSanitizer.php b/src/Extension/Image/Sanitizer/ImgSrcSanitizer.php
@@ -41,10 +41,12 @@ public function sanitize(?string $input): ?string
             $allowedHosts[] = null;
         }
 
-        $sanitized = $this->sanitizeUrl($input, $allowedSchemes, $allowedHosts, $this->forceHttps);
+        if (!$sanitized = $this->sanitizeUrl($input, $allowedSchemes, $allowedHosts, $this->forceHttps)) {
+            return null;
+        }
 
         // Allow only images in data URIs
-        if (strpos($sanitized, 'data:') === 0 && strpos($sanitized, 'data:image/') !== 0) {
+        if (0 === strpos($sanitized, 'data:') && 0 !== strpos($sanitized, 'data:image/')) {
             return null;
         }
 

diff --git a/src/Node/AbstractNode.php b/src/Node/AbstractNode.php
@@ -16,6 +16,9 @@
  */
 abstract class AbstractNode implements NodeInterface
 {
+    /**
+     * @var NodeInterface
+     */
     private $parent;
 
     public function __construct(NodeInterface $parent)

diff --git a/src/Node/AbstractTagNode.php b/src/Node/AbstractTagNode.php
@@ -59,13 +59,13 @@ protected function renderAttributes(): string
     {
         $rendered = [];
         foreach ($this->attributes as $name => $value) {
-            if ($value === null) {
+            if (null === $value) {
                 // Tag should be removed as a sanitizer found suspect data inside
                 continue;
             }
 
             $attr = $this->encodeHtmlEntities($name);
-            if ($value !== '') {
+            if ('' !== $value) {
                 // In quirks mode, IE8 does a poor job producing innerHTML values.
                 // If JavaScript does:
                 //      nodeA.innerHTML = nodeB.innerHTML;

diff --git a/src/Node/TagNodeInterface.php b/src/Node/TagNodeInterface.php
@@ -33,8 +33,6 @@ public function getAttribute(string $name): ?string;
      *
      * @param string $name
      * @param string $value
-     *
-     * @return void
      */
     public function setAttribute(string $name, string $value);
 }
diff --git a/src/Node/TextNode.php b/src/Node/TextNode.php
@@ -23,7 +23,7 @@ class TextNode extends AbstractNode
     use IsChildlessTrait;
     use StringSanitizerTrait;
 
-    private $text = '';
+    private $text;
 
     public function __construct(NodeInterface $parent, string $text)
     {

diff --git a/src/Sanitizer.php b/src/Sanitizer.php
@@ -106,6 +106,6 @@ public function sanitize(string $html): string
     private function isValidUtf8(string $html): bool
     {
         // preg_match() fails silently on strings containing invalid UTF-8.
-        return $html === '' || preg_match('/^./us', $html) === 1;
+        return '' === $html || 1 === preg_match('/^./us', $html);
     }
 }
diff --git a/src/Sanitizer/UrlSanitizerTrait.php b/src/Sanitizer/UrlSanitizerTrait.php
@@ -47,12 +47,12 @@ private function sanitizeUrl(?string $input, array $allowedSchemes, ?array $allo
         }
 
         // Invalid host
-        if ($allowedHosts !== null && !$this->isAllowedHost($url['host'], $allowedHosts)) {
+        if (null !== $allowedHosts && !$this->isAllowedHost($url['host'], $allowedHosts)) {
             return null;
         }
 
         // Force HTTPS
-        if ($forceHttps && $url['scheme'] === 'http') {
+        if ($forceHttps && 'http' === $url['scheme']) {
             $url['scheme'] = 'https';
         }
 
@@ -61,8 +61,8 @@ private function sanitizeUrl(?string $input, array $allowedSchemes, ?array $allo
 
     private function isAllowedHost(?string $host, array $allowedHosts): bool
     {
-        if ($host === null && \in_array(null, $allowedHosts, true)) {
-            return true;
+        if (null === $host) {
+            return \in_array(null, $allowedHosts, true);
         }
 
         $parts = array_reverse(explode('.', $host));

diff --git a/src/SanitizerBuilderInterface.php b/src/SanitizerBuilderInterface.php
@@ -24,8 +24,6 @@ interface SanitizerBuilderInterface
      * Register an extension to use in the sanitizer being built.
      *
      * @param ExtensionInterface $extension
-     *
-     * @return void
      */
     public function registerExtension(ExtensionInterface $extension);
 

diff --git a/src/Util/Dumper.php b/src/Util/Dumper.php
@@ -30,13 +30,14 @@ public static function dumpDomTree(\DOMNode $tree)
         echo "}\n";
     }
 
-    private static function dumpDomNode(\DOMNode $node)
+    private static function dumpDomNode(\DOMNode $node): string
     {
-        self::$id++;
+        ++self::$id;
 
         $name = self::$id.'-'.$node->nodeName;
         echo '    "'.$name."\";\n";
 
+        /** @var \DOMNode $child */
         foreach ($node->childNodes ?: [] as $child) {
             $childName = self::dumpDomNode($child);
             echo '    "'.$name.'" -> "'.$childName."\";\n";

diff --git a/src/Visitor/ScriptNodeVisitor.php b/src/Visitor/ScriptNodeVisitor.php
@@ -26,11 +26,6 @@ public function supports(\DOMNode $domNode, Cursor $cursor): bool
         return 'script' === $domNode->nodeName || 'noscript' === $domNode->nodeName;
     }
 
-    public function getDefaultAllowedAttributes(): array
-    {
-        return [];
-    }
-
     public function enterNode(\DOMNode $domNode, Cursor $cursor)
     {
         $node = new ScriptNode($cursor->node);

diff --git a/src/Visitor/StyleNodeVisitor.php b/src/Visitor/StyleNodeVisitor.php
@@ -26,11 +26,6 @@ public function supports(\DOMNode $domNode, Cursor $cursor): bool
         return 'style' === $domNode->nodeName;
     }
 
-    public function getDefaultAllowedAttributes(): array
-    {
-        return [];
-    }
-
     public function enterNode(\DOMNode $domNode, Cursor $cursor)
     {
         $node = new StyleNode($cursor->node);

diff --git a/src/Visitor/TagVisitorTrait.php b/src/Visitor/TagVisitorTrait.php
@@ -31,21 +31,21 @@ public function supports(\DOMNode $domNode, Cursor $cursor): bool
     /**
      * Set attributes from a DOM node to a sanitized node.
      *
-     * @param \DOMNode $domNode
+     * @param \DOMNode         $domNode
      * @param TagNodeInterface $node
-     * @param array $allowedAttributes
+     * @param array            $allowedAttributes
      */
     private function setAttributes(\DOMNode $domNode, TagNodeInterface $node, array $allowedAttributes = [])
     {
-        if (!count($domNode->attributes)) {
+        if (!\count($domNode->attributes)) {
             return;
         }
 
         /** @var \DOMAttr $attribute */
         foreach ($domNode->attributes as $attribute) {
             $name = strtolower($attribute->name);
 
-            if (in_array($name, $allowedAttributes)) {
+            if (\in_array($name, $allowedAttributes, true)) {
                 $node->setAttribute($name, $attribute->value);
             }
         }
@@ -55,13 +55,13 @@ private function setAttributes(\DOMNode $domNode, TagNodeInterface $node, array
      * Read the value of a DOMNode attribute.
      *
      * @param \DOMNode $domNode
-     * @param string $name
+     * @param string   $name
      *
      * @return null|string
      */
     private function getAttribute(\DOMNode $domNode, string $name): ?string
     {
-        if (!count($domNode->attributes)) {
+        if (!\count($domNode->attributes)) {
             return null;
         }
 

diff --git a/tests/AbstractSanitizerTest.php b/tests/AbstractSanitizerTest.php
@@ -23,7 +23,6 @@ public function provideFixtures(): array
     {
         // Fixtures shared by all sanitizers
         return [
-
             [
                 'hello world',
                 'hello world',
@@ -72,7 +71,6 @@ public function provideFixtures(): array
                 'Lorem ipsum<![CDATA[ <!-- ]]> <script>alert(1337)</script> <!-- -->',
                 'Lorem ipsum  ',
             ],
-
         ];
     }
 
@@ -94,7 +92,7 @@ public function testSanitize($input, $expectedOutput)
     public function testRemoveNullByte()
     {
         $this->assertSame('Null byte', $this->createSanitizer()->sanitize("Null byte\0"));
-        $this->assertSame('Null byte', $this->createSanitizer()->sanitize("Null byte&#0;"));
+        $this->assertSame('Null byte', $this->createSanitizer()->sanitize('Null byte&#0;'));
     }
 
     public function testDeeplyNestedTagDos()

diff --git a/tests/EmptySanitizerTest.php b/tests/EmptySanitizerTest.php
@@ -24,7 +24,6 @@ public function createSanitizer(): SanitizerInterface
     public function provideFixtures(): array
     {
         return array_merge(parent::provideFixtures(), [
-
             /*
              * Normal tags
              */
@@ -243,7 +242,6 @@ public function provideFixtures(): array
                 '<a style="font-size: 40px; color: red;">Lorem ipsum dolor sit amet, consectetur.</a>',
                 'Lorem ipsum dolor sit amet, consectetur.',
             ],
-
         ]);
     }
 }
diff --git a/tests/Extension/NodeVisitor/CustomNodeVisitor.php b/tests/Extension/NodeVisitor/CustomNodeVisitor.php
@@ -32,7 +32,7 @@ protected function getDomNodeName(): string
     public function getDefaultAllowedAttributes(): array
     {
         return [
-            'class', 'width', 'height'
+            'class', 'width', 'height',
         ];
     }
 

diff --git a/tests/FullSanitizerTest.php b/tests/FullSanitizerTest.php
@@ -39,7 +39,6 @@ public function createSanitizer(): SanitizerInterface
     public function provideFixtures(): array
     {
         return array_merge(parent::provideFixtures(), [
-
             /*
              * Normal tags
              */
@@ -383,7 +382,6 @@ public function provideFixtures(): array
                 '<a style="font-size: 40px; color: red;">Lorem ipsum dolor sit amet, consectetur.</a>',
                 '<a>Lorem ipsum dolor sit amet, consectetur.</a>',
             ],
-
         ]);
     }
 }
diff --git a/tests/Sanitizer/StringSanitizerTraitTest.php b/tests/Sanitizer/StringSanitizerTraitTest.php
@@ -25,10 +25,10 @@ public function provideEncodeHtmlEntites()
             '"' => '&#34;',
             '\'' => '&#039;',
             '&' => '&amp;',
-            "<" => "&lt;",
-            ">" => "&gt;",
-            "&lt;" => "&amp;lt;",
-            "&gt;" => "&amp;gt;",
+            '<' => '&lt;',
+            '>' => '&gt;',
+            '&lt;' => '&amp;lt;',
+            '&gt;' => '&amp;gt;',
             '+' => '&#43;',
             '=' => '&#61;',
             '@' => '&#64;',