Minor fixes:

- Bug: Rule didn't always match when URL contained any of the following characters +()[]
- Added tests for the above characters
- Minor readability and code optimizations

Test improvements/fixes
- List of hosts randomized when searching for an test subject, returning an HTTP 500 status code (we don't want to prioritize)
- Mark MySQL tests as skipped (instead of failing) if unable to connect to the test database

Author: JanPetterMG

src/Client/Directives/DirectiveClientTrait.php

@@ -82,18 +82,20 @@ private function isBetween($timestamp, $fromTime, $toTime, $format = 'Hi')
      */
     private function checkPaths($path, array $paths)
     {
-        $pairs = [
+        $reserved = [
             '?' => '\?',
             '.' => '\.',
             '*' => '.*',
+            '+' => '\+',
+            '(' => '\(',
+            ')' => '\)',
+            '[' => '\[',
+            ']' => '\]',
         ];
         $errorHandler = new ErrorHandler();
         set_error_handler([$errorHandler, 'callback'], E_NOTICE | E_WARNING);
         foreach ($paths as $rule) {
-            $escaped = $rule;
-            foreach ($pairs as $search => $replace) {
-                $escaped = str_replace($search, $replace, $escaped);
-            }
+            $escaped = str_replace(array_keys($reserved), array_values($reserved), $rule);
             if (preg_match('#^' . $escaped . '#', $path) === 1) {
                 restore_error_handler();
                 return mb_strlen($rule);

src/Parser/UriParser.php

@@ -70,7 +70,6 @@ public function encode()
             '!%21!ui' => "!",
             '!%23!ui' => "#",
             '!%24!ui' => "$",
-            '!%25!ui' => "%",
             '!%26!ui' => "&",
             '!%27!ui' => "'",
             '!%28!ui' => "(",
@@ -86,7 +85,10 @@ public function encode()
             '!%40!ui' => "@",
             '!%5B!ui' => "[",
             '!%5D!ui' => "]",
+            '!%25!ui' => "%",
         ];
+        // The % character must be the last in the $reserved array.
+        // This makes sure that the already encoded values are not lost or encoded again.
         $this->uri = preg_replace(array_keys($reserved), array_values($reserved), rawurlencode($this->uri));
         return $this->baseToLowercase();
     }

tests/EscapingTest.php

@@ -32,11 +32,14 @@ public function testEscaping($robotsTxtContent, $rendered)
         $this->assertTrue($parser->userAgent()->isAllowed("/%5C."));
         $this->assertFalse($parser->userAgent()->isDisallowed("/%5C."));
 
-        /**
-         * Additional tests to enable in the future, currently disabled due to bugs
-         */
-        //$this->assertTrue($parser->userAgent()->isDisallowed("/("));
-        //$this->assertFalse($parser->userAgent()->isAllowed("/("));
+        $this->assertTrue($parser->userAgent()->isDisallowed("/("));
+        $this->assertFalse($parser->userAgent()->isAllowed("/("));
+
+        $this->assertTrue($parser->userAgent()->isDisallowed("/)"));
+        $this->assertFalse($parser->userAgent()->isAllowed("/)"));
+
+        $this->assertTrue($parser->userAgent()->isAllowed("/+"));
+        $this->assertFalse($parser->userAgent()->isDisallowed("/+"));
 
         if ($rendered !== false) {
             if (version_compare(phpversion(), '7.0.0', '<')) {

tests/HttpRetryAfterTest.php

@@ -50,7 +50,7 @@ public function testHttpRetryAfter()
             }
         }
         if ($count === 0) {
-            $this->markTestSkipped("No URLs returned both `HTTP 503` and the `Retry-after` header. NB! Such circumstances are uncommon, and if the criteria is met, it's only a temporary state.");
+            $this->markTestIncomplete('None of the test urls returned both `HTTP 503` and the `Retry-after` header. Such circumstances are usually temporary and uncommon. Try updating the list of test subjects.');
         }
     }
 }

tests/MysqlCacheDisplaceTest.php

@@ -29,7 +29,7 @@ class MysqlCacheDisplaceTest extends TestCase
     public function testCacheDisplace()
     {
         // This URL list is based on real data. Update when needed!
-        // It loops thou the list, until an http 500 match is found. Therefore not all URL are fetched...
+        // It loops thou the list, until an http 500 match is found. Therefore not all URLs are fetched...
         $bases = [
             'http://acheterdesvues.fr:80',
             'http://basbaassauce.com:80',
@@ -67,14 +67,14 @@ public function testCacheDisplace()
             'https://www.aei.org:443',
             'https://www.edoramedia.com:443',
         ];
-        $result = false;
+        // Query these (random) hosts at random order, we don't want to prioritize any of them over each other.
+        shuffle($bases);
         foreach ($bases as $base) {
-            if ($result) {
-                continue;
+            if ($this->check($base)) {
+                return;
             }
-            $result = $this->check($base);
         }
-        $this->assertTrue($result);
+        $this->markTestIncomplete('None of the test urls returned an HTTP 500 status code. Try updating the list of test subjects.');
     }
 
     /**
@@ -84,7 +84,12 @@ public function testCacheDisplace()
      */
     private function check($baseUri)
     {
-        $pdo = new \PDO($GLOBALS['DB_DSN'], $GLOBALS['DB_USER'], $GLOBALS['DB_PASSWD']);
+        try {
+            $pdo = new \PDO($GLOBALS['DB_DSN'], $GLOBALS['DB_USER'], $GLOBALS['DB_PASSWD']);
+        } catch (\PDOException $e) {
+            $this->markTestSkipped('Unable to connect to the MySQL database');
+            return false;
+        }
         $pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION);
 
         $cache = (new RobotsTxtParser\Database($pdo))->cache();
@@ -116,9 +121,6 @@ private function check($baseUri)
         $query->execute();
         // Delete fake data
         $base->invalidate();
-        if ($query->rowCount() > 0) {
-            return true;
-        }
-        return false;
+        return $query->rowCount() > 0;
     }
 }

tests/MysqlCacheTest.php

@@ -27,7 +27,12 @@ class MysqlCacheTest extends TestCase
      */
     public function testCache($uri, $baseUri)
     {
-        $pdo = new \PDO($GLOBALS['DB_DSN'], $GLOBALS['DB_USER'], $GLOBALS['DB_PASSWD']);
+        try {
+            $pdo = new \PDO($GLOBALS['DB_DSN'], $GLOBALS['DB_USER'], $GLOBALS['DB_PASSWD']);
+        } catch (\PDOException $e) {
+            $this->markTestSkipped('Unable to connect to the MySQL database');
+            return;
+        }
         $pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION);
 
         $database = new RobotsTxtParser\Database($pdo);

tests/MysqlDelayTest.php

@@ -26,7 +26,12 @@ class MysqlDelayTest extends TestCase
      */
     public function testDelay($uri, $userAgent)
     {
-        $pdo = new \PDO($GLOBALS['DB_DSN'], $GLOBALS['DB_USER'], $GLOBALS['DB_PASSWD']);
+        try {
+            $pdo = new \PDO($GLOBALS['DB_DSN'], $GLOBALS['DB_USER'], $GLOBALS['DB_PASSWD']);
+        } catch (\PDOException $e) {
+            $this->markTestSkipped('Unable to connect to the MySQL database');
+            return;
+        }
         $pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION);
 
         $handler = (new RobotsTxtParser\Database($pdo))->delay();

tests/RenderTest.php

@@ -42,7 +42,11 @@ public function testRender($robotsTxtContent, $rendered)
         $this->assertEquals($rendered['compatibility'], $parser->render()->compatibility("\n"));
 
         // Make sure the compatibility robots.txt has a newline at the end
-        foreach (["\r", "\n", "\r\n"] as $separator) {
+        foreach ([
+                     "\r",
+                     "\n",
+                     "\r\n"
+                 ] as $separator) {
             $length = strlen($separator);
             $this->assertEquals($separator, substr($parser->render()->compatibility($separator), -$length));
         }