Project

General

Profile

Actions

Bug #77870

closed

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

Added by Mads Lønne Jensen over 7 years ago. Updated almost 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 #1

Updated by Mads Lønne Jensen over 7 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 #2

Updated by Daniel Corn about 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 #3

Updated by Daniel Corn about 7 years ago

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

Actions #4

Updated by Benni Mack over 6 years ago

Hey Mads,

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

Actions #5

Updated by Benni Mack over 6 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 #6

Updated by Gerrit Code Review about 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 #7

Updated by Gerrit Code Review about 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 #8

Updated by Susanne Moog about 4 years ago

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

Updated by Benni Mack almost 4 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF