[TASK] EXT:form - Optimize file upload/ handling of files 36/43836/4
authorRalf Zimmermann <ralf.zimmermann@tritum.de>
Tue, 6 Oct 2015 08:34:01 +0000 (10:34 +0200)
committerWouter Wolters <typo3@wouterwolters.nl>
Thu, 8 Oct 2015 20:37:07 +0000 (22:37 +0200)
Determine the file mime type with \TYPO3\CMS\Core\Type\File\FileInfo
before a validation or other operations with the files are made.

Resolves: #69956
Releases: master
Change-Id: Iac0381b9847b82dfa7bc7a78f970c91ce51d4272
Reviewed-on: http://review.typo3.org/43836
Tested-by: Bjoern Jacob <bjoern.jacob@tritum.de>
Reviewed-by: Bjoern Jacob <bjoern.jacob@tritum.de>
Reviewed-by: Wouter Wolters <typo3@wouterwolters.nl>
Tested-by: Wouter Wolters <typo3@wouterwolters.nl>
12 files changed:
typo3/sysext/form/Classes/Domain/Property/TypeConverter/ArrayToValidationElementConverter.php
typo3/sysext/form/Classes/Domain/Validator/FileAllowedTypesValidator.php
typo3/sysext/form/Classes/Domain/Validator/ValidationElementValidator.php
typo3/sysext/form/Classes/Hooks/HandleIncomingFormValues.php
typo3/sysext/form/Classes/PostProcess/MailPostProcessor.php
typo3/sysext/form/Classes/Utility/SessionUtility.php
typo3/sysext/form/Resources/Private/Partials/Compatibility/Confirmation/FlatElements/Upload.html
typo3/sysext/form/Resources/Private/Partials/Compatibility/PostProcessor/Mail/Html/FlatElements/Upload.html
typo3/sysext/form/Resources/Private/Partials/Compatibility/PostProcessor/Mail/Plain/FlatElements/Upload.html
typo3/sysext/form/Resources/Private/Partials/Default/Confirmation/FlatElements/Upload.html
typo3/sysext/form/Resources/Private/Partials/Default/PostProcessor/Mail/Html/FlatElements/Upload.html
typo3/sysext/form/Resources/Private/Partials/Default/PostProcessor/Mail/Plain/FlatElements/Upload.html

index 09d8974..f4b55ce 100644 (file)
@@ -63,8 +63,57 @@ class ArrayToValidationElementConverter extends AbstractTypeConverter {
        public function convertFrom($source, $targetType, array $convertedChildProperties = array(), PropertyMappingConfigurationInterface $configuration = NULL) {
                /** @var ValidationElement $validationElement */
                $validationElement = GeneralUtility::makeInstance(ValidationElement::class);
-
                if (is_array($source)) {
+                       /**
+                        * Find uploaded files.
+                        *
+                        * Extbase has already mapped the $_FILES data into the request
+                        * @see TYPO3\CMS\Extbase\Mvc\Web\Request::build()
+                        * If a $_FILES array is found in the request data ($source),
+                        * set the file mime type with
+                        * \TYPO3\CMS\Core\Type\File\FileInfo
+                        * and write the data back into $source.
+                        */
+                       foreach ($source as $propertyName => $value) {
+                               if (is_array($value)) {
+                                       $uploadedFiles = array();
+                                       if (
+                                               isset($value['name'])
+                                               && isset($value['type'])
+                                               && isset($value['tmp_name'])
+                                               && isset($value['size'])
+                                       ) {
+                                               // if single file upload - cast to array
+                                               $uploadedFiles[] = $value;
+                                       } elseif (
+                                               isset($value[0]['name'])
+                                               && isset($value[0]['type'])
+                                               && isset($value[0]['tmp_name'])
+                                               && isset($value[0]['size'])
+                                       ) {
+                                               // multi file upload
+                                               $uploadedFiles = $value;
+                                       }
+
+                                       if (!empty($uploadedFiles)) {
+                                               foreach ($uploadedFiles as $key => &$file) {
+                                                       if (
+                                                               $file['name'] === ''
+                                                               && $file['type'] === ''
+                                                               && $file['tmp_name'] === ''
+                                                               && $file['size'] === 0
+                                                       ) {
+                                                               unset($uploadedFiles[$key]);
+                                                               continue;
+                                                       }
+                                                       $fileInfo = GeneralUtility::makeInstance(\TYPO3\CMS\Core\Type\File\FileInfo::class, $file['tmp_name']);
+                                                       $file['type'] = $fileInfo->getMimeType();
+                                                       $file['name'] = htmlspecialchars($file['name']);
+                                               }
+                                               $source[$propertyName] = $uploadedFiles;
+                                       }
+                               }
+                       }
                        $validationElement->setIncomingFields($source);
                }
 
index 039a2c8..c627fe1 100755 (executable)
@@ -35,48 +35,19 @@ class FileAllowedTypesValidator extends AbstractValidator {
        const LOCALISATION_OBJECT_NAME = 'tx_form_system_validate_fileallowedtypes';
 
        /**
-        * Check if $value is valid. If it is not valid, needs to add an error
-        * to result.
+        * Check if the file mime type is allowed.
+        *
+        * The mime type is set in the propertymapper
+        * @see TYPO3\CMS\Form\Domain\Property\TypeConverter::convertFrom
         *
         * @param mixed $value
         * @return void
         */
        public function isValid($value) {
-               // @todo $value is never used, what's the process flow here?
-
                $allowedTypes = strtolower($this->options['types']);
-               $this->options['types'] = GeneralUtility::trimExplode(', ', $allowedTypes);
-
-               if (isset($this->rawArgument[$this->options['element']]['name'])) {
-                       $request = $this->rawArgument[$this->options['element']];
-                       $this->checkFileType($request);
-               } else {
-                               // multi upload
-                       foreach ($this->rawArgument[$this->options['element']] as $file) {
-                               if (
-                                       $file['name'] === ''
-                                       && $file['type'] === ''
-                                       && $file['tmp_name'] === ''
-                                       && $file['size'] === 0
-                               ) {
-                                       continue;
-                               }
-                               $this->checkFileType($file);
-                       }
-               }
-       }
-
-       /**
-        * Check if $value is valid. If it is not valid, needs to add an error
-        * to result.
-        *
-        * @param array $request
-        * @return void
-        */
-       public function checkFileType($request) {
-               // @todo Using $_FILES[...]['type] is probably insecure, since it's submitted by the client directly
-               $value = strtolower($request['type']);
-               if (!in_array($value, $this->options['types'])) {
+               $allowedMimeTypes = GeneralUtility::trimExplode(', ', $allowedTypes);
+               $fileMimeType = strtolower($value['type']);
+               if (!in_array($fileMimeType, $allowedMimeTypes, TRUE)) {
                        $this->addError(
                                $this->renderMessage(
                                        $this->options['errorMessage'][0],
index e363d49..4ecb95b 100644 (file)
@@ -28,6 +28,18 @@ class ValidationElementValidator extends \TYPO3\CMS\Extbase\Validation\Validator
        protected $propertyValidators = array();
 
        /**
+        * @var \TYPO3\CMS\Form\Utility\SessionUtility
+        */
+       protected $sessionUtility;
+
+       /**
+        * @param \TYPO3\CMS\Form\Utility\SessionUtility $sessionUtility
+        */
+       public function injectSessionUtility(\TYPO3\CMS\Form\Utility\SessionUtility $sessionUtility) {
+               $this->sessionUtility = $sessionUtility;
+       }
+
+       /**
         * Checks if the given value is valid according to the validator, and returns
         * the Error Messages object which occurred.
         *
@@ -58,7 +70,19 @@ class ValidationElementValidator extends \TYPO3\CMS\Extbase\Validation\Validator
         * @return mixed
         */
        protected function getPropertyValue(\TYPO3\CMS\Form\Domain\Model\ValidationElement $validationElement, $propertyName) {
-               return $validationElement->getIncomingField($propertyName);
+               /**
+                * If a confirmation page is set and a fileupload was done before
+                * there is no incoming data if the process action is called.
+                * The data is only in the session at this time.
+                * This results in a negative validation (if a validation is set).
+                * Therefore, look first in the session.
+                */
+               if ($this->sessionUtility->getSessionData($propertyName)) {
+                       $propertyValue = $this->sessionUtility->getSessionData($propertyName);
+               } else {
+                       $propertyValue = $validationElement->getIncomingField($propertyName);
+               }
+               return $propertyValue;
        }
 
        /**
@@ -77,12 +101,38 @@ class ValidationElementValidator extends \TYPO3\CMS\Extbase\Validation\Validator
                        if ($validator instanceof ObjectValidatorInterface) {
                                $validator->setValidatedInstancesContainer($this->validatedInstancesContainer);
                        }
-                       $currentResult = $validator->validate($value);
-                       if ($currentResult->hasMessages()) {
-                               if ($result == NULL) {
-                                       $result = $currentResult;
-                               } else {
-                                       $result->merge($currentResult);
+
+                       /**
+                        * File upload validation.
+                        *
+                        * If a $_FILES array is found in the request data,
+                        * iterate over all requested files and validate each
+                        * single file.
+                        */
+                       if (
+                               isset($value[0]['name'])
+                               && isset($value[0]['type'])
+                               && isset($value[0]['tmp_name'])
+                               && isset($value[0]['size'])
+                       ) {
+                               foreach ($value as $file) {
+                                       $currentResult = $validator->validate($file);
+                                       if ($currentResult->hasMessages()) {
+                                               if ($result == NULL) {
+                                                       $result = $currentResult;
+                                               } else {
+                                                       $result->merge($currentResult);
+                                               }
+                                       }
+                               }
+                       } else {
+                               $currentResult = $validator->validate($value);
+                               if ($currentResult->hasMessages()) {
+                                       if ($result == NULL) {
+                                               $result = $currentResult;
+                                       } else {
+                                               $result->merge($currentResult);
+                                       }
                                }
                        }
                }
index 06a867f..4a8fc75 100644 (file)
@@ -145,25 +145,19 @@ class HandleIncomingFormValues implements SingletonInterface {
                                        && $formBuilder->getValidationErrors()->forProperty($elementName)->hasErrors() !== TRUE
                                )
                        ) {
-                               $formPrefix = $formBuilder->getFormPrefix();
-                               if (
-                                       isset($_FILES['tx_form_form']['tmp_name'][$formPrefix])
-                                       && is_array($_FILES['tx_form_form']['tmp_name'][$formPrefix])
-                               ) {
-                                       foreach ($_FILES['tx_form_form']['tmp_name'][$formPrefix] as $fieldName => $uploadedFile) {
-                                               $uploadedFiles = array();
-                                               if (is_string($uploadedFile)) {
-                                                       $uploadedFiles[] = $this->saveUploadedFile($formPrefix, $fieldName, -1, $uploadedFile);
-                                               } else {
-                                                               // multi upload
-                                                       foreach ($uploadedFile as $key => $file) {
-                                                               $uploadedFiles[] = $this->saveUploadedFile($formPrefix, $fieldName, $key, $file);
-                                                       }
+                               $uploadedFiles = $formBuilder->getIncomingData()->getIncomingField($elementName);
+                               if (is_array($uploadedFiles)) {
+                                       foreach ($uploadedFiles as $key => &$file) {
+                                               $tempFilename = $this->saveUploadedFile($file['tmp_name']);
+                                               if (!$tempFilename) {
+                                                       unset($uploadedFiles[$key]);
+                                                       continue;
                                                }
-                                               $element->setAdditionalArgument('uploadedFiles', $uploadedFiles);
-                                               $this->setAttribute($element, 'value', '');
-                                               $this->sessionUtility->setSessionData($fieldName, $uploadedFiles);
+                                               $file['tempFilename'] = $tempFilename;
                                        }
+                                       $element->setAdditionalArgument('uploadedFiles', $uploadedFiles);
+                                       $this->setAttribute($element, 'value', '');
+                                       $this->sessionUtility->setSessionData($elementName, $uploadedFiles);
                                }
                        }
                }
@@ -172,33 +166,17 @@ class HandleIncomingFormValues implements SingletonInterface {
        /**
         * Save a uploaded file
         *
-        * @param string $formPrefix
-        * @param string $fieldName
-        * @param integer $key
         * @param string $uploadedFile
-        * @return NULL|array
+        * @return NULL|string
         */
-       public function saveUploadedFile($formPrefix, $fieldName, $key, $uploadedFile) {
+       public function saveUploadedFile($uploadedFile) {
                if (is_uploaded_file($uploadedFile)) {
                        $tempFilename = GeneralUtility::upload_to_tempfile($uploadedFile);
                        if (TYPO3_OS === 'WIN') {
                                $tempFilename = GeneralUtility::fixWindowsFilePath($tempFilename);
                        }
                        if ($tempFilename !== '') {
-                               if ($key == -1) {
-                                       $originalFilename = $_FILES['tx_form_form']['name'][$formPrefix][$fieldName];
-                                       $size = $_FILES['tx_form_form']['size'][$formPrefix][$fieldName];
-                               } else {
-                                       $originalFilename = $_FILES['tx_form_form']['name'][$formPrefix][$fieldName][$key];
-                                       $size = $_FILES['tx_form_form']['size'][$formPrefix][$fieldName][$key];
-                               }
-                               $fileInfo = GeneralUtility::makeInstance(\TYPO3\CMS\Core\Type\File\FileInfo::class, $tempFilename);
-                               return array(
-                                               'tempFilename' => $tempFilename,
-                                               'originalFilename' => $originalFilename,
-                                               'type' => $fileInfo->getMimeType(),
-                                               'size' => (int)$size
-                                       );
+                               return $tempFilename;
                        }
                }
                return NULL;
index c704b4a..00a2950 100644 (file)
@@ -425,7 +425,7 @@ class MailPostProcessor extends AbstractPostProcessor implements PostProcessorIn
                                                                is_file($file['tempFilename'])
                                                                && GeneralUtility::isAllowedAbsPath($file['tempFilename'])
                                                        ) {
-                                                               $this->mailMessage->attach(\Swift_Attachment::fromPath($file['tempFilename'])->setFilename($file['originalFilename']));
+                                                               $this->mailMessage->attach(\Swift_Attachment::fromPath($file['tempFilename'])->setFilename($file['name']));
                                                        }
                                                }
                                        }
index e22dd96..82ea377 100644 (file)
@@ -141,7 +141,6 @@ class SessionUtility implements SingletonInterface {
                        foreach ($sessionData as $fieldName => $values) {
                                if (is_array($values)) {
                                        foreach ($values as $file) {
-
                                                if (isset($file['tempFilename'])) {
                                                        GeneralUtility::unlink_tempfile($file['tempFilename']);
                                                }
index 1afa3ca..9837361 100644 (file)
@@ -1,7 +1,7 @@
 <f:if condition="{model.showElement}">
        <f:format.raw>{model.layout.elementOuterWrap.0}</f:format.raw>
        <f:for each="{model.additionalArguments.uploadedFiles}" as="uploadedFile">
-               {uploadedFile.originalFilename}<br />
+               {uploadedFile.name}<br />
        </f:for>
        <f:format.raw>{model.layout.elementOuterWrap.1}</f:format.raw>
 </f:if>
index f9eec9c..4e29340 100644 (file)
@@ -1,5 +1,5 @@
 <f:if condition="{model.showElement}">
        <f:format.raw>{model.layout.elementOuterWrap.0}</f:format.raw>
-               <f:for each="{model.additionalArguments.uploadedFiles}" as="uploadedFile">{uploadedFile.originalFilename}<br /></f:for>
+               <f:for each="{model.additionalArguments.uploadedFiles}" as="uploadedFile">{uploadedFile.name}<br /></f:for>
        <f:format.raw>{model.layout.elementOuterWrap.1}</f:format.raw>
 </f:if>
index 5a0a821..bb95731 100644 (file)
@@ -1,2 +1,2 @@
-{namespace form=TYPO3\CMS\Form\ViewHelpers}<f:if condition="{model.showElement}"><form:plainMail labelContent="{model}" newLineAfterLabel="1" indent="4" /><f:for each="{model.additionalArguments.uploadedFiles}" as="uploadedFile"><form:plainMail content="{uploadedFile.originalFilename}" />
+{namespace form=TYPO3\CMS\Form\ViewHelpers}<f:if condition="{model.showElement}"><form:plainMail labelContent="{model}" newLineAfterLabel="1" indent="4" /><f:for each="{model.additionalArguments.uploadedFiles}" as="uploadedFile"><form:plainMail content="{uploadedFile.name}" />
 </f:for><form:plainMail indent="-4"/></f:if>
\ No newline at end of file
index 50c5b55..0bd9d2c 100644 (file)
@@ -2,7 +2,7 @@
        <li class="csc-form-{model.elementCounter} csc-form-element csc-form-element-{model.elementTypeLowerCase}">
                <label>{model.additionalArguments.label}</label>
                <f:for each="{model.additionalArguments.uploadedFiles}" as="uploadedFile">
-                       {uploadedFile.originalFilename}<br />
+                       {uploadedFile.name}<br />
                </f:for>
        </li>
 </f:if>
index 042c67b..34082b1 100644 (file)
@@ -4,7 +4,7 @@
     <em>{model.additionalArguments.label}</em>
   </td>
   <td>
-<f:for each="{model.additionalArguments.uploadedFiles}" as="uploadedFile">{uploadedFile.originalFilename}<br /></f:for>
+<f:for each="{model.additionalArguments.uploadedFiles}" as="uploadedFile">{uploadedFile.name}<br /></f:for>
   </td>
 </tr>
 </f:if>
index 5a0a821..bb95731 100644 (file)
@@ -1,2 +1,2 @@
-{namespace form=TYPO3\CMS\Form\ViewHelpers}<f:if condition="{model.showElement}"><form:plainMail labelContent="{model}" newLineAfterLabel="1" indent="4" /><f:for each="{model.additionalArguments.uploadedFiles}" as="uploadedFile"><form:plainMail content="{uploadedFile.originalFilename}" />
+{namespace form=TYPO3\CMS\Form\ViewHelpers}<f:if condition="{model.showElement}"><form:plainMail labelContent="{model}" newLineAfterLabel="1" indent="4" /><f:for each="{model.additionalArguments.uploadedFiles}" as="uploadedFile"><form:plainMail content="{uploadedFile.name}" />
 </f:for><form:plainMail indent="-4"/></f:if>
\ No newline at end of file