Refactor RealPath

- Drop Windows support - Add additional tests for existing and non-existing paths Signed-off-by: ramchale <[email protected]>
laminas · Nov 14, 2024 · ebd5236 · ebd5236
1 parent 55ef969
commit ebd5236
Show file tree

Hide file tree

Showing 5 changed files with 105 additions and 157 deletions.
diff --git a/docs/book/v3/migration/v2-to-v3.md b/docs/book/v3/migration/v2-to-v3.md
@@ -131,6 +131,23 @@ Additionally, `$options['pattern']` _must_ be provided at construction time or a
 
 Exceptions for invalid or empty patterns are now thrown during construct rather than when the filter is invoked.
 
+#### `RealPath`
+
+The following methods have been removed:
+
+- `setExists`
+- `getExists`
+
+The constructor now only accepts an associative array of [documented options](../standard-filters.md#realPath).
+
+`false` is no longer returned when the path must exist and does not.
+Instead, the original value is returned.
+Filters are not intended to provide validation.
+So, to check if the path exists, ensure a validator (such as `Laminas\Validator\File\Exists') is also used.
+
+Windows support has been dropped.
+Which in some cases may now need a custom filter to handle Windows specific issues.
+
 #### `SeparatorToCamelCase`
 
 The constructor now only accepts an associative array of [documented options](../word.md#separatorToCamelCase).

diff --git a/docs/book/v3/standard-filters.md b/docs/book/v3/standard-filters.md
@@ -1125,8 +1125,7 @@ For more complex usage, read the
 
 ## RealPath
 
-This filter will resolve given links and pathnames, and returns the canonicalized
-absolute pathnames.
+This filter will resolve given links and pathnames, and returns the canonicalized absolute pathnames.
 
 ### Supported Options
 
@@ -1137,13 +1136,12 @@ The following options are supported for `Laminas\Filter\RealPath`:
 
 ### Basic Usage
 
-For any given link or pathname, its absolute path will be returned. References
-to `/./`, `/../` and extra `/` sequences in the input path will be stripped. The
-resulting path will not have any symbolic links, `/./`, or `/../` sequences.
+For any given link or pathname, its absolute path will be returned.
+References to `/./`, `/../` and extra `/` sequences in the input path will be stripped.
+The resulting path will not have any symbolic links, `/./`, or `/../` sequences.
 
-`Laminas\Filter\RealPath` will return `FALSE` on failure, e.g. if the file does not exist. On BSD
-systems `Laminas\Filter\RealPath` doesn't fail if only the last path component doesn't exist, while
-other systems will return `FALSE`.
+`Laminas\Filter\RealPath` will return the value passed to the filter on failure, e.g. if the file does not exist.
+On BSD systems `Laminas\Filter\RealPath` doesn't fail if only the last path component doesn't exist, while other systems will return the value passed to the filter.
 
 ```php
 $filter = new Laminas\Filter\RealPath();
@@ -1155,12 +1153,11 @@ $filtered = $filter->filter($path);
 
 ### Non-Existing Paths
 
-Sometimes it is useful to get paths to files that do n0t exist; e.g., when you
-want to get the real path for a path you want to create. You can then either
-provide a `FALSE` `exists` value at initiation, or use `setExists()` to set it.
+Sometimes it is useful to get paths to files that do not exist; e.g., when you want to get the real path for a path you want to create.
+You can then provide `false` for the `exists` option during construction.
 
 ```php
-$filter = new Laminas\Filter\RealPath(false);
+$filter = new Laminas\Filter\RealPath(['exists' => false]);
 $path = '/www/var/path/../../non/existing/path';
 $filtered = $filter->filter($path);
 

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -24,6 +24,7 @@
     </PossiblyInvalidPropertyAssignmentValue>
     <PossiblyUnusedMethod>
       <code><![CDATA[applyFilterOnlyToStringableValuesAndStringableArrayValues]]></code>
+      <code><![CDATA[isOptions]]></code>
     </PossiblyUnusedMethod>
     <RedundantConditionGivenDocblockType>
       <code><![CDATA[is_object($options)]]></code>
@@ -536,27 +537,6 @@
       <code><![CDATA[Module]]></code>
     </UnusedClass>
   </file>
-  <file src="src/RealPath.php">
-    <DeprecatedClass>
-      <code><![CDATA[AbstractFilter]]></code>
-    </DeprecatedClass>
-    <InvalidPropertyAssignmentValue>
-      <code><![CDATA[$this->options]]></code>
-      <code><![CDATA[[
-        'exists' => true,
-    ]]]></code>
-    </InvalidPropertyAssignmentValue>
-    <PossiblyInvalidArgument>
-      <code><![CDATA[$existsOrOptions]]></code>
-      <code><![CDATA[$existsOrOptions]]></code>
-    </PossiblyInvalidArgument>
-    <RedundantCastGivenDocblockType>
-      <code><![CDATA[(bool) $flag]]></code>
-    </RedundantCastGivenDocblockType>
-    <RedundantConditionGivenDocblockType>
-      <code><![CDATA[$existsOrOptions !== null]]></code>
-    </RedundantConditionGivenDocblockType>
-  </file>
   <file src="src/StripTags.php">
     <DeprecatedClass>
       <code><![CDATA[AbstractFilter]]></code>
@@ -828,10 +808,9 @@
     </PossiblyUnusedMethod>
   </file>
   <file src="test/RealPathTest.php">
-    <InvalidArgument>
-      <code><![CDATA[['unknown']]]></code>
-    </InvalidArgument>
     <PossiblyUnusedMethod>
+      <code><![CDATA[returnExistingFilePathDataProvider]]></code>
+      <code><![CDATA[returnNonExistentPathDataProvider]]></code>
       <code><![CDATA[returnUnfilteredDataProvider]]></code>
     </PossiblyUnusedMethod>
   </file>

diff --git a/src/RealPath.php b/src/RealPath.php
@@ -4,122 +4,58 @@
 
 namespace Laminas\Filter;
 
-use Laminas\Stdlib\ErrorHandler;
-use Traversable;
-
 use function array_pop;
 use function explode;
+use function file_exists;
 use function getcwd;
 use function implode;
 use function is_string;
-use function preg_match;
-use function preg_replace;
 use function realpath;
 use function str_starts_with;
-use function stripos;
-use function substr;
 
 use const DIRECTORY_SEPARATOR;
-use const PHP_OS;
 
 /**
  * @psalm-type Options = array{
- *     exists: bool,
- *     ...
+ *     exists?: bool,
  * }
- * @template TOptions of Options
- * @extends AbstractFilter<TOptions>
+ * @implements FilterInterface<string>
  */
-final class RealPath extends AbstractFilter
+final class RealPath implements FilterInterface
 {
-    /** @var TOptions $options */
-    protected $options = [
-        'exists' => true,
-    ];
-
-    /**
-     * @param  bool|Traversable|Options $existsOrOptions Options to set
-     */
-    public function __construct($existsOrOptions = true)
-    {
-        if ($existsOrOptions !== null) {
-            if (! static::isOptions($existsOrOptions)) {
-                $this->setExists($existsOrOptions);
-            } else {
-                $this->setOptions($existsOrOptions);
-            }
-        }
-    }
+    private readonly bool $pathMustExist;
 
-    /**
-     * Sets if the path has to exist
-     * TRUE when the path must exist
-     * FALSE when not existing paths can be given
-     *
-     * @param  bool $flag Path must exist
-     * @return self
-     */
-    public function setExists($flag = true)
+    /** @param Options $options */
+    public function __construct(array $options = [])
     {
-        $this->options['exists'] = (bool) $flag;
-        return $this;
+        $this->pathMustExist = $options['exists'] ?? true;
     }
 
-    /**
-     * Returns true if the filtered path must exist
-     *
-     * @return bool
-     */
-    public function getExists()
-    {
-        return $this->options['exists'];
-    }
-
-    /**
-     * Defined by Laminas\Filter\FilterInterface
-     *
-     * Returns realpath($value)
-     *
-     * If the value provided is non-scalar, the value will remain unfiltered
-     *
-     * @psalm-return ($value is string ? string : mixed)
-     */
     public function filter(mixed $value): mixed
     {
         if (! is_string($value)) {
             return $value;
         }
-        $path = (string) $value;
 
-        if ($this->options['exists']) {
-            return realpath($path);
+        if ($this->pathMustExist && ! file_exists($value)) {
+            return $value;
         }
 
-        ErrorHandler::start();
-        $realpath = realpath($path);
-        ErrorHandler::stop();
-        if ($realpath !== false) {
-            return $realpath;
+        $realPath = realpath($value);
+
+        if ($realPath !== false) {
+            return $realPath;
         }
 
-        $drive = '';
-        if (stripos(PHP_OS, 'WIN') === 0) {
-            $path = preg_replace('/[\\\\\/]/', DIRECTORY_SEPARATOR, $path);
-            if (preg_match('/([a-zA-Z]\:)(.*)/', $path, $matches)) {
-                [, $drive, $path] = $matches;
-            } else {
-                $cwd   = getcwd();
-                $drive = substr($cwd, 0, 2);
-                if (! str_starts_with($path, DIRECTORY_SEPARATOR)) {
-                    $path = substr($cwd, 3) . DIRECTORY_SEPARATOR . $path;
-                }
-            }
-        } elseif (! str_starts_with($path, DIRECTORY_SEPARATOR)) {
+        $path = $value;
+
+        if (! str_starts_with($path, DIRECTORY_SEPARATOR)) {
             $path = getcwd() . DIRECTORY_SEPARATOR . $path;
         }
 
         $stack = [];
         $parts = explode(DIRECTORY_SEPARATOR, $path);
+
         foreach ($parts as $dir) {
             if ($dir !== '' && $dir !== '.') {
                 if ($dir === '..') {
@@ -130,6 +66,11 @@ public function filter(mixed $value): mixed
             }
         }
 
-        return $drive . DIRECTORY_SEPARATOR . implode(DIRECTORY_SEPARATOR, $stack);
+        return DIRECTORY_SEPARATOR . implode(DIRECTORY_SEPARATOR, $stack);
+    }
+
+    public function __invoke(mixed $value): mixed
+    {
+        return $this->filter($value);
     }
 }
diff --git a/test/RealPathTest.php b/test/RealPathTest.php
@@ -9,69 +9,83 @@
 use PHPUnit\Framework\TestCase;
 use stdClass;
 
+use function dirname;
+use function getcwd;
 use function str_contains;
 
-use const DIRECTORY_SEPARATOR;
 use const PHP_OS;
 
 class RealPathTest extends TestCase
 {
-    private RealPathFilter $filter;
+    public static function returnExistingFilePathDataProvider(): array
+    {
+        return [
+            [__DIR__ . '/_files/file.1'],
+            [__DIR__ . '/_files/../_files/file.1'],
+            [__DIR__ . '/_files/././file.1'],
+            [__DIR__ . '///_files///file.1'],
+        ];
+    }
 
-    public function setUp(): void
+    #[DataProvider('returnExistingFilePathDataProvider')]
+    public function testExistingFileReturnsRealPath(string $filePath): void
     {
-        $this->filter = new RealPathFilter();
+        $filter = new RealPathFilter();
+
+        $result = $filter->filter($filePath);
+
+        self::assertSame(__DIR__ . '/_files/file.1', $result);
     }
 
-    /**
-     * Ensures expected behavior for existing file
-     */
-    public function testFileExists(): void
+    public function testPathWithNonExistingPartsButRealResolutionIsNotValid(): void
     {
-        $filename = __DIR__ . '/_files/file.1';
-        $result   = $this->filter->filter($filename);
-        self::assertStringContainsString($filename, $result);
+        $filter = new RealPathFilter();
+
+        $path = __DIR__ . '/_files/foo/../bar/../file.1';
+
+        $result = $filter->filter($path);
+
+        self::assertSame($path, $result);
     }
 
-    /**
-     * Ensures expected behavior for nonexistent file
-     */
-    public function testFileNonexistent(): void
+    public function testNonexistentFileReturnsValuePassedToFilter(): void
     {
+        $filter = new RealPathFilter();
+
         $path = '/path/to/nonexistent';
+        self::assertSame($path, $filter->filter($path));
+    }
+
+    public function testBSDAllowsLastPortionToNotExist(): void
+    {
+        $filter = new RealPathFilter();
+
+        $path = './nonexistent';
+
         if (str_contains(PHP_OS, 'BSD')) {
-            self::assertSame($path, $this->filter->filter($path));
+            self::assertSame(getcwd() . '/nonexistent', $filter($path));
         } else {
-            self::assertSame(false, $this->filter->filter($path));
+            self::assertSame($path, $filter($path));
         }
     }
 
-    public function testGetAndSetExistsParameter(): void
+    public static function returnNonExistentPathDataProvider(): array
     {
-        self::assertTrue($this->filter->getExists());
-        $this->filter->setExists(false);
-        self::assertFalse($this->filter->getExists());
-
-        $this->filter->setExists(['unknown']);
-        self::assertTrue($this->filter->getExists());
+        return [
+            ['/nonexistent/absolute/path', '/nonexistent/absolute/path'],
+            ['/nonexistent/absolute/extra///slashes', '/nonexistent/absolute/extra/slashes'],
+            ['./nonexistent/relative/path', getcwd() . '/nonexistent/relative/path'],
+            ['./dropped/parts/../../path', getcwd() . '/path'],
+            ['../relative/from/parent', dirname(getcwd()) . '/relative/from/parent'],
+        ];
     }
 
-    public function testNonExistentPath(): void
+    #[DataProvider('returnNonExistentPathDataProvider')]
+    public function testNonExistentPathAllowed(string $path, string $expectedPath): void
     {
-        $filter = $this->filter;
-        $filter->setExists(false);
-
-        $path = __DIR__ . DIRECTORY_SEPARATOR . '_files';
-        self::assertSame($path, $filter($path));
-
-        $path2 = __DIR__ . DIRECTORY_SEPARATOR . '_files'
-               . DIRECTORY_SEPARATOR . '..' . DIRECTORY_SEPARATOR . '_files';
-        self::assertSame($path, $filter($path2));
+        $filter = new RealPathFilter(['exists' => false]);
 
-        $path3 = __DIR__ . DIRECTORY_SEPARATOR . '_files'
-               . DIRECTORY_SEPARATOR . '..' . DIRECTORY_SEPARATOR . '.'
-               . DIRECTORY_SEPARATOR . '_files';
-        self::assertSame($path, $filter($path3));
+        self::assertSame($expectedPath, $filter($path));
     }
 
     /** @return list<array{0: mixed}> */