diff --git a/tests/UploadImageTest.php b/tests/UploadImageTest.php index 89a95ba..6f9f973 100644 --- a/tests/UploadImageTest.php +++ b/tests/UploadImageTest.php @@ -25,10 +25,6 @@ protected function setUp(): void // Use a writable temp directory for CI safety $this->uploadDir = sys_get_temp_dir() . '/uploader_tests'; - if (!is_dir($this->uploadDir)) { - mkdir($this->uploadDir, 0777, true); - } - $this->uploadImage = new UploadImage(); $this->uploadImage ->setUploadFolder($this->uploadDir) @@ -111,4 +107,27 @@ public function testMissingFile() $this->assertEquals(0, $result['uploaded']); $this->assertEquals('Missing file post.', $result['description']); } + + public function testExistingUploadFolder() + { + if (!is_dir($this->uploadDir)) { + mkdir($this->uploadDir, 0777, true); + } + + // Simulate a valid PNG file upload + $_FILES['file'] = [ + 'name' => 'test.png', + 'type' => 'image/png', + 'tmp_name' => __DIR__ . '/test_files/small_image.png', + 'error' => UPLOAD_ERR_OK, + 'size' => 500 * 1024, // 500 KB + ]; + + $result = $this->uploadImage->Upload(); + + $this->assertArrayHasKey('uploaded', $result); + $this->assertEquals(1, $result['uploaded']); + $this->assertArrayHasKey('file', $result); + $this->assertFileExists($this->uploadImage->getFileTarget()); + } } diff --git a/uploader-architecture-refactor-plan.md b/uploader-architecture-refactor-plan.md new file mode 100644 index 0000000..c607be0 --- /dev/null +++ b/uploader-architecture-refactor-plan.md @@ -0,0 +1,39 @@ +# Uploader Architecture Refactor Plan: Inheritance to Composition + +## 1. Current State & Responsibilities +The current architecture relies on a rigid and fragile inheritance tree: +`UploadFolderCreate` -> `MimeValidate` -> `UploadBase` -> (Specific Types: `UploadImage`, `UploadVideo`, etc.) + +* **`UploadFolderCreate`:** Handles directory creation and adding basic `index.php` protection files. +* **`MimeValidate`:** Manages sizing limits (max width, height, size) and validates MIME types against allowed mappings for various file categories (images, audio, video, PDF). It also contains success/error response formatting. +* **`UploadBase`:** Orchestrates the upload flow, handles the `$_FILES` global, moves files, integrates with cloud storage (`StorageAdapterInterface`), and coordinates file naming. + +This violates the Single Responsibility Principle and makes testing and extending individual pieces difficult. + +## 2. Proposed Architecture (Composition) +The goal is to move from deep inheritance to dependency injection (composition) while maintaining the existing public API of `UploadBase` so consuming code does not break. + +### Step 1: Extract a Filesystem Service +* **Create `Maatify\Uploader\Services\LocalFilesystem`** +* Move directory creation logic (`createUploadFolder`) and basic file operations (moving/copying) into this service. +* This service will handle the physical writing of files and checking paths (e.g., `realpath`). + +### Step 2: Extract a Validation Service +* **Create `Maatify\Uploader\Services\FileValidator`** +* Move MIME validation logic (`mimeValidate`, `mime2ext*` methods), size checks, and dimension checks into this service. +* This service will be responsible for evaluating the `$_FILES` input and returning whether it is valid or throwing specific validation exceptions. + +### Step 3: Refactor `UploadBase` +* Change `UploadBase` to no longer extend `MimeValidate`. +* Inject the new `LocalFilesystem` and `FileValidator` services into `UploadBase` (either via constructor injection or instantiated internally if not provided, to preserve backward compatibility). +* Delegate the validation and file moving logic from `UploadBase` to these respective services. +* Retain the specific abstract methods (`allowedExtensions`, `validateMime`) in `UploadBase` for the child classes, but have them configure the `FileValidator` rather than extending it directly. + +### Step 4: Deprecate Legacy Classes +* Mark `UploadFolderCreate` and `MimeValidate` as `@deprecated`. +* Eventually remove them in a future major release once consumers have fully migrated. + +## 3. Backward Compatibility Strategy +* Keep all public setter methods (`setUploadFolder`, `setMaxSize`, `setFileInputName`, etc.) on `UploadBase`. Under the hood, these setters will configure the newly injected services. +* Maintain the same array return structure (`['uploaded' => 1, 'file' => ...]`) for the `upload()` method. +* Existing child classes (`UploadImage`, etc.) will not need to change their implementations of `allowedExtensions()` or `validateMime()`. \ No newline at end of file diff --git a/uploader/DownloadStreamFile.php b/uploader/DownloadStreamFile.php index d76cb56..ef48872 100644 --- a/uploader/DownloadStreamFile.php +++ b/uploader/DownloadStreamFile.php @@ -20,17 +20,17 @@ public function DownloadFile(): void }else{ $file_type = strtolower(pathinfo((string)$this->file_path, PATHINFO_EXTENSION)); header('Content-Disposition: attachment;filename="' . $this->file_saved_name . '-' . time() . '.' . $file_type . '"'); - $file = @fopen((string)$this->file_path, "rb"); + $file = fopen((string)$this->file_path, "rb"); if ($file) { while (! feof($file)) { print(fread($file, 1024 * 8)); flush(); if (connection_status() != 0) { - @fclose($file); + fclose($file); die(); } } - @fclose($file); + fclose($file); } } } diff --git a/uploader/Services/FileValidator.php b/uploader/Services/FileValidator.php new file mode 100644 index 0000000..a6e9b22 --- /dev/null +++ b/uploader/Services/FileValidator.php @@ -0,0 +1,8 @@ +localFilesystem = new LocalFilesystem(); + $this->fileValidator = new FileValidator(); + } + + public function setFileInputName(string $name): self + { + $this->file_input_name = $name; + return $this; + } public function setStorageAdapter(StorageAdapterInterface $adapter): static { @@ -37,7 +55,7 @@ protected function pushToStorage(string $localPath, string $relativePath): void return; } $this->storageAdapter->upload($localPath, $relativePath); - @unlink($localPath); + unlink($localPath); } /** * Set the upload folder. @@ -130,17 +148,17 @@ abstract protected function validateMime(string $mime): string; */ public function upload(): array { - if (empty($_FILES["file"]) || !is_array($_FILES["file"]) || empty($_FILES["file"]["tmp_name"])) { + if (empty($_FILES[$this->file_input_name]) || !is_array($_FILES[$this->file_input_name]) || empty($_FILES[$this->file_input_name]["tmp_name"])) { return $this->returnError('Missing file post.'); } // Check for any upload errors - if ($_FILES["file"]["error"] !== UPLOAD_ERR_OK) { - return $this->returnError('File upload error: ' . $_FILES["file"]["error"]); + if ($_FILES[$this->file_input_name]["error"] !== UPLOAD_ERR_OK) { + return $this->returnError('File upload error: ' . $_FILES[$this->file_input_name]["error"]); } // Get the MIME type of the uploaded file - $mime = mime_content_type((string)$_FILES["file"]["tmp_name"]); + $mime = mime_content_type((string)$_FILES[$this->file_input_name]["tmp_name"]); if ($mime === false) { return $this->returnError('Could not determine mime type.'); } @@ -152,8 +170,8 @@ public function upload(): array // Generate a unique filename if none is provided if (empty($this->file_name)) { - $fileName = round(microtime(true) * 1000) . uniqid(); - $file = $this->uploaded_for_id . '_' . time() . "_" . $fileName . uniqid() . '.' . $this->extension; + $fileName = round(microtime(true) * 1000) . bin2hex(random_bytes(16)); + $file = $this->uploaded_for_id . '_' . time() . "_" . $fileName . bin2hex(random_bytes(16)) . '.' . $this->extension; } else { $file = $this->file_name . '.' . $this->extension; } @@ -161,9 +179,17 @@ public function upload(): array // Sanitize the filename using basename and preg_replace for security $file = preg_replace('/[^a-zA-Z0-9_\-\.]/', '', basename($file)); + // Create the upload folder if it doesn't exist + if (!$this->createUploadFolder()) { + return $this->returnError('Failed to create upload folder.'); + } + // Set the target path for the file upload - // Set the target path for the file upload - $basePath = (string)realpath($this->upload_folder); + $basePath = realpath($this->upload_folder); + if ($basePath === false) { + return $this->returnError('Invalid upload folder.'); + } + $basePath = (string)$basePath; $target_path = $basePath . '/' . $file; if (! str_starts_with($target_path, $basePath)) { @@ -173,26 +199,14 @@ public function upload(): array $this->file_target = $target_path; // Check the file size against the maximum allowed size (if defined) - if (!empty($this->max_size) && $_FILES["file"]["size"] > $this->max_size) { + if (!empty($this->max_size) && $_FILES[$this->file_input_name]["size"] > $this->max_size) { return $this->returnError("Your file is too large, cannot be more than " . ($this->max_size / self::MB) . " MB."); } - // Create the upload folder if it doesn't exist - if (!$this->createUploadFolder()) { - return $this->returnError('Failed to create upload folder.'); - } - // Move the uploaded file to the target directory and verify success - if (defined('PHPUNIT_TEST') || getenv('PHPUNIT_TEST') === '1') { - if (copy($_FILES["file"]["tmp_name"], $this->file_target)) { - $this->pushToStorage($this->file_target, (string)$file); - return $this->returnSuccess((string)$file); - } - } else { - if (move_uploaded_file($_FILES["file"]["tmp_name"], $this->file_target)) { - $this->pushToStorage($this->file_target, (string)$file); - return $this->returnSuccess((string)$file); - } + if ($this->localFilesystem->moveUploadedFile($_FILES[$this->file_input_name]["tmp_name"], $this->file_target)) { + $this->pushToStorage($this->file_target, (string)$file); + return $this->returnSuccess((string)$file); } return $this->returnError('Failed to move uploaded file.'); @@ -200,28 +214,6 @@ public function upload(): array protected function createUploadFolder(): bool { - if (!file_exists($this->upload_folder)) { - set_error_handler( - /** - * @throws ErrorException - */ - function ($errno, $errstr, $errfile, $errline) { - if (0 === error_reporting()) { - return false; - } - throw new ErrorException($errstr, 0, $errno, $errfile, $errline); - }); - - try { - mkdir($this->upload_folder, 0777, true); - return true; - } catch (ErrorException $e) { - Logger::RecordLog($e, 'uploader_error'); - return false; - } finally { - restore_error_handler(); - } - } - return true; + return $this->localFilesystem->createUploadFolder($this->upload_folder); } } diff --git a/uploader/UploadFolderCreate.php b/uploader/UploadFolderCreate.php index 9ee7cbf..326fc93 100644 --- a/uploader/UploadFolderCreate.php +++ b/uploader/UploadFolderCreate.php @@ -17,15 +17,15 @@ protected function creatUploadFolder(): void { if (! file_exists($this->upload_folder)) { mkdir($this->upload_folder); - $f = @fopen($this->upload_folder . '/index.php', 'a+'); + $f = fopen($this->upload_folder . '/index.php', 'a+'); if ($f) { - @fputs( + fputs( $f, '