diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index b4160ab8a1c..99b7d0ac11b 100644 --- a/framework/CHANGELOG.md +++ b/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) diff --git a/framework/web/MultipartFormDataParser.php b/framework/web/MultipartFormDataParser.php index 13489c65675..0685160f1bd 100644 --- a/framework/web/MultipartFormDataParser.php +++ b/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 } diff --git a/framework/web/UploadedFile.php b/framework/web/UploadedFile.php index 49a0ffa3128..85d586fea49 100644 --- a/framework/web/UploadedFile.php +++ b/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,9 +167,8 @@ 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 @@ -159,15 +176,59 @@ public static function reset() */ 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, diff --git a/tests/framework/web/UploadedFileTest.php b/tests/framework/web/UploadedFileTest.php index e63d96d2925..a14033f6516 100644 --- a/tests/framework/web/UploadedFileTest.php +++ b/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); + } } diff --git a/tests/framework/web/mocks/UploadedFileMock.php b/tests/framework/web/mocks/UploadedFileMock.php new file mode 100644 index 00000000000..9bebceee9d0 --- /dev/null +++ b/tests/framework/web/mocks/UploadedFileMock.php @@ -0,0 +1,19 @@ +