Fix realpath vulnerability and hardcoded file input

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());
+    }
 }

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()`.

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);
             }
         }
     }

uploader/Services/FileValidator.php

@@ -0,0 +1,8 @@
+<?php
+
+namespace Maatify\Uploader\Services;
+
+class FileValidator
+{
+    // Future home for MIME and size validation
+}

uploader/Services/LocalFilesystem.php

@@ -0,0 +1,45 @@
+<?php
+
+namespace Maatify\Uploader\Services;
+
+use ErrorException;
+use Maatify\Logger\Logger;
+
+class LocalFilesystem
+{
+    public function createUploadFolder(string $upload_folder): bool
+    {
+        if (!file_exists($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($upload_folder, 0777, true);
+                return true;
+            } catch (ErrorException $e) {
+                Logger::RecordLog($e, 'uploader_error');
+                return false;
+            } finally {
+                restore_error_handler();
+            }
+        }
+        return true;
+    }
+
+    public function moveUploadedFile(string $tmp_name, string $file_target): bool
+    {
+        if (defined('PHPUNIT_TEST') || getenv('PHPUNIT_TEST') === '1') {
+            return copy($tmp_name, $file_target);
+        } else {
+            return move_uploaded_file($tmp_name, $file_target);
+        }
+    }
+}

uploader/UploadBase.php

@@ -12,6 +12,8 @@
 use ErrorException;
 use Maatify\Logger\Logger;
 use Maatify\Uploader\Mime\MimeValidate;
+use Maatify\Uploader\Services\FileValidator;
+use Maatify\Uploader\Services\LocalFilesystem;
 use Maatify\Uploader\Storage\StorageAdapterInterface;
 
 abstract class UploadBase extends MimeValidate
@@ -24,6 +26,22 @@
 
     protected ?StorageAdapterInterface $storageAdapter = null;
     protected bool $skipStoragePush = false;
+    protected string $file_input_name = 'file';
+
+    protected ?LocalFilesystem $localFilesystem = null;
+    protected ?FileValidator $fileValidator = null;
+
+    public function __construct()
+    {
+        $this->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 @@
             return;
         }
         $this->storageAdapter->upload($localPath, $relativePath);
-        @unlink($localPath);
+        unlink($localPath);
     }
     /**
      * Set the upload folder.
@@ -130,17 +148,17 @@
      */
     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,18 +170,26 @@
 
         // 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;
         }
 
         // 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,55 +199,21 @@
         $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.');
     }
 
     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);
     }
 }

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,
                     '<?php' . PHP_EOL
                     . 'header("Location: https://" . $_SERVER[\'HTTP_HOST\'] . "/404.php");'
                     . PHP_EOL
                 );
-                @fclose($f);
+                fclose($f);
             }
         }
     }