From 7dc64e7c9909cce1267580378136f08dd74a7e62 Mon Sep 17 00:00:00 2001 From: Thorsten Rinne Date: Sat, 5 Oct 2024 19:34:06 +0200 Subject: [PATCH] refactor: get rid of direct access of $_FILES --- phpmyfaq/admin/backup.import.php | 16 +++++++----- phpmyfaq/admin/category.main.php | 4 ++- .../phpMyFAQ/Attachment/AttachmentFactory.php | 21 --------------- .../Attachment/Filesystem/AbstractFile.php | 11 ++++---- phpmyfaq/src/phpMyFAQ/Category/Image.php | 2 -- .../Administration/AttachmentController.php | 26 +++++++++---------- .../Administration/ImageController.php | 20 +++++++------- 7 files changed, 39 insertions(+), 61 deletions(-) diff --git a/phpmyfaq/admin/backup.import.php b/phpmyfaq/admin/backup.import.php index f40461df27..813a56a9ea 100644 --- a/phpmyfaq/admin/backup.import.php +++ b/phpmyfaq/admin/backup.import.php @@ -26,6 +26,7 @@ use phpMyFAQ\Template\TwigWrapper; use phpMyFAQ\Translation; use phpMyFAQ\User\CurrentUser; +use Symfony\Component\HttpFoundation\Request; if (!defined('IS_VALID_PHPMYFAQ')) { http_response_code(400); @@ -48,14 +49,17 @@ 'adminHeaderRestore' => Translation::get('ad_csv_rest') ]; - if (isset($_FILES['userfile']) && 0 === $_FILES['userfile']['error']) { + $request = Request::createFromGlobals(); + $file = $request->files->get('userfile'); + + if ($file && $file->isValid()) { $ok = 1; $fileInfo = new finfo(FILEINFO_MIME_ENCODING); $dbHelper = new DatabaseHelper($faqConfig); $backup = new Backup($faqConfig, $dbHelper); - if ('utf-8' !== $fileInfo->file($_FILES['userfile']['tmp_name'])) { + if ('utf-8' !== $fileInfo->file($file->getPathname())) { $templateVars = [ ...$templateVars, 'errorMessageWrongEncoding' => 'This file is not UTF-8 encoded.' @@ -63,16 +67,16 @@ $ok = 0; } - $handle = fopen($_FILES['userfile']['tmp_name'], 'r'); + $handle = fopen($file->getPathname(), 'r'); $backupData = fgets($handle, 65536); $versionFound = Strings::substr($backupData, 0, 9); $versionExpected = '-- pmf' . substr($faqConfig->getVersion(), 0, 3); $queries = []; - $fileName = $_FILES['userfile']['name']; + $fileName = $file->getClientOriginalName(); try { - $verification = $backup->verifyBackup(file_get_contents($_FILES['userfile']['tmp_name']), $fileName); + $verification = $backup->verifyBackup(file_get_contents($file->getPathname()), $fileName); if ($verification) { $ok = 1; } else { @@ -184,7 +188,7 @@ ]; } } else { - $errorMessage = match ($_FILES['userfile']['error']) { + $errorMessage = match ($file->getError()) { 1 => 'The uploaded file exceeds the upload_max_filesize directive in php.ini.', 2 => 'The uploaded file exceeds the MAX_FILE_SIZE directive that was specified in the ' . 'HTML form.', 3 => 'The uploaded file was only partially uploaded.', diff --git a/phpmyfaq/admin/category.main.php b/phpmyfaq/admin/category.main.php index e5d209121f..52fed2a6f0 100644 --- a/phpmyfaq/admin/category.main.php +++ b/phpmyfaq/admin/category.main.php @@ -31,6 +31,7 @@ use phpMyFAQ\Template\TwigWrapper; use phpMyFAQ\Translation; use phpMyFAQ\User\CurrentUser; +use Symfony\Component\HttpFoundation\Request; if (!defined('IS_VALID_PHPMYFAQ')) { http_response_code(400); @@ -45,7 +46,8 @@ // // Image upload // -$uploadedFile = (isset($_FILES['image']['size']) && $_FILES['image']['size'] > 0) ? $_FILES['image'] : []; +$request = Request::createFromGlobals(); +$uploadedFile = $request->files->get('image'); $categoryImage = new Image($faqConfig); $categoryImage->setUploadedFile($uploadedFile); diff --git a/phpmyfaq/src/phpMyFAQ/Attachment/AttachmentFactory.php b/phpmyfaq/src/phpMyFAQ/Attachment/AttachmentFactory.php index 6af7734f68..298ab66016 100644 --- a/phpmyfaq/src/phpMyFAQ/Attachment/AttachmentFactory.php +++ b/phpmyfaq/src/phpMyFAQ/Attachment/AttachmentFactory.php @@ -116,25 +116,4 @@ public static function init(string $defaultKey, bool $encryptionEnabled): void self::$encryptionEnabled = $encryptionEnabled; } } - - /** - * Re-arranges the $_FILES array for multiple file uploads. - * - * @param array $filePost - * @return array - */ - public static function rearrangeUploadedFiles(array $filePost): array - { - $filesArray = []; - $filesCount = is_countable($filePost['name']) ? count($filePost['name']) : 0; - $filesKeys = array_keys($filePost); - - for ($i = 0; $i < $filesCount; ++$i) { - foreach ($filesKeys as $fileKey) { - $filesArray[$i][$fileKey] = $filePost[$fileKey][$i]; - } - } - - return $filesArray; - } } diff --git a/phpmyfaq/src/phpMyFAQ/Attachment/Filesystem/AbstractFile.php b/phpmyfaq/src/phpMyFAQ/Attachment/Filesystem/AbstractFile.php index 7d5f78d7e7..d9ad286eba 100644 --- a/phpmyfaq/src/phpMyFAQ/Attachment/Filesystem/AbstractFile.php +++ b/phpmyfaq/src/phpMyFAQ/Attachment/Filesystem/AbstractFile.php @@ -20,6 +20,7 @@ use phpMyFAQ\Attachment\Filesystem\File\FileException; use RecursiveDirectoryIterator; use RecursiveIteratorIterator; +use Symfony\Component\HttpFoundation\Request; /** * Class File. @@ -97,23 +98,21 @@ abstract public function putChunk($chunk); /** * Deletes the file. * - * @see inc/PMF_Attachment/Filesystem/PMF_Attachment_Filesystem_Entry#delete() - * * @throws FileException */ public function delete(): bool { - $deleted = true; - if ($this->handle) { fclose($this->handle); } - if (isset($_FILES['userfile']) && $this->path !== $_FILES['userfile']['tmp_name'] && file_exists($this->path)) { + $request = Request::createFromGlobals(); + $uploadedFile = $request->files->get('userfile'); + if ($uploadedFile && $this->path !== $uploadedFile->getPathname() && file_exists($this->path)) { return $this->deleteDir(dirname($this->path)); } - return $deleted; + return true; } /** diff --git a/phpmyfaq/src/phpMyFAQ/Category/Image.php b/phpmyfaq/src/phpMyFAQ/Category/Image.php index 6a507e9225..25191f1b12 100644 --- a/phpmyfaq/src/phpMyFAQ/Category/Image.php +++ b/phpmyfaq/src/phpMyFAQ/Category/Image.php @@ -47,8 +47,6 @@ public function __construct(private readonly Configuration $configuration) /** * Sets the uploaded file array from $_FILES. - * - * */ public function setUploadedFile(array $uploadedFile): Image { diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Administration/AttachmentController.php b/phpmyfaq/src/phpMyFAQ/Controller/Administration/AttachmentController.php index c1ac3eabbb..061bb305e5 100644 --- a/phpmyfaq/src/phpMyFAQ/Controller/Administration/AttachmentController.php +++ b/phpmyfaq/src/phpMyFAQ/Controller/Administration/AttachmentController.php @@ -49,12 +49,10 @@ public function delete(Request $request): JsonResponse $attachment = AttachmentFactory::create($deleteData->attId); if ($attachment->delete()) { - $result = ['success' => Translation::get('msgAttachmentsDeleted')]; - return $this->json($result, Response::HTTP_OK); - } else { - $result = ['error' => Translation::get('ad_att_delfail')]; - return $this->json($result, Response::HTTP_BAD_REQUEST); + return $this->json(['success' => Translation::get('msgAttachmentsDeleted')], Response::HTTP_OK); } + + return $this->json(['error' => Translation::get('ad_att_delfail')], Response::HTTP_BAD_REQUEST); } catch (AttachmentException $attachmentException) { $result = ['error' => $attachmentException->getMessage()]; return $this->json($result, Response::HTTP_INTERNAL_SERVER_ERROR); @@ -99,24 +97,24 @@ public function upload(Request $request): JsonResponse { $this->userHasPermission(PermissionType::ATTACHMENT_ADD); - if (!isset($_FILES['filesToUpload'])) { - return $this->json([], Response::HTTP_BAD_REQUEST); + $files = $request->files->get('filesToUpload'); + + if (!$files) { + return $this->json(['error' => 'No files to upload.'], Response::HTTP_BAD_REQUEST); } - $files = AttachmentFactory::rearrangeUploadedFiles($_FILES['filesToUpload']); $uploadedFiles = []; - foreach ($files as $file) { if ( - is_uploaded_file($file['tmp_name']) && - $file['size'] <= Configuration::getConfigurationInstance()->get('records.maxAttachmentSize') && - $file['type'] !== "text/html" + $file->isValid() && + $file->getSize() <= $this->configuration->get('records.maxAttachmentSize') && + $file->getMimeType() !== 'text/html' ) { $attachment = AttachmentFactory::create(); $attachment->setRecordId($request->request->get('record_id')); $attachment->setRecordLang($request->request->get('record_lang')); try { - if (!$attachment->save($file['tmp_name'], $file['name'])) { + if (!$attachment->save($file->getPathname(), $file->getClientOriginalName())) { throw new AttachmentException(); } } catch (AttachmentException) { @@ -130,7 +128,7 @@ public function upload(Request $request): JsonResponse 'faqLanguage' => $request->request->get('record_lang') ]; } else { - return $this->json('The image is too large.', Response::HTTP_BAD_REQUEST); + return $this->json(['error' => 'The image is too large.'], Response::HTTP_BAD_REQUEST); } } diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Administration/ImageController.php b/phpmyfaq/src/phpMyFAQ/Controller/Administration/ImageController.php index 604de252bf..03635f06b4 100644 --- a/phpmyfaq/src/phpMyFAQ/Controller/Administration/ImageController.php +++ b/phpmyfaq/src/phpMyFAQ/Controller/Administration/ImageController.php @@ -38,8 +38,7 @@ public function upload(Request $request): JsonResponse { $this->userHasPermission(PermissionType::FAQ_EDIT); - $configuration = Configuration::getConfigurationInstance(); - $uploadDir = PMF_CONTENT_DIR . '/user/images/'; + $uploadDir = PMF_CONTENT_DIR . '/user/images/'; $validFileExtensions = ['gif', 'jpg', 'jpeg', 'png']; $timestamp = time(); @@ -47,34 +46,33 @@ public function upload(Request $request): JsonResponse return $this->json(['error' => Translation::get('err_NotAuth')], Response::HTTP_UNAUTHORIZED); } - reset($_FILES); - $temp = current($_FILES); + $file = $request->files->get('file'); $headers = []; - if (is_uploaded_file($temp['tmp_name'])) { + if ($file && $file->isValid()) { if ( $request->server->get('HTTP_ORIGIN') !== null && - $request->server->get('HTTP_ORIGIN') . '/' === $configuration->getDefaultUrl() + $request->server->get('HTTP_ORIGIN') . '/' === $this->configuration->getDefaultUrl() ) { $headers = ['Access-Control-Allow-Origin', $request->server->get('HTTP_ORIGIN')]; } // Sanitize input - if (preg_match("/([^\w\s\d\-_~,;:\[\]\(\).])|([\.]{2,})/", (string) $temp['name'])) { + if (preg_match("/([^\w\s\d\-_~,;:\[\]\(\).])|([\.]{2,})/", $file->getClientOriginalName())) { return $this->json([], Response::HTTP_BAD_REQUEST, $headers); } // Verify extension - if (!in_array(strtolower(pathinfo((string) $temp['name'], PATHINFO_EXTENSION)), $validFileExtensions)) { + if (!in_array(strtolower($file->getClientOriginalExtension()), $validFileExtensions)) { return $this->json([], Response::HTTP_BAD_REQUEST, $headers); } // Accept upload if there was no origin, or if it is an accepted origin - $fileName = $timestamp . '_' . $temp['name']; - move_uploaded_file($temp['tmp_name'], $uploadDir . $fileName); + $fileName = $timestamp . '_' . $file->getClientOriginalName(); + $file->move($uploadDir, $fileName); // Respond to the successful upload with JSON with the full URL of the uploaded image. return $this->json( - ['location' => $configuration->getDefaultUrl() . 'content/user/images/' . $fileName], + ['location' => $this->configuration->getDefaultUrl() . 'content/user/images/' . $fileName], Response::HTTP_OK, $headers );