Fix #17037, Fix #17729: Fix uploaded file saving for multipart forms,…

… add path alias support for UploadFile::saveAs()

framework/CHANGELOG.md

@@ -4,8 +4,10 @@ Yii Framework 2 Change Log
 2.0.32 under development
 ------------------------
 
+- Bug #17037: Fix uploaded file saving method in case generated by `MultipartFormDataParser` (sup-ham)
 - Bug #17803: Fixed ErrorHandler unregister and register only change global state when applicable (SamMousa)
 - Bug #17744: Fix a bug with setting incorrect `defaultValue` to AR column with `CURRENT_TIMESTAMP(x)` as default expression (MySQL >= 5.6.4) (bizley)
+- Enh #17729: Path alias support was added to `yii\web\UploadFile::saveAs()` (sup-ham)
 - Bug #17749: Dispatcher fix if target crashed in PHP 7.0+ (kamarton)
 - Bug #17762: PHP 7.4: Remove special condition for converting PHP errors to exceptions if they occurred inside of `__toString()` call (rob006)
 - Bug #17771: migrate/fresh was not returning exit code (samdark)

framework/web/MultipartFormDataParser.php

@@ -54,8 +54,7 @@
  *
  * > Note: although this parser fully emulates regular structure of the `$_FILES`, related temporary
  * files, which are available via `tmp_name` key, will not be recognized by PHP as uploaded ones.
- * Thus functions like `is_uploaded_file()` and `move_uploaded_file()` will fail on them. This also
- * means [[UploadedFile::saveAs()]] will fail as well.
+ * Thus functions like `is_uploaded_file()` and `move_uploaded_file()` will fail on them.
  *
  * @property int $uploadFileMaxCount Maximum upload files count.
  * @property int $uploadFileMaxSize Upload file max size in bytes.
@@ -191,6 +190,7 @@ public function parse($rawBody, $contentType)
                             @fclose($tmpResource);
                         } else {
                             fwrite($tmpResource, $value);
+                            rewind($tmpResource);
                             $fileInfo['tmp_name'] = $tmpFileName;
                             $fileInfo['tmp_resource'] = $tmpResource; // save file resource, otherwise it will be deleted
                         }

framework/web/UploadedFile.php

@@ -7,7 +7,9 @@
 
 namespace yii\web;
 
+use Yii;
 use yii\base\BaseObject;
+use yii\helpers\ArrayHelper;
 use yii\helpers\Html;
 
 /**
@@ -56,9 +58,25 @@ class UploadedFile extends BaseObject
      */
     public $error;
 
+    /**
+     * @var resource a temporary uploaded stream resource used within PUT and PATCH request.
+     */
+    private $_tempResource;
+
     private static $_files;
 
 
+    /**
+     * UploadedFile constructor.
+     *
+     * @param array $config name-value pairs that will be used to initialize the object properties
+     */
+    public function __construct($config = [])
+    {
+        $this->_tempResource = ArrayHelper::remove($config, 'tempResource');
+        parent::__construct($config);
+    }
+
     /**
      * String output.
      * This is PHP magic method that returns string representation of an object.
@@ -149,25 +167,68 @@ public static function reset()
 
     /**
      * Saves the uploaded file.
-     * Note that this method uses php's move_uploaded_file() method. If the target file `$file`
-     * already exists, it will be overwritten.
-     * @param string $file the file path used to save the uploaded file
+     * If the target file `$file` already exists, it will be overwritten.
+     * @param string $file the file path or a path alias used to save the uploaded file.
      * @param bool $deleteTempFile whether to delete the temporary file after saving.
      * If true, you will not be able to save the uploaded file again in the current request.
      * @return bool true whether the file is saved successfully
      * @see error
      */
     public function saveAs($file, $deleteTempFile = true)
     {
-        if ($this->error == UPLOAD_ERR_OK) {
-            if ($deleteTempFile) {
-                return move_uploaded_file($this->tempName, $file);
-            } elseif (is_uploaded_file($this->tempName)) {
-                return copy($this->tempName, $file);
-            }
+        if ($this->error !== UPLOAD_ERR_OK) {
+            return false;
+        }
+        if (false === $this->copyTempFile(Yii::getAlias($file))) {
+            return false;
         }
+        return !$deleteTempFile || $this->deleteTempFile();
+    }
 
-        return false;
+    /**
+     * Copy temporary file into file specified
+     *
+     * @param string $targetFile path of the file to copy to
+     * @return bool|int the total count of bytes copied, or false on failure
+     */
+    protected function copyTempFile($targetFile)
+    {
+        if (!is_resource($this->_tempResource)) {
+            return $this->isUploadedFile($this->tempName) && copy($this->tempName, $targetFile);
+        }
+        $target = fopen($targetFile, 'wb');
+        if ($target === false) {
+            return false;
+        }
+
+        $result = stream_copy_to_stream($this->_tempResource, $target);
+        @fclose($target);
+
+        return $result;
+    }
+
+    /**
+     * Delete temporary file
+     *
+     * @return bool if file was deleted
+     */
+    protected function deleteTempFile()
+    {
+        if (is_resource($this->_tempResource)) {
+            return @fclose($this->_tempResource);
+        }
+        return $this->isUploadedFile($this->tempName) && @unlink($this->tempName);
+    }
+
+    /**
+     * Check if file is uploaded file
+     *
+     * @param string $file path to the file to check
+     * @return bool
+     */
+    protected function isUploadedFile($file)
+    {
+        return is_uploaded_file($file);
     }
 
     /**
@@ -207,7 +268,8 @@ private static function loadFiles()
             self::$_files = [];
             if (isset($_FILES) && is_array($_FILES)) {
                 foreach ($_FILES as $class => $info) {
-                    self::loadFilesRecursive($class, $info['name'], $info['tmp_name'], $info['type'], $info['size'], $info['error']);
+                    $resource = isset($info['tmp_resource']) ? $info['tmp_resource'] : [];
+                    self::loadFilesRecursive($class, $info['name'], $info['tmp_name'], $info['type'], $info['size'], $info['error'], $resource);
                 }
             }
         }
@@ -224,16 +286,18 @@ private static function loadFiles()
      * @param mixed $sizes file sizes provided by PHP
      * @param mixed $errors uploading issues provided by PHP
      */
-    private static function loadFilesRecursive($key, $names, $tempNames, $types, $sizes, $errors)
+    private static function loadFilesRecursive($key, $names, $tempNames, $types, $sizes, $errors, $tempResources)
     {
         if (is_array($names)) {
             foreach ($names as $i => $name) {
-                self::loadFilesRecursive($key . '[' . $i . ']', $name, $tempNames[$i], $types[$i], $sizes[$i], $errors[$i]);
+                $resource = isset($tempResources[$i]) ? $tempResources[$i] : [];
+                self::loadFilesRecursive($key . '[' . $i . ']', $name, $tempNames[$i], $types[$i], $sizes[$i], $errors[$i], $resource);
             }
         } elseif ((int) $errors !== UPLOAD_ERR_NO_FILE) {
             self::$_files[$key] = [
                 'name' => $names,
                 'tempName' => $tempNames,
+                'tempResource' => $tempResources,
                 'type' => $types,
                 'size' => $sizes,
                 'error' => $errors,

tests/framework/web/UploadedFileTest.php

@@ -8,6 +8,7 @@
 namespace yiiunit\framework\web;
 
 use yii\web\UploadedFile;
+use yiiunit\framework\web\mocks\UploadedFileMock;
 use yiiunit\framework\web\stubs\ModelStub;
 use yiiunit\framework\web\stubs\VendorImage;
 use yiiunit\TestCase;
@@ -35,6 +36,17 @@ private function generateFakeFileData()
         ];
     }
 
+    private function generateTempFileData()
+    {
+        return [
+            'name' => md5(mt_rand()),
+            'tmp_name' => tempnam(sys_get_temp_dir(), ''),
+            'type' => 'image/jpeg',
+            'size' => mt_rand(1000, 10000),
+            'error' => 0,
+        ];
+    }
+
     private function generateFakeFiles()
     {
         $_FILES['ModelStub[prod_image]'] = $this->generateFakeFileData();
@@ -46,6 +58,8 @@ private function generateFakeFiles()
         $_FILES['ModelStub[vendor_images][]'] = $this->generateFakeFileData();
         $_FILES['ModelStub[vendor_images][]'] = $this->generateFakeFileData();
         $_FILES['ModelStub[vendor_images][]'] = $this->generateFakeFileData();
+
+        $_FILES['ModelStub[temp_image]'] = $this->generateTempFileData();
     }
 
     // Tests :
@@ -72,4 +86,31 @@ public function testGetInstances()
             $this->assertInstanceOf(VendorImage::className(), $vendorImage);
         }
     }
+
+    public function testSaveAs()
+    {
+        $tmpImage = UploadedFileMock::getInstance(new ModelStub(), 'temp_image');
+        $targetFile = '@runtime/test_saved_uploaded_file_' . time();
+
+        $this->assertEquals(true, $tmpImage->saveAs($targetFile, $deleteTempFile = false));
+        $this->assertEquals(true, $tmpImage->saveAs($targetFile, $deleteTempFile = true));
+        $this->assertEquals(false, $tmpImage->saveAs($targetFile)); // temp file should not exist
+
+        @unlink($targetFile);
+    }
+
+    public function testSaveFileFromMultipartFormDataParser()
+    {
+        $_FILES = [];
+        UploadedFile::reset();
+        $model = new ModelStub();
+        $targetFile = '@runtime/test_saved_uploaded_file_' . time();
+
+        (new MultipartFormDataParserTest)->testParse();
+        $_FILES['ModelStub'] = $_FILES['Item']; // $_FILES[Item] here from testParse() above
+        $tmpFile = UploadedFile::getInstance($model, 'file');
+
+        $this->assertEquals(true, $tmpFile->saveAs($targetFile));
+        @unlink($targetFile);
+    }
 }

tests/framework/web/mocks/UploadedFileMock.php

@@ -0,0 +1,19 @@
+<?php
+/**
+ * @link http://www.yiiframework.com/
+ * @copyright Copyright (c) 2008 Yii Software LLC
+ * @license http://www.yiiframework.com/license/
+ */
+
+namespace yiiunit\framework\web\mocks;
+
+class UploadedFileMock extends \yii\web\UploadedFile
+{
+    /**
+     * @inheritDoc
+     */
+    protected function isUploadedFile($file)
+    {
+        return is_file($file); // is_uploaded_file() won't work in test
+    }
+}