From ca07015b284d64db4231d57ff2a6d5dece9f8e6f Mon Sep 17 00:00:00 2001
From: Ralf Zimmermann <ralf.zimmermann@tritum.de>
Date: Tue, 6 Oct 2015 10:34:01 +0200
Subject: [PATCH] [TASK] EXT:form - Optimize file upload/ handling of files
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>
---
.../ArrayToValidationElementConverter.php | 51 ++++++++++++++-
.../Validator/FileAllowedTypesValidator.php | 43 ++-----------
.../Validator/ValidationElementValidator.php | 64 +++++++++++++++++--
.../Hooks/HandleIncomingFormValues.php | 50 ++++-----------
.../Classes/PostProcess/MailPostProcessor.php | 2 +-
.../form/Classes/Utility/SessionUtility.php | 1 -
.../Confirmation/FlatElements/Upload.html | 2 +-
.../Mail/Html/FlatElements/Upload.html | 2 +-
.../Mail/Plain/FlatElements/Upload.html | 2 +-
.../Confirmation/FlatElements/Upload.html | 2 +-
.../Mail/Html/FlatElements/Upload.html | 2 +-
.../Mail/Plain/FlatElements/Upload.html | 2 +-
12 files changed, 135 insertions(+), 88 deletions(-)
diff --git a/typo3/sysext/form/Classes/Domain/Property/TypeConverter/ArrayToValidationElementConverter.php b/typo3/sysext/form/Classes/Domain/Property/TypeConverter/ArrayToValidationElementConverter.php
index 09d8974858b9..f4b55ce4c227 100644
--- a/typo3/sysext/form/Classes/Domain/Property/TypeConverter/ArrayToValidationElementConverter.php
+++ b/typo3/sysext/form/Classes/Domain/Property/TypeConverter/ArrayToValidationElementConverter.php
@@ -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);
}
diff --git a/typo3/sysext/form/Classes/Domain/Validator/FileAllowedTypesValidator.php b/typo3/sysext/form/Classes/Domain/Validator/FileAllowedTypesValidator.php
index 039a2c87e694..c627fe131aae 100755
--- a/typo3/sysext/form/Classes/Domain/Validator/FileAllowedTypesValidator.php
+++ b/typo3/sysext/form/Classes/Domain/Validator/FileAllowedTypesValidator.php
@@ -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],
diff --git a/typo3/sysext/form/Classes/Domain/Validator/ValidationElementValidator.php b/typo3/sysext/form/Classes/Domain/Validator/ValidationElementValidator.php
index e363d496e8c8..4ecb95b77e58 100644
--- a/typo3/sysext/form/Classes/Domain/Validator/ValidationElementValidator.php
+++ b/typo3/sysext/form/Classes/Domain/Validator/ValidationElementValidator.php
@@ -27,6 +27,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);
+ }
}
}
}
diff --git a/typo3/sysext/form/Classes/Hooks/HandleIncomingFormValues.php b/typo3/sysext/form/Classes/Hooks/HandleIncomingFormValues.php
index 06a867f3e977..4a8fc75469b2 100644
--- a/typo3/sysext/form/Classes/Hooks/HandleIncomingFormValues.php
+++ b/typo3/sysext/form/Classes/Hooks/HandleIncomingFormValues.php
@@ -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;
diff --git a/typo3/sysext/form/Classes/PostProcess/MailPostProcessor.php b/typo3/sysext/form/Classes/PostProcess/MailPostProcessor.php
index c704b4a271de..00a29509a270 100644
--- a/typo3/sysext/form/Classes/PostProcess/MailPostProcessor.php
+++ b/typo3/sysext/form/Classes/PostProcess/MailPostProcessor.php
@@ -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']));
}
}
}
diff --git a/typo3/sysext/form/Classes/Utility/SessionUtility.php b/typo3/sysext/form/Classes/Utility/SessionUtility.php
index e22dd96519a9..82ea3778b5f5 100644
--- a/typo3/sysext/form/Classes/Utility/SessionUtility.php
+++ b/typo3/sysext/form/Classes/Utility/SessionUtility.php
@@ -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']);
}
diff --git a/typo3/sysext/form/Resources/Private/Partials/Compatibility/Confirmation/FlatElements/Upload.html b/typo3/sysext/form/Resources/Private/Partials/Compatibility/Confirmation/FlatElements/Upload.html
index 1afa3ca04e5c..9837361c8b93 100644
--- a/typo3/sysext/form/Resources/Private/Partials/Compatibility/Confirmation/FlatElements/Upload.html
+++ b/typo3/sysext/form/Resources/Private/Partials/Compatibility/Confirmation/FlatElements/Upload.html
@@ -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>
diff --git a/typo3/sysext/form/Resources/Private/Partials/Compatibility/PostProcessor/Mail/Html/FlatElements/Upload.html b/typo3/sysext/form/Resources/Private/Partials/Compatibility/PostProcessor/Mail/Html/FlatElements/Upload.html
index f9eec9cfaa4c..4e2934003840 100644
--- a/typo3/sysext/form/Resources/Private/Partials/Compatibility/PostProcessor/Mail/Html/FlatElements/Upload.html
+++ b/typo3/sysext/form/Resources/Private/Partials/Compatibility/PostProcessor/Mail/Html/FlatElements/Upload.html
@@ -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>
diff --git a/typo3/sysext/form/Resources/Private/Partials/Compatibility/PostProcessor/Mail/Plain/FlatElements/Upload.html b/typo3/sysext/form/Resources/Private/Partials/Compatibility/PostProcessor/Mail/Plain/FlatElements/Upload.html
index 5a0a8219f736..bb9573121a7a 100644
--- a/typo3/sysext/form/Resources/Private/Partials/Compatibility/PostProcessor/Mail/Plain/FlatElements/Upload.html
+++ b/typo3/sysext/form/Resources/Private/Partials/Compatibility/PostProcessor/Mail/Plain/FlatElements/Upload.html
@@ -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
diff --git a/typo3/sysext/form/Resources/Private/Partials/Default/Confirmation/FlatElements/Upload.html b/typo3/sysext/form/Resources/Private/Partials/Default/Confirmation/FlatElements/Upload.html
index 50c5b5520046..0bd9d2c01c8a 100644
--- a/typo3/sysext/form/Resources/Private/Partials/Default/Confirmation/FlatElements/Upload.html
+++ b/typo3/sysext/form/Resources/Private/Partials/Default/Confirmation/FlatElements/Upload.html
@@ -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>
diff --git a/typo3/sysext/form/Resources/Private/Partials/Default/PostProcessor/Mail/Html/FlatElements/Upload.html b/typo3/sysext/form/Resources/Private/Partials/Default/PostProcessor/Mail/Html/FlatElements/Upload.html
index 042c67be9ba0..34082b12609c 100644
--- a/typo3/sysext/form/Resources/Private/Partials/Default/PostProcessor/Mail/Html/FlatElements/Upload.html
+++ b/typo3/sysext/form/Resources/Private/Partials/Default/PostProcessor/Mail/Html/FlatElements/Upload.html
@@ -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>
diff --git a/typo3/sysext/form/Resources/Private/Partials/Default/PostProcessor/Mail/Plain/FlatElements/Upload.html b/typo3/sysext/form/Resources/Private/Partials/Default/PostProcessor/Mail/Plain/FlatElements/Upload.html
index 5a0a8219f736..bb9573121a7a 100644
--- a/typo3/sysext/form/Resources/Private/Partials/Default/PostProcessor/Mail/Plain/FlatElements/Upload.html
+++ b/typo3/sysext/form/Resources/Private/Partials/Default/PostProcessor/Mail/Plain/FlatElements/Upload.html
@@ -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
--
GitLab