Merge pull request #6193 from kenjis/fix-Autoloader-sanitizeFilename

feat: Autoloader::sanitizeFilename() throws Exception

Author: kenjis

rector.php

@@ -94,6 +94,7 @@
         // check on constant compare
         UnwrapFutureCompatibleIfPhpVersionRector::class => [
             __DIR__ . '/system/CodeIgniter.php',
+            __DIR__ . '/system/Autoloader/Autoloader.php',
         ],
 
         // session handlers have the gc() method with underscored parameter `$max_lifetime`

system/Autoloader/Autoloader.php

@@ -15,6 +15,7 @@
 use Config\Autoload;
 use Config\Modules;
 use InvalidArgumentException;
+use RuntimeException;
 
 /**
  * An autoloader that uses both PSR4 autoloading, and traditional classmaps.
@@ -290,9 +291,9 @@ protected function includeFile(string $file)
     }
 
     /**
-     * Sanitizes a filename, replacing spaces with dashes.
+     * Check file path.
      *
-     * Removes special characters that are illegal in filenames on certain
+     * Checks special characters that are illegal in filenames on certain
      * operating systems and special characters requiring special escaping
      * to manipulate at the command line. Replaces spaces and consecutive
      * dashes with a single dash. Trim period, dash and underscore from beginning
@@ -306,10 +307,34 @@ public function sanitizeFilename(string $filename): string
         // Plus the forward slash for directory separators since this might be a path.
         // http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_278
         // Modified to allow backslash and colons for on Windows machines.
-        $filename = preg_replace('/[^0-9\p{L}\s\/\-\_\.\:\\\\]/u', '', $filename);
+        $result = preg_match_all('/[^0-9\p{L}\s\/\-_.:\\\\]/u', $filename, $matches);
+
+        if ($result > 0) {
+            $chars = implode('', $matches[0]);
+
+            throw new InvalidArgumentException(
+                'The file path contains special characters "' . $chars
+                . '" that are not allowed: "' . $filename . '"'
+            );
+        }
+        if ($result === false) {
+            if (version_compare(PHP_VERSION, '8.0.0', '>=')) {
+                $message = preg_last_error_msg();
+            } else {
+                $message = 'Regex error. error code: ' . preg_last_error();
+            }
+
+            throw new RuntimeException($message . '. filename: "' . $filename . '"');
+        }
 
         // Clean up our filename edges.
-        return trim($filename, '.-_');
+        $cleanFilename = trim($filename, '.-_');
+
+        if ($filename !== $cleanFilename) {
+            throw new InvalidArgumentException('The characters ".-_" are not allowed in filename edges: "' . $filename . '"');
+        }
+
+        return $cleanFilename;
     }
 
     private function loadComposerNamespaces(ClassLoader $composer): void

tests/system/Autoloader/AutoloaderTest.php

@@ -16,6 +16,8 @@
 use Config\Autoload;
 use Config\Modules;
 use Config\Services;
+use InvalidArgumentException;
+use RuntimeException;
 use UnnamespacedClass;
 
 /**
@@ -197,20 +199,44 @@ public function testloadClassNonNamespaced()
         $this->assertFalse($this->loader->loadClass('Modules'));
     }
 
-    public function testSanitizationSimply()
+    public function testSanitizationContailsSpecialChars()
     {
-        $test     = '${../path}!#/to/some/file.php_';
-        $expected = '/path/to/some/file.php';
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage(
+            'The file path contains special characters "${}!#" that are not allowed: "${../path}!#/to/some/file.php_"'
+        );
+
+        $test = '${../path}!#/to/some/file.php_';
+
+        $this->loader->sanitizeFilename($test);
+    }
+
+    public function testSanitizationFilenameEdges()
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage(
+            'The characters ".-_" are not allowed in filename edges: "/path/to/some/file.php_"'
+        );
 
-        $this->assertSame($expected, $this->loader->sanitizeFilename($test));
+        $test = '/path/to/some/file.php_';
+
+        $this->loader->sanitizeFilename($test);
+    }
+
+    public function testSanitizationRegexError()
+    {
+        $this->expectException(RuntimeException::class);
+
+        $test = mb_convert_encoding('クラスファイル.php', 'EUC-JP', 'UTF-8');
+
+        $this->loader->sanitizeFilename($test);
     }
 
     public function testSanitizationAllowUnicodeChars()
     {
-        $test     = 'Ä/path/to/some/file.php_';
-        $expected = 'Ä/path/to/some/file.php';
+        $test = 'Ä/path/to/some/file.php';
 
-        $this->assertSame($expected, $this->loader->sanitizeFilename($test));
+        $this->assertSame($test, $this->loader->sanitizeFilename($test));
     }
 
     public function testSanitizationAllowsWindowsFilepaths()