Merge pull request #39 from thephpleague/bugfix/path-encoding

bug fixing issue #38
thephpleague · Mar 18, 2016 · 7628a30 · 7628a30
2 parents 977c560 + 6d0299f
commit 7628a30
Show file tree

Hide file tree

Showing 11 changed files with 117 additions and 183 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -28,4 +28,4 @@ script:
   - composer test
 
 after_script:
- - if [ "$COLLECT_COVERAGE" == "true" ]; then wget https://scrutinizer-ci.com/ocular.phar; && php ocular.phar code-coverage:upload --format=php-clover build/logs/clover.xml; fi
+ - if [ "$COLLECT_COVERAGE" == "true" ]; then wget https://scrutinizer-ci.com/ocular.phar && php ocular.phar code-coverage:upload --format=php-clover build/clover.xml; fi
diff --git a/src/Components/AbstractComponent.php b/src/Components/AbstractComponent.php
@@ -77,7 +77,7 @@ protected function validate($data)
      */
     public function __toString()
     {
-        return static::encode((string) $this->data);
+        return $this->encode($this->data);
     }
 
     /**

diff --git a/src/Components/AbstractHierarchicalComponent.php b/src/Components/AbstractHierarchicalComponent.php
@@ -104,9 +104,9 @@ public function getUriComponent()
     public function prepend($component)
     {
         return $this->createFromArray(
-            $this->validateComponent($component),
-            $this->isAbsolute)
-        ->append($this);
+                $this->validateComponent($component),
+                $this->isAbsolute
+            )->append($this);
     }
 
     /**

diff --git a/src/Components/DataPath.php b/src/Components/DataPath.php
@@ -64,22 +64,6 @@ class DataPath extends AbstractComponent implements DataPathInterface
      */
     protected $document;
 
-    /**
-     * @inheritdoc
-     */
-    protected static $characters_set = [
-        '/', ':', '@', '!', '$', '&', "'", '%',
-        '(', ')', '*', '+', ',', ';', '=', '?',
-    ];
-
-    /**
-     * @inheritdoc
-     */
-    protected static $characters_set_encoded = [
-        '%2F', '%3A', '%40', '%21', '%24', '%26', '%27', '%25',
-        '%28', '%29', '%2A', '%2B', '%2C', '%3B', '%3D', '%3F',
-    ];
-
     /**
      * @inheritdoc
      */
@@ -345,7 +329,7 @@ protected static function format($mimetype, $parameters, $isBinaryData, $data)
             $parameters .= ';'.static::BINARY_PARAMETER;
         }
 
-        return static::encode($mimetype.$parameters.','.$data);
+        return static::encodePath($mimetype.$parameters.','.$data);
     }
 
     /**

diff --git a/src/Components/Fragment.php b/src/Components/Fragment.php
@@ -25,18 +25,7 @@ class Fragment extends AbstractComponent implements FragmentInterface
     /**
      * @inheritdoc
      */
-    protected static $characters_set = [
-        '!', '$', '&', "'", '(', ')', '*', '+',
-        ',', ';', '=', ':', '@', '/', '?',
-    ];
-
-    /**
-     * @inheritdoc
-     */
-    protected static $characters_set_encoded = [
-        '%21', '%24', '%26', '%27', '%28', '%29', '%2A', '%2B',
-        '%2C', '%3B', '%3D', '%3A', '%40', '%2F', '%3F',
-    ];
+    protected static $reservedCharactersRegex = "\!\$&'\(\)\*\+,;\=\:\/@\?";
 
     /**
      * Returns the instance string representation

diff --git a/src/Components/HierarchicalPath.php b/src/Components/HierarchicalPath.php
@@ -30,22 +30,6 @@ class HierarchicalPath extends AbstractHierarchicalComponent implements Hierarch
      */
     protected static $separator = '/';
 
-    /**
-     * @inheritdoc
-     */
-    protected static $characters_set = [
-        '/', ':', '@', '!', '$', '&', "'", '%',
-        '(', ')', '*', '+', ',', ';', '=', '?',
-    ];
-
-    /**
-     * @inheritdoc
-     */
-    protected static $characters_set_encoded = [
-        '%2F', '%3A', '%40', '%21', '%24', '%26', '%27', '%25',
-        '%28', '%29', '%2A', '%2B', '%2C', '%3B', '%3D', '%3F',
-    ];
-
     /**
      * @inheritdoc
      */
@@ -91,11 +75,7 @@ protected function validate($data)
             return isset($segment);
         };
 
-        $data = preg_replace_callback(
-            $this->getReservedRegex(),
-            [$this, 'decodeSegmentPart'],
-            $data
-        );
+        $data = $this->decodePath($data);
 
         return array_filter(explode(static::$separator, $data), $filterSegment);
     }
@@ -126,9 +106,8 @@ public function __toString()
         if ($this->isAbsolute == self::IS_ABSOLUTE) {
             $front_delimiter = static::$separator;
         }
-        $path = $front_delimiter.implode(static::$separator, array_map([$this, 'encode'], $this->data));
 
-        return $this->encode($path);
+        return $front_delimiter.$this->encodePath(implode(static::$separator, $this->data));
     }
 
     /**

diff --git a/src/Components/Path.php b/src/Components/Path.php
@@ -24,22 +24,6 @@ class Path extends AbstractComponent implements PathInterface
 {
     use PathTrait;
 
-    /**
-     * @inheritdoc
-     */
-    protected static $characters_set = [
-        '/', ':', '@', '!', '$', '&', "'", '%',
-        '(', ')', '*', '+', ',', ';', '=', '?',
-    ];
-
-    /**
-     * @inheritdoc
-     */
-    protected static $characters_set_encoded = [
-        '%2F', '%3A', '%40', '%21', '%24', '%26', '%27', '%25',
-        '%28', '%29', '%2A', '%2B', '%2C', '%3B', '%3D', '%3F',
-    ];
-
     /**
      * @inheritdoc
      */
@@ -55,32 +39,22 @@ public function __construct($path = '')
         parent::__construct($this->validateString($path));
     }
 
+    public function __toString()
+    {
+        return $this->encodePath($this->data);
+    }
+
     /**
      * validate the submitted data
      *
      * @param string $path
      *
-     * @return array
+     * @return string
      */
     protected function validate($path)
     {
         $this->assertValidComponent($path);
 
-        return preg_replace_callback(
-            $this->getReservedRegex(),
-            [$this, 'decodeSegmentPart'],
-            $path
-        );
-    }
-
-    /**
-     * Returns the instance string representation; If the
-     * instance is not defined an empty string is returned
-     *
-     * @return string
-     */
-    public function __toString()
-    {
-        return static::encode($this->data);
+        return $this->decodePath($path);
     }
 }
diff --git a/src/Components/PathTrait.php b/src/Components/PathTrait.php
@@ -28,6 +28,11 @@ trait PathTrait
      */
     protected static $typeRegex = ',^(?P<basename>.*);type=(?P<typecode>a|i|d)$,';
 
+    /**
+     * Paht reserved characters regular expression
+     */
+    protected static $pathReservedCharactersRegex = "/(?:[^\!\$&'\(\)\*\+,;\=\:\/@\?%]+|%(?![A-Fa-f0-9]{2}))/S";
+
     /**
      * Typecode value
      *
@@ -265,13 +270,40 @@ public function withTypecode($type)
     }
 
     /**
-     * Encode a segment or the entire path string
+     * Encode a path string according to RFC3986
      *
-     * @param  array  $matches
-     * @return string
+     * @param string $subject can be a string or an array
+     *
+     * @return string The same type as the input parameter
+     */
+    protected static function encodePath($subject)
+    {
+        $encoder = function (array $matches) {
+            return rawurlencode($matches[0]);
+        };
+
+        $formatter = function (array $matches) {
+            return strtoupper($matches['encode']);
+        };
+
+        $subject = preg_replace_callback(self::$pathReservedCharactersRegex, $encoder, $subject);
+
+        return preg_replace_callback(',(?<encode>%[0-9a-f]{2}),', $formatter, $subject);
+    }
+
+    /**
+     * Decode a path string according to RFC3986
+     *
+     * @param string $subject can be a string or an array
+     *
+     * @return string The same type as the input parameter
      */
-    protected function decodeSegmentPart(array $matches)
+    protected static function decodePath($subject)
     {
-        return rawurldecode(array_shift($matches));
+        $decoder = function (array $matches) {
+            return rawurldecode($matches[0]);
+        };
+
+        return preg_replace_callback(self::$pathReservedCharactersRegex, $decoder, $subject);
     }
 }
diff --git a/src/Types/ImmutableComponentTrait.php b/src/Types/ImmutableComponentTrait.php
@@ -27,39 +27,21 @@ trait ImmutableComponentTrait
     use ValidatorTrait;
 
     /**
-     * Characters to conform to RFC3986 - http://tools.ietf.org/html/rfc3986#section-2
-     *
-     * @var array
-     */
-    protected static $characters_set = [
-        '!', '$', '&', "'", '(', ')', '*', '+', ',', ';', '=', ':',
-    ];
-
-    /**
-     * Path Character Decoding Regular Expression
+     * Reserved characters list
      *
      * @var string
      */
-    protected static $characters_set_compiled;
+    protected static $reservedCharactersRegex = "\!\$&'\(\)\*\+,;\=\:";
 
     /**
-     * Encoded characters to conform to RFC3986 - http://tools.ietf.org/html/rfc3986#section-2
-     *
-     * @var array
-     */
-    protected static $characters_set_encoded = [
-        '%21', '%24', '%26', '%27', '%28', '%29', '%2A', '%2B', '%2C', '%3B', '%3D', '%3A',
-    ];
-
-    /**
-     * Invalid Characters list
+     * Invalid characters list
      *
      * @var string
      */
     protected static $invalidCharactersRegex;
 
     /**
-     * Check the string against RFC3986 rules
+     * Asserts the string against RFC3986 rules
      *
      * @param string $str
      *
@@ -101,40 +83,30 @@ abstract public function getUriComponent();
      */
     abstract public function __toString();
 
-    /**
-     * Return the precompiled Path Character Decoding Regular Expression
-     *
-     * @return string
-     */
-    protected static function getReservedRegex()
-    {
-        if (!isset(static::$characters_set_compiled)) {
-            $reserved = preg_quote(implode('', static::$characters_set), '/');
-            static::$characters_set_compiled = '/(?:[^'.$reserved.']+|%(?![A-Fa-f0-9]{2}))/S';
-        }
-        return static::$characters_set_compiled;
-    }
-
     /**
      * Encoding string according to RFC3986
      *
-     * @param string $value
+     * @param string $str
      *
      * @return string
      */
-    protected static function encode($value)
+    protected static function encode($str)
     {
+        $encoder = function (array $matches) {
+            return rawurlencode($matches[0]);
+        };
+
+        $formatter = function (array $matches) {
+            return strtoupper($matches['encode']);
+        };
+
         $str = preg_replace_callback(
-            self::getReservedRegex(),
-            function (array $matches) {
-                return rawurlencode($matches[0]);
-            },
-            $value
+            '/(?:[^'.static::$reservedCharactersRegex.']+|%(?![A-Fa-f0-9]{2}))/S',
+            $encoder,
+            $str
         );
 
-        return preg_replace_callback(',(?<encode>%[0-9a-f]{2}),', function (array $matches) {
-            return strtoupper($matches['encode']);
-        }, $str);
+        return preg_replace_callback(',(?<encode>%[0-9a-f]{2}),', $formatter, $str);
     }
 
     /**

diff --git a/test/Components/PathTest.php b/test/Components/PathTest.php
@@ -5,7 +5,6 @@
 use InvalidArgumentException;
 use League\Uri\Components\Path;
 use PHPUnit_Framework_TestCase;
-use ReflectionClass;
 
 /**
  * @group path
@@ -19,9 +18,6 @@ class PathTest extends PHPUnit_Framework_TestCase
     public function testGetUriComponent($raw, $parsed)
     {
         $path = new Path($raw);
-        $property = (new ReflectionClass(Path::class))->getProperty('characters_set_compiled');
-        $property->setAccessible(true);
-        $property->setValue($path, null);
         $this->assertSame($parsed, $path->getUriComponent());
     }