diff --git a/composer.json b/composer.json index a27f293..70196dc 100644 --- a/composer.json +++ b/composer.json @@ -10,7 +10,8 @@ "require": { "php": ">=8.1.0", "cakephp/filesystem": "^3.0", - "monolog/monolog": "^2.0" + "monolog/monolog": "^2.0", + "ondrej-vrto/php-filename-sanitize": "^1.4" }, "require-dev": { "phpunit/phpunit": "~10.0" diff --git a/src/Resumable.php b/src/Resumable.php index 294a447..00ec150 100644 --- a/src/Resumable.php +++ b/src/Resumable.php @@ -8,6 +8,7 @@ use Dilab\Network\Response; use Monolog\Logger; use Monolog\Handler\StreamHandler; +use OndrejVrto\FilenameSanitize\FilenameSanitize; class Resumable { @@ -159,18 +160,14 @@ public function getExtension() } /** - * Makes sure the orginal extension never gets overriden by user defined filename. + * Creates a safe name * - * @param string User defined filename - * @param string Original filename - * @return string Filename that always has an extension from the original file + * @param string $name Original name + * @return string A safer name */ - private function createSafeFilename($filename, $originalFilename) + private function createSafeName(string $name): string { - $filename = $this->removeExtension($filename); - $extension = $this->findExtension($originalFilename); - - return sprintf('%s.%s', $filename, $extension); + return FilenameSanitize::of($name)->get(); } public function handleTestChunk() @@ -227,9 +224,9 @@ private function createFileAndDeleteTmp($identifier, $filename) // if the user has set a custom filename if (null !== $this->filename) { - $finalFilename = $this->createSafeFilename($this->filename, $filename); + $finalFilename = $this->createSafeName($this->filename); } else { - $finalFilename = $filename; + $finalFilename = $this->createSafeName($filename); } // replace filename reference by the final file @@ -288,7 +285,7 @@ public function tmpChunkDir($identifier) if (!empty($this->instanceId)){ $tmpChunkDir .= $this->instanceId . DIRECTORY_SEPARATOR; } - $tmpChunkDir .= $identifier; + $tmpChunkDir .= $this->createSafeName($identifier); $this->ensureDirExists($tmpChunkDir); return $tmpChunkDir; } @@ -318,7 +315,7 @@ private function ensureDirExists($path) public function tmpChunkFilename($filename, $chunkNumber) { - return $filename . '.' . str_pad($chunkNumber, 4, 0, STR_PAD_LEFT); + return $this->createSafeName($filename) . '.' . str_pad($chunkNumber, 4, 0, STR_PAD_LEFT); } public function getExclusiveFileHandle($name) diff --git a/test/src/ResumableTest.php b/test/src/ResumableTest.php index 68673ee..38a8f31 100644 --- a/test/src/ResumableTest.php +++ b/test/src/ResumableTest.php @@ -235,6 +235,55 @@ public function testTmpChunkFile() $this->assertEquals($expected, $this->resumbable->tmpChunkFilename($filename,$chunkNumber)); } + public static function fileNameProvider(): array + { + return [ + ['example-file.png', 'example-file.png'], + ['../unsafe-one-level.txt', 'unsafe-one-level.txt'], + ]; + } + + /** + * @dataProvider fileNameProvider + */ + public function testResumableSanitizeFileName(string $filename, string $filenameSanitized): void + { + $resumableParams = array( + 'resumableChunkNumber'=> 1, + 'resumableTotalChunks'=> 1, + 'resumableChunkSize'=> 200, + 'resumableIdentifier'=> 'identifier', + 'resumableFilename'=> $filename, + 'resumableRelativePath'=> 'upload', + ); + + + $this->request->method('is') + ->willReturn(true); + + $this->request->method('data') + ->willReturn($resumableParams); + + $this->request->method('file') + ->willReturn(array( + 'name'=> 'mock.png', + 'tmp_name'=> 'test/files/mock.png.0003', + 'error'=> 0, + 'size'=> 27000, + )); + + $this->resumbable = new Resumable($this->request, $this->response); + $this->resumbable->tempFolder = 'test/tmp'; + $this->resumbable->uploadFolder = 'test/uploads'; + $this->resumbable->deleteTmpFolder = false; + $this->resumbable->handleChunk(); + + $this->assertFileExists('test/uploads/' . $filenameSanitized); + $this->assertFileExists('test/tmp/identifier/' . $filenameSanitized . '.0001'); + $this->assertTrue(unlink('test/tmp/identifier/' . $filenameSanitized . '.0001')); + $this->assertTrue(unlink('test/uploads/' . $filenameSanitized)); + } + public function testCreateFileFromChunks() { $files = array(