Make file and folder names safe

code-lts · Dec 7, 2023 · b61c252 · b61c252
1 parent a09ba7a
commit b61c252
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 63 deletions.
diff --git a/README.md b/README.md
@@ -36,23 +36,27 @@ $resumable->process();
 ### Setting custom filename(s)
 
 ```php
-// NOTE: this fork removed the getOriginalFilename() parameter to return without an extension.
 $originalName = $resumable->getOriginalFilename(); // will give you the original end-user file-name
 
-// do some slugification or whatever you need...
-$slugifiedname = my_slugify($originalName); // this is up to you, it as ported out of the library.
-$resumable->setFilename($slugifiedname);
+$mySafeName = Security::sanitizeFileName($request->query('resumableFilename'));
+$resumable->setFilename($mySafeName);// Override the safe filename
 
 // process upload as normal
 $resumable->process();
 
 // you can also get file information after the upload is complete
 if (true === $resumable->isUploadComplete()) { // true when the final file has been uploaded and chunks reunited.
-    $extension = $resumable->getExtension();
     $filename = $resumable->getFilename();
 }
 ```
 
+## Removed features
+
+- `$resumable->getOriginalFilename()` does not have a parameter to return the name without the extension
+- `$resumable->getExtension()` implement the logic yourself
+- `preProcess()` no longer exists, it was not very useful
+- the default value of `uploadFolder` was `test/files/uploads` and is now `uploads`
+
 ## Testing
 
 ```sh

diff --git a/composer.json b/composer.json
@@ -15,7 +15,8 @@
         "php": "^7.4 || ^8.0",
         "psr/log": "^1.1",
         "psr/http-message": "^1.0",
-        "knplabs/gaufrette": "^0.11.1"
+        "knplabs/gaufrette": "^0.11.1",
+        "ondrej-vrto/php-filename-sanitize": "^1.4"
     },
     "require-dev": {
         "phpunit/phpunit": "^7 || ^8 || ^9 || ^10",

diff --git a/src/Resumable.php b/src/Resumable.php
@@ -11,6 +11,7 @@
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\UploadedFileInterface;
+use OndrejVrto\FilenameSanitize\FilenameSanitize;
 
 class Resumable
 {
@@ -24,7 +25,7 @@ class Resumable
     public $tempFolder = 'tmp';
 
     /** @var string */
-    public $uploadFolder = 'test/files/uploads';
+    public $uploadFolder = 'uploads';
 
     /**
      * For testing purposes
@@ -70,8 +71,6 @@ class Resumable
 
     protected $filepath;
 
-    protected $extension;
-
     protected $originalFilename;
 
     /** @var bool */
@@ -102,8 +101,6 @@ public function __construct(
         }
 
         $this->logger = $logger;
-
-        $this->preProcess();
     }
 
     public static function getLocalFileSystem(string $baseDir): Filesystem
@@ -120,26 +117,6 @@ public function setResumableOption(array $resumableOption): void
         $this->resumableOption = array_merge($this->resumableOption, $resumableOption);
     }
 
-    /**
-     * sets original filename and extension, blah blah
-     */
-    public function preProcess(): void
-    {
-        if (!empty($this->resumableParams())) {
-            if (!empty($this->request->getUploadedFiles())) {
-                $this->originalFilename = $this->resumableParam('filename');
-                $this->extension        = $this->findExtension($this->originalFilename);
-                $this->log(
-                    'Defined extension',
-                    [
-                        'extension' => $this->extension,
-                        'originalFilename' => $this->originalFilename,
-                    ]
-                );
-            }
-        }
-    }
-
     /**
      * @return ResponseInterface|null
      */
@@ -209,32 +186,14 @@ public function getFilepath(): string
     }
 
     /**
-     * Get final extension.
+     * Creates a safe name
      *
-     * @return string Final extension name
+     * @param string $name Original name
+     * @return string A safer name
      */
-    public function getExtension(): string
+    private function createSafeName(string $name): string
     {
-        return $this->extension;
-    }
-
-    /**
-     * Makes sure the original extension never gets overridden by user defined filename.
-     *
-     * @param string User defined filename
-     * @param string Original filename
-     * @return string Filename that always has an extension from the original file
-     */
-    private function createSafeFilename(string $filename, string $originalFilename): string
-    {
-        return sprintf('%s.%s', $filename, $extension);
-    }
-
-    private function findExtension(string $filename): string
-    {
-        $parts = explode('.', basename($filename));
-
-        return end($parts);
+        return FilenameSanitize::of($name)->get();
     }
 
     /**
@@ -323,17 +282,16 @@ private function createFileAndDeleteTmp(string $identifier, ?string $filename):
             $chunkDir
         )['keys'];
 
-        $finalFilename = $filename;
+        $this->originalFilename = $filename;
 
         // if the user has set a custom filename
-        if (null !== $this->filename) {
-            $finalFilename = $this->createSafeFilename($this->filename, $filename);
-            $this->log('Created safe filename', ['finalFilename' => $finalFilename]);
+        if (null === $this->filename) {
+            $this->filename = $this->createSafeName($this->originalFilename);
+            $this->log('Created safe filename', ['finalFilename' => $this->filename]);
         }
 
         // replace filename reference by the final file
-        $this->filepath  = $this->uploadFolder . DIRECTORY_SEPARATOR . $finalFilename;
-        $this->extension = $this->findExtension($this->filepath);
+        $this->filepath = $this->uploadFolder . DIRECTORY_SEPARATOR . $this->filename;
 
         $finalFileCreated = $this->createFileFromChunks($chunkFiles, $this->filepath);
 
@@ -409,7 +367,7 @@ public function isChunkUploaded(string $identifier, string $filename, int $chunk
 
     public function tmpChunkDir(string $identifier): string
     {
-        return $this->tempFolder . DIRECTORY_SEPARATOR . $identifier;
+        return $this->tempFolder . DIRECTORY_SEPARATOR . $this->createSafeName($identifier);
     }
 
     /**

diff --git a/test/src/ResumableTest.php b/test/src/ResumableTest.php
@@ -141,7 +141,8 @@ public function testHandleTestChunk(): void
 
     public function testHandleChunk(): void
     {
-        $uploadedFile     = tempnam(sys_get_temp_dir(), 'resumable.js_mock.png');
+        $uploadedFile = strtolower(tempnam(sys_get_temp_dir(), 'resumable.js-mock')) . '.png';
+        touch($uploadedFile);// Create the uploaded file
         $uploadedFileName = basename($uploadedFile);
         $resumableParams  = [
             'resumableChunkNumber' => 3,
@@ -167,7 +168,6 @@ public function testHandleChunk(): void
                 ]
             );
 
-        touch($uploadedFile);// Create the uploaded file
         $this->resumable                  = new Resumable($this->request, $this->response);
         $this->resumable->tempFolder      = 'test/tmp';
         $this->resumable->uploadFolder    = 'test/uploads';
@@ -216,6 +216,56 @@ public function testResumableParamsGetRequest(): void
         $this->assertEquals($resumableParams, $this->resumable->resumableParams());
     }
 
+    public static function fileNameProvider(): array
+    {
+        return [
+            ['mock.png', 'mock.png'],
+            ['../unsafe-one-level.txt', 'unsafe-one-level.txt'],
+        ];
+    }
+
+    /**
+     * @dataProvider fileNameProvider
+     */
+    public function testResumableSanitizeFileName(string $filename, string $filenameSanitized): void
+    {
+        $resumableParams = [
+            'resumableChunkNumber' => 1,
+            'resumableTotalChunks' => 1,
+            'resumableChunkSize' => 200,
+            'resumableIdentifier' => 'identifier',
+            'resumableFilename' => $filename,
+            'resumableRelativePath' => 'upload',
+        ];
+
+        $uploadedFile = tempnam(sys_get_temp_dir(), 'resumable.js_sanitize');
+
+        $this->request = $this->psr17Factory->createServerRequest(
+            'POST',
+            'http://example.com'
+        )
+            ->withParsedBody($resumableParams)
+            ->withUploadedFiles(
+                [
+                new UploadedFile(
+                    $uploadedFile,
+                    27000, // Size
+                    0 // Error status
+                ),
+                ]
+            );
+
+        $this->resumable               = new Resumable($this->request, $this->response);
+        $this->resumable->uploadFolder = 'upld';
+
+        $this->assertNotNull($this->resumable->handleChunk());
+        unlink($uploadedFile);
+        $this->assertTrue($this->resumable->isUploadComplete());
+        $this->assertSame($filename, $this->resumable->getOriginalFilename());
+        $this->assertSame($filenameSanitized, $this->resumable->getFilename());
+        $this->assertSame('upld/' . $filenameSanitized, $this->resumable->getFilepath());
+    }
+
     public static function isFileUploadCompleteProvider(): array
     {
         return [