Paths containing special characters paths in the configuration are wrongly resolved

phpdoc.dist.xml

@@ -8,6 +8,7 @@
     <title>phpDocumentor</title>
     <paths>
         <output>build/website/docs</output>
+        <cache>build/ex$a#mple cache/</cache>
     </paths>
     <version number="3.0.0">
         <folder>latest</folder>

src/phpDocumentor/Configuration/PathNormalizingMiddleware.php

@@ -76,7 +76,7 @@ private function makeDsnRelativeToConfig(Configuration $configuration, ?UriInter
             $version->setApi($apiConfigs);
 
             foreach ($version->getGuides() ?? [] as $key => $guide) {
-                $version->guides[$key] = $version->guides[$key]->withSource(
+                $version->guides[$key]->withSource(
                     $guide->source()->withDsn(
                         $guide->source()->dsn()->resolve($configDsn)
                     )
@@ -174,7 +174,8 @@ private function pathToGlobPattern(string $path): string
 
     public function normalizeCachePath(?UriInterface $uri, Path $cachePath): Path
     {
-        if ($cachePath::isAbsolutePath((string) $cachePath)) {
+        $cachePathAsString = $cachePath->decoded();
+        if ($cachePath::isAbsolutePath($cachePathAsString)) {
             return $cachePath;
         }
 
@@ -185,11 +186,6 @@ public function normalizeCachePath(?UriInterface $uri, Path $cachePath): Path
         $configFile = Dsn::createFromUri($uri);
         $configPath = $configFile->withPath(Path::dirname($configFile->getPath()));
 
-        // Since League URI 6.5 it will url-encode backslashes. Since this is used in windows, we convert it
-        // into a forward slash
-        $cachePathAsString = (string) $cachePath;
-        $cachePathAsString = str_replace('\\', '/', $cachePathAsString);
-
         return Dsn::createFromString($cachePathAsString)->resolve($configPath)->getPath();
     }
 }

src/phpDocumentor/Configuration/Source.php

@@ -20,6 +20,7 @@
 use phpDocumentor\Path;
 use ReturnTypeWillChange;
 
+use Webmozart\Assert\Assert;
 use function array_map;
 use function in_array;
 use function ltrim;
@@ -32,14 +33,16 @@
 final class Source implements ArrayAccess
 {
     /** @var Dsn */
-    private $dsn;
+    private Dsn $dsn;
 
     /** @var array<array-key, Path> */
-    private $paths;
+    private array $paths;
 
     /** @param array<array-key, Path> $paths */
     public function __construct(Dsn $dsn, array $paths)
     {
+        Assert::allIsInstanceOf($paths, Path::class);
+
         $this->dsn = $dsn;
         $this->paths = $paths;
     }
@@ -77,7 +80,7 @@ public function globPatterns(): array
     {
         return array_map(
             function (Path $path): string {
-                return $this->pathToGlobPattern((string) $path);
+                return $this->pathToGlobPattern($path);
             },
             $this->paths
         );
@@ -96,15 +99,15 @@ private function normalizePath(string $path): string
         return rtrim($path, '/');
     }
 
-    private function pathToGlobPattern(string $path): string
+    private function pathToGlobPattern(Path $path): string
     {
-        $path = $this->normalizePath($path);
+        $pathAsString = $this->normalizePath($path->decoded());
 
-        if (substr($path, -1) !== '*' && strpos($path, '.') === false) {
-            $path .= '/**/*';
+        if (substr($pathAsString, -1) !== '*' && strpos($pathAsString, '.') === false) {
+            $pathAsString .= '/**/*';
         }
 
-        return $path;
+        return $pathAsString;
     }
 
     /** @param string $offset */

src/phpDocumentor/Dsn.php

@@ -219,7 +219,8 @@ public function resolve(Dsn $baseDsn): self
 
     public function withPath(Path $path): self
     {
-        $pathString = (string) $path;
+        // The URI object will want reencode; so we need the decoded version to prevent double encoding
+        $pathString = $path->decoded();
         if (strpos($pathString, '/') !== 0) {
             $pathString = '/' . $pathString;
         }

src/phpDocumentor/Parser/Cache/Locator.php

@@ -66,15 +66,15 @@ public function providePath(Path $path): void
     {
         $this->path = $path;
 
-        $this->fileCache->init('files', (string) $path);
-        $this->descriptorCache->init('descriptors', (string) $path);
+        $this->fileCache->init('files', $path->decoded());
+        $this->descriptorCache->init('descriptors', $path->decoded());
     }
 
     public function locate(string $namespace = ''): Path
     {
-        $namespacePath = rtrim(sprintf('%s/%s', (string) $this->root(), $namespace), '/');
+        $namespacePath = rtrim(sprintf('%s/%s', $this->root()->decoded(), $namespace), '/');
 
-        if (!is_dir($namespacePath) && !@mkdir($namespacePath, 0777, true)) {
+        if (!@mkdir($namespacePath, 0777, true) && !is_dir($namespacePath)) {
             $error = error_get_last();
             if ($error) {
                 throw new RuntimeException(

src/phpDocumentor/Path.php

@@ -13,27 +13,17 @@
 
 namespace phpDocumentor;
 
+use Symfony\Component\Filesystem\Path as SymfonyPath;
 use Webmozart\Assert\Assert;
-
-use function array_pop;
-use function ctype_alpha;
-use function explode;
-use function implode;
-use function parse_url;
 use function sprintf;
-use function strlen;
-use function strspn;
-
-use const PHP_URL_SCHEME;
 
 /**
  * Value Object for paths.
  * This can be absolute or relative.
  */
 final class Path
 {
-    /** @var string */
-    private $path;
+    private string $path;
 
     /**
      * Initializes the path.
@@ -45,7 +35,12 @@ public function __construct(string $path)
             sprintf('"%s" is not a valid path', $path)
         );
 
-        $this->path = $path;
+        // Canonicalizing paths will ensure they are all *NIX-like paths whose relative components are
+        // converted into a regular representation. This normalization step will make it easier to work with.
+        $this->path = SymfonyPath::canonicalize($path);
+
+        // URL encoding will ensure we have a consistent format for the paths
+        $this->path = implode('/', array_map('rawurlencode', explode('/', $this->path)));
     }
 
     /**
@@ -56,12 +51,22 @@ public function equals(self $otherPath): bool
         return $this->path === (string) $otherPath;
     }
 
+    public function encoded(): string
+    {
+        return $this->path;
+    }
+
+    public function decoded(): string
+    {
+        return implode('/', array_map('rawurldecode', explode('/', $this->path)));
+    }
+
     /**
-     * returns a string representation of the path.
+     * Returns a string representation of the path.
      */
     public function __toString(): string
     {
-        return $this->path;
+        return $this->encoded();
     }
 
     /**
@@ -71,19 +76,11 @@ public function __toString(): string
      */
     public static function isAbsolutePath(string $file): bool
     {
-        return strspn($file, '/\\', 0, 1)
-            || (strlen($file) > 3 && ctype_alpha($file[0])
-                && $file[1] === ':'
-                && strspn($file, '/\\', 2, 1)
-            )
-            || parse_url($file, PHP_URL_SCHEME) !== null;
+        return SymfonyPath::isAbsolute($file);
     }
 
     public static function dirname(Path $input): self
     {
-        $parts = explode('/', (string) $input);
-        array_pop($parts);
-
-        return new self(implode('/', $parts));
+        return new Path(SymfonyPath::getDirectory($input->decoded()));
     }
 }

tests/unit/phpDocumentor/Configuration/Definition/Version3Test.php

@@ -55,7 +55,6 @@ public function testNormalizingTheOutputTransformsTheConfig(): void
         $expected['paths']['output'] = Dsn::createFromString($expected['paths']['output']);
         $expected['paths']['cache'] = new Path($expected['paths']['cache']);
         $expected['versions']['1.0.0']['api'] = $expected['versions']['1.0.0']['apis'];
-        unset($expected['versions']['1.0.0']['apis']);
         $expected['versions']['1.0.0']['api'][0]['ignore-tags']
             = $expected['versions']['1.0.0']['api'][0]['ignore-tags']['ignore_tags'];
         $expected['versions']['1.0.0']['api'][0]['extensions']
@@ -66,6 +65,18 @@ public function testNormalizingTheOutputTransformsTheConfig(): void
             = $expected['versions']['1.0.0']['api'][0]['visibilities'];
         unset($expected['versions']['1.0.0']['api'][0]['visibilities']);
         $expected['templates'][0]['location'] = null;
+        $expected['versions']['1.0.0']['api'][0]['source']['dsn']
+            = Dsn::createFromString($expected['versions']['1.0.0']['api'][0]['source']['dsn']);
+        $expected['versions']['1.0.0']['api'][0]['source']['paths'] = array_map(
+            static fn (string $path) => new Path($path),
+            $expected['versions']['1.0.0']['api'][0]['source']['paths']
+        );
+
+        // Pluralities are converted into a singular version and a such these keys no longer exist
+        unset(
+            $expected['versions']['1.0.0']['apis'],
+            $expected['versions']['1.0.0']['api'][0]['visibilities']
+        );
 
         $configuration = $definition->normalize($configuration);
 

tests/unit/phpDocumentor/Configuration/PathNormalizingMiddlewareTest.php

@@ -75,7 +75,7 @@ public function testNormalizeCachePath(string $input, string $output, string $co
             Uri::createFromString($configPath)
         );
 
-        self::assertSame($output, (string) $outputConfig['phpdocumentor']['paths']['cache']);
+        self::assertSame($output, $outputConfig['phpdocumentor']['paths']['cache']);
     }
 
     /**
@@ -111,7 +111,7 @@ public function testDsnResolvedByConfigPath(): void
             Uri::createFromString('/data/phpDocumentor/config.xml')
         );
 
-        self::assertEquals(
+        self::assertSame(
             '/data/phpDocumentor/',
             (string) $outputConfig['phpdocumentor']['versions']['1.0.0']->api[0]['source']['dsn']
         );
@@ -144,20 +144,40 @@ public function cachePathProvider(): array
                 '/opt/myProject',
                 '/data/phpdocumentor/config.xml',
             ],
-            'Absolute windows paths are not normalized' => [
-                'D:\opt\myProject',
+            'Absolute windows paths are normalized' => [
                 'D:\opt\myProject',
+                'D:/opt/myProject',
+                '/data/phpdocumentor/config.xml',
+            ],
+            'Absolute paths could contain special characters' => [
+                '/opt/#myProject/with a space',
+                '/opt/#myProject/with a space',
+                '/data/phpdocumentor/config.xml',
+            ],
+            'Absolute windows paths could contain hashes' => [
+                'D:\opt\#myProject',
+                'D:/opt/#myProject',
                 '/data/phpdocumentor/config.xml',
             ],
             'Relative unix paths are changed to an absolute path with the config folder as prefix' => [
                 '.phpdoc/cache',
                 '/data/phpdocumentor/.phpdoc/cache',
                 '/data/phpdocumentor/config.xml',
             ],
+            'Relative paths may contain spaces' => [
+                '.phpdoc/my cache',
+                '/data/phpdocumentor/.phpdoc/my cache',
+                '/data/phpdocumentor/config.xml',
+            ],
+            'Relative paths may contain hashes' => [
+                '.phpdoc/#cache',
+                '/data/phpdocumentor/.phpdoc/#cache',
+                '/data/phpdocumentor/config.xml',
+            ],
             'Relative paths on Windows are changed to an absolute path with the config folder as prefix' => [
                 '.phpdoc\cache',
                 'd:/data/phpdocumentor/.phpdoc/cache',
-                'D:/data/phpdocumentor/config.xml',
+                'D:\data\phpdocumentor\config.xml',
             ],
         ];
     }

tests/unit/phpDocumentor/PathTest.php

@@ -29,7 +29,7 @@ public function testItCanContainALocationOnAStorageService(): void
     {
         $path = new Path('/my/Path');
 
-        $this->assertSame('/my/Path', (string) $path);
+        self::assertSame('/my/Path', (string) $path);
     }
 
     /**
@@ -41,24 +41,23 @@ public function testItCanCompareItselfToAnotherPath(): void
         $similar    = new Path('a');
         $dissimilar = new Path('b');
 
-        $this->assertTrue($subject->equals($similar));
-        $this->assertFalse($subject->equals($dissimilar));
+        self::assertTrue($subject->equals($similar));
+        self::assertFalse($subject->equals($dissimilar));
     }
 
     /**
      * @covers ::isAbsolutePath
      */
     public function testItCanCheckWhetherTheGivenPathIsAnAbsolutePath(): void
     {
-        $this->assertTrue(Path::isAbsolutePath('\\\\my\\absolute\\path'));
-        $this->assertTrue(Path::isAbsolutePath('/my/absolute/path'));
-        $this->assertTrue(Path::isAbsolutePath('c:\\my\\absolute\\path'));
-        $this->assertTrue(Path::isAbsolutePath('http://my/absolute/path'));
-        $this->assertTrue(Path::isAbsolutePath('//my/absolute/path'));
-
-        $this->assertFalse(Path::isAbsolutePath('path'));
-        $this->assertFalse(Path::isAbsolutePath('my/absolute/path'));
-        $this->assertFalse(Path::isAbsolutePath('./my/absolute/path'));
-        $this->assertFalse(Path::isAbsolutePath('../my/absolute/path'));
+        self::assertTrue(Path::isAbsolutePath('\\\\my\\absolute\\path'));
+        self::assertTrue(Path::isAbsolutePath('/my/absolute/path'));
+        self::assertTrue(Path::isAbsolutePath('c:\\my\\absolute\\path'));
+        self::assertTrue(Path::isAbsolutePath('//my/absolute/path'));
+
+        self::assertFalse(Path::isAbsolutePath('path'));
+        self::assertFalse(Path::isAbsolutePath('my/absolute/path'));
+        self::assertFalse(Path::isAbsolutePath('./my/absolute/path'));
+        self::assertFalse(Path::isAbsolutePath('../my/absolute/path'));
     }
 }