diff --git a/src/Forms/FileField.php b/src/Forms/FileField.php index d56585ee1..6a6b54b49 100644 --- a/src/Forms/FileField.php +++ b/src/Forms/FileField.php @@ -177,13 +177,46 @@ class FileField extends FormField implements FileHandleField public function validate($validator) { - if (!isset($_FILES[$this->name])) { + // FileField with the name multi_file_syntax[] or multi_file_syntax[key] will have the brackets trimmed in + // $_FILES super-global so it will be stored as $_FILES['mutli_file_syntax'] + // multi-file uploads, which are not officially supported by Silverstripe, though may be + // implemented in custom code, so we should still ensure they are at least validated + $isMultiFileUpload = strpos($this->name, '[') !== false; + $fieldName = preg_replace('#\[(.*?)\]$#', '', $this->name); + + if (!isset($_FILES[$fieldName])) { return true; } - $tmpFile = $_FILES[$this->name]; + if ($isMultiFileUpload) { + $isValid = true; + foreach (array_keys($_FILES[$fieldName]['name']) as $key) { + $fileData = [ + 'name' => $_FILES[$fieldName]['name'][$key], + 'type' => $_FILES[$fieldName]['type'][$key], + 'tmp_name' => $_FILES[$fieldName]['tmp_name'][$key], + 'error' => $_FILES[$fieldName]['error'][$key], + 'size' => $_FILES[$fieldName]['size'][$key], + ]; + if (!$this->validateFileData($validator, $fileData)) { + $isValid = false; + } + } + return $isValid; + } - $valid = $this->upload->validate($tmpFile); + // regular single-file upload + return $this->validateFileData($validator, $_FILES[$this->name]); + } + + /** + * @param Validator $validator + * @param array $fileData + * @return bool + */ + private function validateFileData($validator, array $fileData): bool + { + $valid = $this->upload->validate($fileData); if (!$valid) { $errors = $this->upload->getErrors(); if ($errors) { @@ -193,7 +226,6 @@ class FileField extends FormField implements FileHandleField } return false; } - return true; } diff --git a/tests/php/Forms/FileFieldTest.php b/tests/php/Forms/FileFieldTest.php index 577d279e8..19779d986 100644 --- a/tests/php/Forms/FileFieldTest.php +++ b/tests/php/Forms/FileFieldTest.php @@ -3,6 +3,7 @@ namespace SilverStripe\Forms\Tests; use ReflectionMethod; +use SilverStripe\Assets\Upload_Validator; use SilverStripe\Dev\FunctionalTest; use SilverStripe\Control\Controller; use SilverStripe\Forms\FileField; @@ -39,6 +40,67 @@ class FileFieldTest extends FunctionalTest $this->assertTrue($form->validationResult()->isValid()); } + /** + * Test that FileField::validate() is run on FileFields with both single and multi-file syntax + * By default FileField::validate() will return true early if the $_FILES super-global does not contain the + * corresponding FileField::name. This early return means the files was not fully run through FileField::validate() + * So for this test we create an invalid file upload on purpose and test that false was returned which means that + * the file was run through FileField::validate() function + */ + public function testMultiFileSyntaxUploadIsValidated() + { + $names = [ + 'single_file_syntax', + 'multi_file_syntax_a[]', + 'multi_file_syntax_b[0]', + 'multi_file_syntax_c[key]' + ]; + foreach ($names as $name) { + $form = new Form( + Controller::curr(), + 'Form', + new FieldList($fileField = new FileField($name, 'My desc')), + new FieldList() + ); + $fileData = $this->createInvalidUploadedFileData($name, "FileFieldTest.txt"); + // FileFields with multi_file_syntax[] files will appear in the $_FILES super-global + // with the [] brackets trimmed e.g. $_FILES[multi_file_syntax] + $_FILES = [preg_replace('#\[(.*?)\]#', '', $name) => $fileData]; + $fileField->setValue($fileData); + $validator = $form->getValidator(); + $isValid = $fileField->validate($validator); + $this->assertFalse($isValid, "$name was run through the validate() function"); + } + } + + protected function createInvalidUploadedFileData($name, $tmpFileName): array + { + $tmpFilePath = TEMP_PATH . DIRECTORY_SEPARATOR . $tmpFileName; + + // multi_file_syntax + if (strpos($name, '[') !== false) { + $key = 0; + if (preg_match('#\[(.+?)\]#', $name, $m)) { + $key = $m[1]; + } + return [ + 'name' => [$key => $tmpFileName], + 'type' => [$key => 'text/plaintext'], + 'size' => [$key => 0], + 'tmp_name' => [$key => $tmpFilePath], + 'error' => [$key => UPLOAD_ERR_NO_FILE], + ]; + } + // single_file_syntax + return [ + 'name' => $tmpFileName, + 'type' => 'text/plaintext', + 'size' => 0, + 'tmp_name' => $tmpFilePath, + 'error' => UPLOAD_ERR_NO_FILE, + ]; + } + /** * @skipUpgrade */