Project

General

Profile

Actions
Copy link

Bug #77870

closed

Method moveTo in UploadedFile not implemented correctly according to PSR-7

Added by Mads Lønne Jensen about 8 years ago. Updated over 4 years ago.

Status:
Closed
Priority:
Must have
Assignee:
-
Category:
-
Target version:
-
Start date:
2016-09-07
Due date:
% Done:

100%

Estimated time:
1.00 h
TYPO3 Version:
8
PHP Version:
Tags:
Complexity:
no-brainer
Is Regression:
No
Sprint Focus:

Description

The method moveTo in TYPO3\CMS\Core\Http\UploadedFile should move the uploaded file to $target but is instead moved to $targetPath . basename($this->file), where $this->file is the temporary uploaded file.

The documentation for the moveTo method states that $targetPath is the "Path to which to move the uploaded file" and while it does not state that this path should be the path to the file and not just the folder, the accompanying example shows uses the path with a filename:

$file0->moveTo(DATA_DIR . '/' . $filename);

This is an issue in both 7.6.10 and 8.2.1

This issue is files as "bug" as it is an incorrect implementation of an interface.

Ref: http://www.php-fig.org/psr/psr-7/#1-6-uploaded-files

Actions
Copy link
 #1

Updated by Mads Lønne Jensen about 8 years ago

A proposed patch to fix this:

--- a/typo3/sysext/core/Classes/Http/UploadedFile.php
+++ b/typo3/sysext/core/Classes/Http/UploadedFile.php
@@ -185,7 +185,7 @@ class UploadedFile implements UploadedFileInterface
         }

         if (!empty($this->file) && is_uploaded_file($this->file)) {
-            if (GeneralUtility::upload_copy_move($this->file, $targetPath . basename($this->file)) === false) {
+            if (GeneralUtility::upload_copy_move($this->file, $targetPath) === false) {
                 throw new \RuntimeException('An error occurred while moving uploaded file', 1436717310);
             }
         } elseif ($this->stream) {
Actions
Copy link
 #2

Updated by Daniel Corn over 7 years ago

  • Priority changed from Should have to Must have

I would like to change the priority to "Must have". The method is not even consistent in it's behavior.

if (!empty($this->file) && is_uploaded_file($this->file)) {
    if (GeneralUtility::upload_copy_move($this->file, $targetPath . basename($this->file)) === false) ...
} elseif ($this->stream) {
    $handle = fopen($targetPath, 'wb+');
    ...
Actions
Copy link
 #3

Updated by Daniel Corn over 7 years ago

Additionally `The original file or stream MUST be removed on completion.` does not seem to be fulfilled for streams.

Actions
Copy link
 #4

Updated by Benni Mack about 7 years ago

Hey Mads,

would you be interested in coming up with a patch for that? Would be great to have that fixed...

Actions
Copy link
 #5

Updated by Benni Mack about 7 years ago

Benni Mack wrote:

Hey Mads,

would you be interested in coming up with a patch for that? Would be great to have that fixed...

Daniel, you're welcome as well :)

Actions
Copy link
 #6

Updated by Gerrit Code Review over 4 years ago

  • Status changed from New to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63920

Actions
Copy link
 #7

Updated by Gerrit Code Review over 4 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63920

Actions
Copy link
 #8

Updated by Susanne Moog over 4 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
Actions
Copy link
 #9

Updated by Benni Mack over 4 years ago

  • Status changed from Resolved to Closed
Actions
Copy link

Also available in: Atom PDF