From 43206127d8810a7801fde4818315f19656f1d176 Mon Sep 17 00:00:00 2001 From: Bahlai Pavlo Date: Fri, 9 Jan 2026 16:10:18 +0200 Subject: [PATCH 1/5] magento/magento2#39720: Can't upload image for custom customer address attribute --- .../Customer/Model/Metadata/Form/File.php | 26 +++++++++++++++--- .../Customer/Model/Metadata/Form/Image.php | 27 ++++++++++++++++--- 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/app/code/Magento/Customer/Model/Metadata/Form/File.php b/app/code/Magento/Customer/Model/Metadata/Form/File.php index a2a57c142dd90..32b5f8134d333 100644 --- a/app/code/Magento/Customer/Model/Metadata/Form/File.php +++ b/app/code/Magento/Customer/Model/Metadata/Form/File.php @@ -194,8 +194,15 @@ public function extractValue(\Magento\Framework\App\RequestInterface $request) */ protected function _validateByRules($value) { - $label = $value['name']; + $label = $value['name'] ?? $value['file'] ?? ''; $rules = $this->getAttribute()->getValidationRules(); + + // For UI component uploads, get name from file path if not provided + if (empty($value['name']) && !empty($value['file'])) { + $value['name'] = basename($value['file']); + $label = $value['name']; + } + $extension = $this->ioFile->getPathInfo($value['name'])['extension']; $fileExtensions = ArrayObjectSearch::getArrayElementByName( $rules, @@ -216,7 +223,9 @@ protected function _validateByRules($value) return $this->_fileValidator->getMessages(); } - if (!$this->_isUploadedFile($value['tmp_name'])) { + // Determine file path for validation + $filePath = $value['tmp_name'] ?? $value['file'] ?? ''; + if (!$this->_isUploadedFile($filePath)) { return [__('"%1" is not a valid file.', $label)]; } @@ -225,7 +234,15 @@ protected function _validateByRules($value) 'max_file_size' ); if ($maxFileSize !== null) { - $size = $value['size']; + $size = $value['size'] ?? 0; + // For UI component uploads, get file size if not provided + if ($size === 0 && !empty($value['file'])) { + $temporaryFile = FileProcessor::TMP_DIR . '/' . basename($value['file']); + if ($this->fileProcessor->isExist($temporaryFile)) { + $stat = $this->fileProcessor->getStat($temporaryFile); + $size = $stat['size'] ?? 0; + } + } if ($maxFileSize < $size) { return [__('"%1" exceeds the allowed file size.', $label)]; } @@ -274,7 +291,8 @@ public function validateValue($value) $label = $attribute->getStoreLabel(); $toDelete = !empty($value['delete']) ? true : false; - $toUpload = !empty($value['tmp_name']) ? true : false; + // Check both tmp_name (traditional upload) and file (UI component upload) + $toUpload = !empty($value['tmp_name']) || (!empty($value['file']) && $value['file'] !== $this->_value); if (!$toUpload && !$toDelete && $this->_value) { return true; diff --git a/app/code/Magento/Customer/Model/Metadata/Form/Image.php b/app/code/Magento/Customer/Model/Metadata/Form/Image.php index f87540c5bd9dc..9b7c1518b3e4e 100644 --- a/app/code/Magento/Customer/Model/Metadata/Form/Image.php +++ b/app/code/Magento/Customer/Model/Metadata/Form/Image.php @@ -136,17 +136,32 @@ public function __construct( */ protected function _validateByRules($value) { - $label = $value['name']; + $label = $value['name'] ?? $value['file'] ?? ''; $rules = $this->getAttribute()->getValidationRules(); + // For UI component uploads, get name from file path if not provided + if (empty($value['name']) && !empty($value['file'])) { + $value['name'] = basename($value['file']); + $label = $value['name']; + } + + // Determine file path for validation + $filePath = $value['tmp_name'] ?? null; + + // For UI component uploads, construct the full temporary file path + if (empty($filePath) && !empty($value['file'])) { + $tmpFileName = ltrim($value['file'], '/'); + $filePath = $this->mediaEntityTmpReadDirectory->getAbsolutePath($tmpFileName); + } + try { // phpcs:ignore Magento2.Functions.DiscouragedFunction - $imageProp = getimagesize($value['tmp_name']); + $imageProp = getimagesize($filePath); } catch (\Throwable $e) { $imageProp = false; } - if (!$this->_isUploadedFile($value['tmp_name']) || !$imageProp) { + if (!$this->_isUploadedFile($filePath) || !$imageProp) { return [__('"%1" is not a valid file.', $label)]; } @@ -170,7 +185,11 @@ protected function _validateByRules($value) ); $errors = []; if ($maxFileSize !== null) { - $size = $value['size']; + $size = $value['size'] ?? 0; + // For UI component uploads, get file size if not provided + if ($size === 0 && !empty($filePath) && file_exists($filePath)) { + $size = filesize($filePath); + } if ($maxFileSize < $size) { $errors[] = __('"%1" exceeds the allowed file size.', $label); } From 4bceb81b446e71987c439355b672afc3a14c1780 Mon Sep 17 00:00:00 2001 From: Bahlai Pavlo Date: Fri, 9 Jan 2026 16:54:40 +0200 Subject: [PATCH 2/5] magento/magento2#39720: Can't upload image for custom customer address attribute --- .../Customer/Model/Metadata/Form/File.php | 81 +++++++++++++++++-- .../Customer/Model/Metadata/Form/Image.php | 56 ++++++++++++- 2 files changed, 128 insertions(+), 9 deletions(-) diff --git a/app/code/Magento/Customer/Model/Metadata/Form/File.php b/app/code/Magento/Customer/Model/Metadata/Form/File.php index 32b5f8134d333..60b07380aba2a 100644 --- a/app/code/Magento/Customer/Model/Metadata/Form/File.php +++ b/app/code/Magento/Customer/Model/Metadata/Form/File.php @@ -199,11 +199,26 @@ protected function _validateByRules($value) // For UI component uploads, get name from file path if not provided if (empty($value['name']) && !empty($value['file'])) { + // Validate file path for security before using basename + if (!$this->isValidFilePath($value['file'])) { + return [__('"%1" is not a valid file.', $label)]; + } $value['name'] = basename($value['file']); $label = $value['name']; } - $extension = $this->ioFile->getPathInfo($value['name'])['extension']; + // Ensure we have a valid file name before attempting to get extension + if (empty($value['name'])) { + return [__('"%1" is not a valid file.', $label)]; + } + + $pathInfo = $this->ioFile->getPathInfo($value['name']); + $extension = $pathInfo['extension'] ?? ''; + + if (empty($extension)) { + return [__('"%1" is not a valid file.', $label)]; + } + $fileExtensions = ArrayObjectSearch::getArrayElementByName( $rules, 'file_extensions' @@ -224,7 +239,11 @@ protected function _validateByRules($value) } // Determine file path for validation - $filePath = $value['tmp_name'] ?? $value['file'] ?? ''; + $filePath = $value['tmp_name'] ?? $value['file'] ?? null; + if (empty($filePath)) { + return [__('"%1" is not a valid file.', $label)]; + } + if (!$this->_isUploadedFile($filePath)) { return [__('"%1" is not a valid file.', $label)]; } @@ -237,11 +256,7 @@ protected function _validateByRules($value) $size = $value['size'] ?? 0; // For UI component uploads, get file size if not provided if ($size === 0 && !empty($value['file'])) { - $temporaryFile = FileProcessor::TMP_DIR . '/' . basename($value['file']); - if ($this->fileProcessor->isExist($temporaryFile)) { - $stat = $this->fileProcessor->getStat($temporaryFile); - $size = $stat['size'] ?? 0; - } + $size = $this->getTemporaryFileSize($value['file']); } if ($maxFileSize < $size) { return [__('"%1" exceeds the allowed file size.', $label)]; @@ -251,6 +266,58 @@ protected function _validateByRules($value) return []; } + /** + * Validate file path for security + * + * @param string $filePath + * @return bool + */ + private function isValidFilePath(string $filePath): bool + { + // Check for null bytes + if (strpos($filePath, "\0") !== false) { + return false; + } + + // Check for path traversal sequences + if (preg_match('#(^|/)\.\.(?:/|$)#', $filePath)) { + return false; + } + + // Check for Windows absolute paths + if (preg_match('#^[a-zA-Z]:[\\\\/]#', $filePath)) { + return false; + } + + // Check for backslashes at the start + if (isset($filePath[0]) && $filePath[0] === '\\') { + return false; + } + + return true; + } + + /** + * Get file size from temporary directory + * + * @param string $filePath + * @return int + */ + private function getTemporaryFileSize(string $filePath): int + { + if (!$this->isValidFilePath($filePath)) { + return 0; + } + + $temporaryFile = FileProcessor::TMP_DIR . '/' . ltrim(basename($filePath), '/'); + if ($this->fileProcessor->isExist($temporaryFile)) { + $stat = $this->fileProcessor->getStat($temporaryFile); + return (int)($stat['size'] ?? 0); + } + + return 0; + } + /** * Helper function that checks if the file was uploaded. * diff --git a/app/code/Magento/Customer/Model/Metadata/Form/Image.php b/app/code/Magento/Customer/Model/Metadata/Form/Image.php index 9b7c1518b3e4e..a3a277a6a914c 100644 --- a/app/code/Magento/Customer/Model/Metadata/Form/Image.php +++ b/app/code/Magento/Customer/Model/Metadata/Form/Image.php @@ -141,6 +141,10 @@ protected function _validateByRules($value) // For UI component uploads, get name from file path if not provided if (empty($value['name']) && !empty($value['file'])) { + // Validate file path for security before using basename + if (!$this->isValidFilePath($value['file'])) { + return [__('"%1" is not a valid file.', $label)]; + } $value['name'] = basename($value['file']); $label = $value['name']; } @@ -151,7 +155,21 @@ protected function _validateByRules($value) // For UI component uploads, construct the full temporary file path if (empty($filePath) && !empty($value['file'])) { $tmpFileName = ltrim($value['file'], '/'); - $filePath = $this->mediaEntityTmpReadDirectory->getAbsolutePath($tmpFileName); + + // Basic path validation to prevent traversal and suspicious paths + if (!$this->isValidFilePath($tmpFileName)) { + return [__('"%1" is not a valid file.', $label)]; + } + + // Ensure tmpFileName is not empty after ltrim + if ($tmpFileName !== '') { + $filePath = $this->mediaEntityTmpReadDirectory->getAbsolutePath($tmpFileName); + } + } + + // Ensure we have a valid file path before attempting to read image properties + if (empty($filePath) || !is_string($filePath) || !file_exists($filePath)) { + return [__('"%1" is not a valid file.', $label)]; } try { @@ -188,7 +206,10 @@ protected function _validateByRules($value) $size = $value['size'] ?? 0; // For UI component uploads, get file size if not provided if ($size === 0 && !empty($filePath) && file_exists($filePath)) { - $size = filesize($filePath); + $fileSize = filesize($filePath); + if ($fileSize !== false) { + $size = $fileSize; + } } if ($maxFileSize < $size) { $errors[] = __('"%1" exceeds the allowed file size.', $label); @@ -220,6 +241,37 @@ protected function _validateByRules($value) return $errors; } + /** + * Validate file path for security + * + * @param string $filePath + * @return bool + */ + private function isValidFilePath(string $filePath): bool + { + // Check for null bytes + if (strpos($filePath, "\0") !== false) { + return false; + } + + // Check for path traversal sequences + if (preg_match('#(^|/)\.\.(?:/|$)#', $filePath)) { + return false; + } + + // Check for Windows absolute paths + if (preg_match('#^[a-zA-Z]:[\\\\/]#', $filePath)) { + return false; + } + + // Check for backslashes at the start + if (isset($filePath[0]) && $filePath[0] === '\\') { + return false; + } + + return true; + } + /** * Process file uploader UI component data * From 9da1e39c482aa388158f375c9ff36e1d8367e785 Mon Sep 17 00:00:00 2001 From: Bahlai Pavlo Date: Fri, 9 Jan 2026 20:10:50 +0200 Subject: [PATCH 3/5] magento/magento2#39720: Can't upload image for custom customer address attribute --- .../Customer/Model/Metadata/Form/File.php | 13 +- .../Customer/Model/Metadata/Form/Image.php | 170 ++++++++++++++---- .../Unit/Model/Metadata/Form/ImageTest.php | 5 + 3 files changed, 146 insertions(+), 42 deletions(-) diff --git a/app/code/Magento/Customer/Model/Metadata/Form/File.php b/app/code/Magento/Customer/Model/Metadata/Form/File.php index 60b07380aba2a..ecf3fc65e2a61 100644 --- a/app/code/Magento/Customer/Model/Metadata/Form/File.php +++ b/app/code/Magento/Customer/Model/Metadata/Form/File.php @@ -199,11 +199,12 @@ protected function _validateByRules($value) // For UI component uploads, get name from file path if not provided if (empty($value['name']) && !empty($value['file'])) { - // Validate file path for security before using basename + // Validate file path for security before extracting filename if (!$this->isValidFilePath($value['file'])) { return [__('"%1" is not a valid file.', $label)]; } - $value['name'] = basename($value['file']); + $pathInfo = $this->ioFile->getPathInfo($value['file']); + $value['name'] = $pathInfo['basename'] ?? ''; $label = $value['name']; } @@ -309,7 +310,13 @@ private function getTemporaryFileSize(string $filePath): int return 0; } - $temporaryFile = FileProcessor::TMP_DIR . '/' . ltrim(basename($filePath), '/'); + $pathInfo = $this->ioFile->getPathInfo($filePath); + $fileName = $pathInfo['basename'] ?? ''; + if (empty($fileName)) { + return 0; + } + + $temporaryFile = FileProcessor::TMP_DIR . '/' . ltrim($fileName, '/'); if ($this->fileProcessor->isExist($temporaryFile)) { $stat = $this->fileProcessor->getStat($temporaryFile); return (int)($stat['size'] ?? 0); diff --git a/app/code/Magento/Customer/Model/Metadata/Form/Image.php b/app/code/Magento/Customer/Model/Metadata/Form/Image.php index a3a277a6a914c..4330d10891cd4 100644 --- a/app/code/Magento/Customer/Model/Metadata/Form/Image.php +++ b/app/code/Magento/Customer/Model/Metadata/Form/Image.php @@ -133,45 +133,105 @@ public function __construct( * @throws LocalizedException * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ protected function _validateByRules($value) { $label = $value['name'] ?? $value['file'] ?? ''; $rules = $this->getAttribute()->getValidationRules(); - // For UI component uploads, get name from file path if not provided + // Extract and validate file name + $fileName = $this->extractAndValidateFileName($value, $label); + if (is_array($fileName)) { + return $fileName; // Return error array + } + $value['name'] = $fileName; + $label = $fileName; + + // Get and validate file path + $filePath = $this->getValidatedFilePath($value, $label); + if (is_array($filePath)) { + return $filePath; // Return error array + } + + // Validate image properties + $imageProp = $this->validateImageProperties($filePath, $label); + if (is_array($imageProp)) { + return $imageProp; // Return error array + } + + // Validate image format + $formatErrors = $this->validateImageFormat($imageProp, $value, $label); + if (!empty($formatErrors)) { + return $formatErrors; + } + + // Validate size and dimensions + return $this->validateSizeAndDimensions($value, $filePath, $imageProp, $rules, $label); + } + + /** + * Extract and validate file name from value + * + * @param array $value + * @param string $label + * @return string|array Returns file name or error array + */ + private function extractAndValidateFileName(array $value, string $label) + { if (empty($value['name']) && !empty($value['file'])) { - // Validate file path for security before using basename if (!$this->isValidFilePath($value['file'])) { return [__('"%1" is not a valid file.', $label)]; } - $value['name'] = basename($value['file']); - $label = $value['name']; + $pathInfo = $this->ioFileSystem->getPathInfo($value['file']); + return $pathInfo['basename'] ?? ''; } + return $value['name'] ?? ''; + } - // Determine file path for validation + /** + * Get and validate file path + * + * @param array $value + * @param string $label + * @return string|array Returns file path or error array + */ + private function getValidatedFilePath(array $value, string $label) + { $filePath = $value['tmp_name'] ?? null; - // For UI component uploads, construct the full temporary file path if (empty($filePath) && !empty($value['file'])) { $tmpFileName = ltrim($value['file'], '/'); - // Basic path validation to prevent traversal and suspicious paths if (!$this->isValidFilePath($tmpFileName)) { return [__('"%1" is not a valid file.', $label)]; } - // Ensure tmpFileName is not empty after ltrim if ($tmpFileName !== '') { $filePath = $this->mediaEntityTmpReadDirectory->getAbsolutePath($tmpFileName); } } - // Ensure we have a valid file path before attempting to read image properties - if (empty($filePath) || !is_string($filePath) || !file_exists($filePath)) { + if (empty($filePath) || !is_string($filePath)) { + return [__('"%1" is not a valid file.', $label)]; + } + + if (!$this->mediaWriteDirectory->getDriver()->isExists($filePath)) { return [__('"%1" is not a valid file.', $label)]; } + return $filePath; + } + + /** + * Validate image properties using getimagesize + * + * @param string $filePath + * @param string $label + * @return array|false Returns image properties or error array + */ + private function validateImageProperties(string $filePath, string $label) + { try { // phpcs:ignore Magento2.Functions.DiscouragedFunction $imageProp = getimagesize($filePath); @@ -183,13 +243,26 @@ protected function _validateByRules($value) return [__('"%1" is not a valid file.', $label)]; } + return $imageProp; + } + + /** + * Validate image format + * + * @param array $imageProp + * @param array $value + * @param string $label + * @return array Returns error array or empty array + */ + private function validateImageFormat(array $imageProp, array &$value, string $label): array + { $allowImageTypes = [1 => 'gif', 2 => 'jpg', 3 => 'png']; if (!isset($allowImageTypes[$imageProp[2]])) { return [__('"%1" is not a valid image format.', $label)]; } - // modify image name + // Modify image name if extension doesn't match $extension = $this->ioFileSystem->getPathInfo($value['name'])['extension']; if ($extension != $allowImageTypes[$imageProp[2]]) { $value['name'] = $this->ioFileSystem->getPathInfo($value['name'])['filename'] @@ -197,50 +270,69 @@ protected function _validateByRules($value) . $allowImageTypes[$imageProp[2]]; } - $maxFileSize = ArrayObjectSearch::getArrayElementByName( - $rules, - 'max_file_size' - ); + return []; + } + + /** + * Validate file size and image dimensions + * + * @param array $value + * @param string $filePath + * @param array $imageProp + * @param array $rules + * @param string $label + * @return array Returns error array + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + */ + private function validateSizeAndDimensions( + array $value, + string $filePath, + array $imageProp, + array $rules, + string $label + ): array { $errors = []; + + $maxFileSize = ArrayObjectSearch::getArrayElementByName($rules, 'max_file_size'); if ($maxFileSize !== null) { $size = $value['size'] ?? 0; - // For UI component uploads, get file size if not provided - if ($size === 0 && !empty($filePath) && file_exists($filePath)) { - $fileSize = filesize($filePath); - if ($fileSize !== false) { - $size = $fileSize; - } + if ($size === 0 && !empty($filePath)) { + $size = $this->getFileSize($filePath); } if ($maxFileSize < $size) { $errors[] = __('"%1" exceeds the allowed file size.', $label); } } - $maxImageWidth = ArrayObjectSearch::getArrayElementByName( - $rules, - 'max_image_width' - ); - if ($maxImageWidth !== null) { - if ($maxImageWidth < $imageProp[0]) { - $r = $maxImageWidth; - $errors[] = __('"%1" width exceeds allowed value of %2 px.', $label, $r); - } + $maxImageWidth = ArrayObjectSearch::getArrayElementByName($rules, 'max_image_width'); + if ($maxImageWidth !== null && $maxImageWidth < $imageProp[0]) { + $errors[] = __('"%1" width exceeds allowed value of %2 px.', $label, $maxImageWidth); } - $maxImageHeight = ArrayObjectSearch::getArrayElementByName( - $rules, - 'max_image_height' - ); - if ($maxImageHeight !== null) { - if ($maxImageHeight < $imageProp[1]) { - $r = $maxImageHeight; - $errors[] = __('"%1" height exceeds allowed value of %2 px.', $label, $r); - } + $maxImageHeight = ArrayObjectSearch::getArrayElementByName($rules, 'max_image_height'); + if ($maxImageHeight !== null && $maxImageHeight < $imageProp[1]) { + $errors[] = __('"%1" height exceeds allowed value of %2 px.', $label, $maxImageHeight); } return $errors; } + /** + * Get file size safely + * + * @param string $filePath + * @return int + */ + private function getFileSize(string $filePath): int + { + try { + $stat = $this->mediaWriteDirectory->getDriver()->stat($filePath); + return (int)($stat['size'] ?? 0); + } catch (\Exception $e) { + return 0; + } + } + /** * Validate file path for security * diff --git a/app/code/Magento/Customer/Test/Unit/Model/Metadata/Form/ImageTest.php b/app/code/Magento/Customer/Test/Unit/Model/Metadata/Form/ImageTest.php index fd13c7da02fbb..4f1ebfa62b46f 100644 --- a/app/code/Magento/Customer/Test/Unit/Model/Metadata/Form/ImageTest.php +++ b/app/code/Magento/Customer/Test/Unit/Model/Metadata/Form/ImageTest.php @@ -193,6 +193,11 @@ public function testValidateIsNotValidFile() ->method('getStoreLabel') ->willReturn('File Input Field Label'); + $this->driverMock->expects($this->once()) + ->method('isExists') + ->with($value['tmp_name']) + ->willReturn(true); + $this->fileProcessorMock->expects($this->once()) ->method('isExist') ->with(FileProcessor::TMP_DIR . '/' . $value['tmp_name']) From d4d36679eb22eae0283e3e8b5625b32378e35a8e Mon Sep 17 00:00:00 2001 From: Bahlai Pavlo Date: Sat, 10 Jan 2026 15:02:56 +0200 Subject: [PATCH 4/5] magento/magento2#39720: Can't upload image for custom customer address attribute --- .../Customer/Model/Metadata/Form/File.php | 119 ++++++++++++++---- .../Customer/Model/Metadata/Form/Image.php | 40 ++++-- .../Unit/Model/Metadata/Form/ImageTest.php | 4 - 3 files changed, 122 insertions(+), 41 deletions(-) diff --git a/app/code/Magento/Customer/Model/Metadata/Form/File.php b/app/code/Magento/Customer/Model/Metadata/Form/File.php index ecf3fc65e2a61..c0e9c5ae5cd75 100644 --- a/app/code/Magento/Customer/Model/Metadata/Form/File.php +++ b/app/code/Magento/Customer/Model/Metadata/Form/File.php @@ -197,6 +197,39 @@ protected function _validateByRules($value) $label = $value['name'] ?? $value['file'] ?? ''; $rules = $this->getAttribute()->getValidationRules(); + // Extract and validate file name + $fileNameResult = $this->extractAndValidateFileName($value, $label); + if (is_array($fileNameResult)) { + return $fileNameResult; // Return error array + } + $value['name'] = $fileNameResult['name']; + $label = $fileNameResult['label']; + + // Validate file extension + $extensionResult = $this->validateFileExtension($value['name'], $rules, $label); + if (!empty($extensionResult)) { + return $extensionResult; + } + + // Validate file path + $filePath = $this->getFilePath($value, $label); + if (is_array($filePath)) { + return $filePath; // Return error array + } + + // Validate file size + return $this->validateFileSize($value, $rules, $label); + } + + /** + * Extract and validate file name from value + * + * @param array $value + * @param string $label + * @return array Returns array with name and label or error array + */ + private function extractAndValidateFileName(array $value, string $label): array + { // For UI component uploads, get name from file path if not provided if (empty($value['name']) && !empty($value['file'])) { // Validate file path for security before extracting filename @@ -204,26 +237,38 @@ protected function _validateByRules($value) return [__('"%1" is not a valid file.', $label)]; } $pathInfo = $this->ioFile->getPathInfo($value['file']); - $value['name'] = $pathInfo['basename'] ?? ''; - $label = $value['name']; + $name = $pathInfo['basename'] ?? ''; + $label = $name; + } else { + $name = $value['name'] ?? ''; } - // Ensure we have a valid file name before attempting to get extension - if (empty($value['name'])) { + // Ensure we have a valid file name + if (empty($name)) { return [__('"%1" is not a valid file.', $label)]; } - $pathInfo = $this->ioFile->getPathInfo($value['name']); + return ['name' => $name, 'label' => $label]; + } + + /** + * Validate file extension + * + * @param string $fileName + * @param array $rules + * @param string $label + * @return array Returns error array or empty array + */ + private function validateFileExtension(string $fileName, array $rules, string $label): array + { + $pathInfo = $this->ioFile->getPathInfo($fileName); $extension = $pathInfo['extension'] ?? ''; if (empty($extension)) { return [__('"%1" is not a valid file.', $label)]; } - $fileExtensions = ArrayObjectSearch::getArrayElementByName( - $rules, - 'file_extensions' - ); + $fileExtensions = ArrayObjectSearch::getArrayElementByName($rules, 'file_extensions'); if ($fileExtensions !== null) { $extensions = explode(',', $fileExtensions); $extensions = array_map('trim', $extensions); @@ -232,14 +277,23 @@ protected function _validateByRules($value) } } - /** - * Check protected file extension - */ + // Check protected file extension if (!$this->_fileValidator->isValid($extension)) { return $this->_fileValidator->getMessages(); } - // Determine file path for validation + return []; + } + + /** + * Get and validate file path + * + * @param array $value + * @param string $label + * @return string|array Returns file path or error array + */ + private function getFilePath(array $value, string $label) + { $filePath = $value['tmp_name'] ?? $value['file'] ?? null; if (empty($filePath)) { return [__('"%1" is not a valid file.', $label)]; @@ -249,19 +303,32 @@ protected function _validateByRules($value) return [__('"%1" is not a valid file.', $label)]; } - $maxFileSize = ArrayObjectSearch::getArrayElementByName( - $rules, - 'max_file_size' - ); - if ($maxFileSize !== null) { - $size = $value['size'] ?? 0; - // For UI component uploads, get file size if not provided - if ($size === 0 && !empty($value['file'])) { - $size = $this->getTemporaryFileSize($value['file']); - } - if ($maxFileSize < $size) { - return [__('"%1" exceeds the allowed file size.', $label)]; - } + return $filePath; + } + + /** + * Validate file size + * + * @param array $value + * @param array $rules + * @param string $label + * @return array Returns error array or empty array + */ + private function validateFileSize(array $value, array $rules, string $label): array + { + $maxFileSize = ArrayObjectSearch::getArrayElementByName($rules, 'max_file_size'); + if ($maxFileSize === null) { + return []; + } + + $size = $value['size'] ?? 0; + // For UI component uploads, get file size if not provided + if ($size === 0 && !empty($value['file'])) { + $size = $this->getTemporaryFileSize($value['file']); + } + + if ($maxFileSize < $size) { + return [__('"%1" exceeds the allowed file size.', $label)]; } return []; diff --git a/app/code/Magento/Customer/Model/Metadata/Form/Image.php b/app/code/Magento/Customer/Model/Metadata/Form/Image.php index 4330d10891cd4..6fded0bc2ebc3 100644 --- a/app/code/Magento/Customer/Model/Metadata/Form/Image.php +++ b/app/code/Magento/Customer/Model/Metadata/Form/Image.php @@ -133,7 +133,6 @@ public function __construct( * @throws LocalizedException * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) - * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ protected function _validateByRules($value) { @@ -143,22 +142,19 @@ protected function _validateByRules($value) // Extract and validate file name $fileName = $this->extractAndValidateFileName($value, $label); if (is_array($fileName)) { - return $fileName; // Return error array + return $fileName; } $value['name'] = $fileName; $label = $fileName; - // Get and validate file path - $filePath = $this->getValidatedFilePath($value, $label); - if (is_array($filePath)) { - return $filePath; // Return error array + // Get and validate file path, then validate image properties + $validationResult = $this->validateFilePathAndProperties($value, $label); + if (is_array($validationResult) && isset($validationResult['error'])) { + return $validationResult['error']; } - // Validate image properties - $imageProp = $this->validateImageProperties($filePath, $label); - if (is_array($imageProp)) { - return $imageProp; // Return error array - } + $filePath = $validationResult['filePath']; + $imageProp = $validationResult['imageProp']; // Validate image format $formatErrors = $this->validateImageFormat($imageProp, $value, $label); @@ -170,6 +166,28 @@ protected function _validateByRules($value) return $this->validateSizeAndDimensions($value, $filePath, $imageProp, $rules, $label); } + /** + * Validate file path and image properties + * + * @param array $value + * @param string $label + * @return array Returns array with filePath and imageProp or error array + */ + private function validateFilePathAndProperties(array $value, string $label): array + { + $filePath = $this->getValidatedFilePath($value, $label); + if (is_array($filePath)) { + return ['error' => $filePath]; + } + + $imageProp = $this->validateImageProperties($filePath, $label); + if (is_array($imageProp) && !isset($imageProp[0])) { + return ['error' => $imageProp]; + } + + return ['filePath' => $filePath, 'imageProp' => $imageProp]; + } + /** * Extract and validate file name from value * diff --git a/app/code/Magento/Customer/Test/Unit/Model/Metadata/Form/ImageTest.php b/app/code/Magento/Customer/Test/Unit/Model/Metadata/Form/ImageTest.php index 4f1ebfa62b46f..24340775369e1 100644 --- a/app/code/Magento/Customer/Test/Unit/Model/Metadata/Form/ImageTest.php +++ b/app/code/Magento/Customer/Test/Unit/Model/Metadata/Form/ImageTest.php @@ -198,10 +198,6 @@ public function testValidateIsNotValidFile() ->with($value['tmp_name']) ->willReturn(true); - $this->fileProcessorMock->expects($this->once()) - ->method('isExist') - ->with(FileProcessor::TMP_DIR . '/' . $value['tmp_name']) - ->willReturn(true); $this->ioFileSystemMock->expects($this->once()) ->method('getPathInfo') From 500ceaa460da0d6eff6ce904eae9c4e068986c57 Mon Sep 17 00:00:00 2001 From: Bahlai Pavlo Date: Mon, 12 Jan 2026 12:16:11 +0200 Subject: [PATCH 5/5] magento/magento2#39720: Can't upload image for custom customer address attribute --- .../Customer/Model/Metadata/Form/File.php | 4 +- .../Customer/Model/Metadata/Form/Image.php | 2 +- .../Unit/Model/Metadata/Form/ImageTest.php | 63 ++++++++++++++++--- 3 files changed, 56 insertions(+), 13 deletions(-) diff --git a/app/code/Magento/Customer/Model/Metadata/Form/File.php b/app/code/Magento/Customer/Model/Metadata/Form/File.php index c0e9c5ae5cd75..d2abf74eab4f3 100644 --- a/app/code/Magento/Customer/Model/Metadata/Form/File.php +++ b/app/code/Magento/Customer/Model/Metadata/Form/File.php @@ -195,11 +195,11 @@ public function extractValue(\Magento\Framework\App\RequestInterface $request) protected function _validateByRules($value) { $label = $value['name'] ?? $value['file'] ?? ''; - $rules = $this->getAttribute()->getValidationRules(); + $rules = $this->getAttribute()->getValidationRules() ?? []; // Extract and validate file name $fileNameResult = $this->extractAndValidateFileName($value, $label); - if (is_array($fileNameResult)) { + if (!isset($fileNameResult['name'])) { return $fileNameResult; // Return error array } $value['name'] = $fileNameResult['name']; diff --git a/app/code/Magento/Customer/Model/Metadata/Form/Image.php b/app/code/Magento/Customer/Model/Metadata/Form/Image.php index 6fded0bc2ebc3..25c39d3ff5d4d 100644 --- a/app/code/Magento/Customer/Model/Metadata/Form/Image.php +++ b/app/code/Magento/Customer/Model/Metadata/Form/Image.php @@ -276,7 +276,7 @@ private function validateImageFormat(array $imageProp, array &$value, string $la { $allowImageTypes = [1 => 'gif', 2 => 'jpg', 3 => 'png']; - if (!isset($allowImageTypes[$imageProp[2]])) { + if (!isset($imageProp[2]) || !isset($allowImageTypes[$imageProp[2]])) { return [__('"%1" is not a valid image format.', $label)]; } diff --git a/app/code/Magento/Customer/Test/Unit/Model/Metadata/Form/ImageTest.php b/app/code/Magento/Customer/Test/Unit/Model/Metadata/Form/ImageTest.php index 24340775369e1..657774e977d16 100644 --- a/app/code/Magento/Customer/Test/Unit/Model/Metadata/Form/ImageTest.php +++ b/app/code/Magento/Customer/Test/Unit/Model/Metadata/Form/ImageTest.php @@ -198,18 +198,20 @@ public function testValidateIsNotValidFile() ->with($value['tmp_name']) ->willReturn(true); - $this->ioFileSystemMock->expects($this->once()) ->method('getPathInfo') ->willReturn($value); $model = $this->initialize([ - 'value' => $value, + 'value' => null, 'isAjax' => false, 'entityTypeCode' => CustomerMetadataInterface::ENTITY_TYPE_CUSTOMER, ]); - $this->assertEquals(['"realFileName" is not a valid file.'], $model->validateValue($value)); + $result = $model->validateValue($value); + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertEquals('"realFileName" is not a valid image format.', (string)$result[0]); } /** @@ -233,9 +235,19 @@ public function testValidate() ->method('getStoreLabel') ->willReturn('File Input Field Label'); + $this->attributeMetadataMock->expects($this->once()) + ->method('getValidationRules') + ->willReturn([]); + + $this->driverMock->expects($this->once()) + ->method('isExists') + ->with($value['tmp_name']) + ->willReturn(true); + + // Mock fileProcessor->isExist to make _isUploadedFile return true $this->fileProcessorMock->expects($this->once()) ->method('isExist') - ->with(FileProcessor::TMP_DIR . '/' . $value['name']) + ->with(FileProcessor::TMP_DIR . '/' . $value['basename']) ->willReturn(true); $model = $this->initialize([ @@ -278,9 +290,15 @@ public function testValidateMaxFileSize() ->method('getValidationRules') ->willReturn([$validationRuleMock]); + $this->driverMock->expects($this->once()) + ->method('isExists') + ->with($value['tmp_name']) + ->willReturn(true); + + // Mock fileProcessor->isExist to make _isUploadedFile return true $this->fileProcessorMock->expects($this->once()) ->method('isExist') - ->with(FileProcessor::TMP_DIR . '/' . $value['name']) + ->with(FileProcessor::TMP_DIR . '/' . $value['basename']) ->willReturn(true); $this->ioFileSystemMock->expects($this->any()) @@ -293,7 +311,10 @@ public function testValidateMaxFileSize() 'entityTypeCode' => CustomerMetadataInterface::ENTITY_TYPE_CUSTOMER, ]); - $this->assertEquals(['"logo.gif" exceeds the allowed file size.'], $model->validateValue($value)); + $result = $model->validateValue($value); + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertEquals('"logo.gif" exceeds the allowed file size.', (string)$result[0]); } /** @@ -330,18 +351,31 @@ public function testValidateMaxImageWidth() ->method('getValidationRules') ->willReturn([$validationRuleMock]); + $this->driverMock->expects($this->once()) + ->method('isExists') + ->with($value['tmp_name']) + ->willReturn(true); + + // Mock fileProcessor->isExist to make _isUploadedFile return true $this->fileProcessorMock->expects($this->once()) ->method('isExist') - ->with(FileProcessor::TMP_DIR . '/' . $value['name']) + ->with(FileProcessor::TMP_DIR . '/' . $value['basename']) ->willReturn(true); + $this->ioFileSystemMock->expects($this->any()) + ->method('getPathInfo') + ->willReturn($value); + $model = $this->initialize([ 'value' => $value, 'isAjax' => false, 'entityTypeCode' => CustomerMetadataInterface::ENTITY_TYPE_CUSTOMER, ]); - $this->assertEquals(['"logo.gif" width exceeds allowed value of 1 px.'], $model->validateValue($value)); + $result = $model->validateValue($value); + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertEquals('"logo.gif" width exceeds allowed value of 1 px.', (string)$result[0]); } /** @@ -374,9 +408,15 @@ public function testValidateMaxImageHeight() ->method('getValidationRules') ->willReturn([$validationRuleMock]); + $this->driverMock->expects($this->once()) + ->method('isExists') + ->with($value['tmp_name']) + ->willReturn(true); + + // Mock fileProcessor->isExist to make _isUploadedFile return true $this->fileProcessorMock->expects($this->once()) ->method('isExist') - ->with(FileProcessor::TMP_DIR . '/' . $value['name']) + ->with(FileProcessor::TMP_DIR . '/' . $value['basename']) ->willReturn(true); $this->ioFileSystemMock->expects($this->any()) @@ -389,7 +429,10 @@ public function testValidateMaxImageHeight() 'entityTypeCode' => CustomerMetadataInterface::ENTITY_TYPE_CUSTOMER, ]); - $this->assertEquals(['"logo.gif" height exceeds allowed value of 1 px.'], $model->validateValue($value)); + $result = $model->validateValue($value); + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertEquals('"logo.gif" height exceeds allowed value of 1 px.', (string)$result[0]); } /**