[HttpFoundation] UploadedFile - moved a security check
Squashed commit of the following:
commit b03b32ecc985c4a4f9dc7df2d3336a4cd75aae30
Merge: fb7004b
fc70e13
Author: Bilal Amarni <bilal.amarni@gmail.com>
Date: Wed Feb 27 11:33:37 2013 +0100
[HttpFoundation] UploadedFile - moved a security check
commit fc70e138c1d3858775c9efe51268cae6d7ec3f69
Author: Bilal Amarni <bilal.amarni@gmail.com>
Date: Thu Jan 24 11:07:29 2013 +0100
explicitly passed UPLOAD_ERR_OK constant in a test
commit dda03a2faab9539ca3a93736dd2bc0ec27feb4e7
Author: Bilal Amarni <bilal.amarni@gmail.com>
Date: Fri Jan 18 17:24:06 2013 +0100
[HttpFoundation] UploadedFile - moved a security check from move() to isValid()
This commit is contained in:
parent
69dbbdda3d
commit
5bb44f52a0
@ -179,13 +179,15 @@ class UploadedFile extends File
|
||||
/**
|
||||
* Returns whether the file was uploaded successfully.
|
||||
*
|
||||
* @return Boolean True if no error occurred during uploading
|
||||
* @return Boolean True if the file has been uploaded with HTTP and no error occurred.
|
||||
*
|
||||
* @api
|
||||
*/
|
||||
public function isValid()
|
||||
{
|
||||
return $this->error === UPLOAD_ERR_OK;
|
||||
$isOk = $this->error === UPLOAD_ERR_OK;
|
||||
|
||||
return $this->test ? $isOk : $isOk && is_uploaded_file($this->getPathname());
|
||||
}
|
||||
|
||||
/**
|
||||
@ -196,7 +198,7 @@ class UploadedFile extends File
|
||||
*
|
||||
* @return File A File object representing the new file
|
||||
*
|
||||
* @throws FileException if the file has not been uploaded via Http
|
||||
* @throws FileException if, for any reason, the file could not have been moved
|
||||
*
|
||||
* @api
|
||||
*/
|
||||
@ -205,21 +207,21 @@ class UploadedFile extends File
|
||||
if ($this->isValid()) {
|
||||
if ($this->test) {
|
||||
return parent::move($directory, $name);
|
||||
} elseif (is_uploaded_file($this->getPathname())) {
|
||||
$target = $this->getTargetFile($directory, $name);
|
||||
|
||||
if (!@move_uploaded_file($this->getPathname(), $target)) {
|
||||
$error = error_get_last();
|
||||
throw new FileException(sprintf('Could not move the file "%s" to "%s" (%s)', $this->getPathname(), $target, strip_tags($error['message'])));
|
||||
}
|
||||
|
||||
@chmod($target, 0666 & ~umask());
|
||||
|
||||
return $target;
|
||||
}
|
||||
|
||||
$target = $this->getTargetFile($directory, $name);
|
||||
|
||||
if (!@move_uploaded_file($this->getPathname(), $target)) {
|
||||
$error = error_get_last();
|
||||
throw new FileException(sprintf('Could not move the file "%s" to "%s" (%s)', $this->getPathname(), $target, strip_tags($error['message'])));
|
||||
}
|
||||
|
||||
@chmod($target, 0666 & ~umask());
|
||||
|
||||
return $target;
|
||||
}
|
||||
|
||||
throw new FileException(sprintf('The file "%s" has not been uploaded via Http', $this->getPathname()));
|
||||
throw new FileException(sprintf('The file "%s" is not valid', $this->getPathname()));
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -197,7 +197,8 @@ class UploadedFileTest extends \PHPUnit_Framework_TestCase
|
||||
'original.gif',
|
||||
null,
|
||||
filesize(__DIR__.'/Fixtures/test.gif'),
|
||||
UPLOAD_ERR_OK
|
||||
UPLOAD_ERR_OK,
|
||||
true
|
||||
);
|
||||
|
||||
$this->assertTrue($file->isValid());
|
||||
@ -229,4 +230,17 @@ class UploadedFileTest extends \PHPUnit_Framework_TestCase
|
||||
array(UPLOAD_ERR_EXTENSION),
|
||||
);
|
||||
}
|
||||
|
||||
public function testIsInvalidIfNotHttpUpload()
|
||||
{
|
||||
$file = new UploadedFile(
|
||||
__DIR__.'/Fixtures/test.gif',
|
||||
'original.gif',
|
||||
null,
|
||||
filesize(__DIR__.'/Fixtures/test.gif'),
|
||||
UPLOAD_ERR_OK
|
||||
);
|
||||
|
||||
$this->assertFalse($file->isValid());
|
||||
}
|
||||
}
|
||||
|
@ -114,7 +114,7 @@ class ClientTest extends \PHPUnit_Framework_TestCase
|
||||
|
||||
$files = array(
|
||||
array('tmp_name' => $source, 'name' => 'original', 'type' => 'mime/original', 'size' => 123, 'error' => UPLOAD_ERR_OK),
|
||||
new UploadedFile($source, 'original', 'mime/original', 123, UPLOAD_ERR_OK),
|
||||
new UploadedFile($source, 'original', 'mime/original', 123, UPLOAD_ERR_OK, true),
|
||||
);
|
||||
|
||||
foreach ($files as $file) {
|
||||
@ -147,7 +147,7 @@ class ClientTest extends \PHPUnit_Framework_TestCase
|
||||
|
||||
$file = $this
|
||||
->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')
|
||||
->setConstructorArgs(array($source, 'original', 'mime/original', 123, UPLOAD_ERR_OK))
|
||||
->setConstructorArgs(array($source, 'original', 'mime/original', 123, UPLOAD_ERR_OK, true))
|
||||
->setMethods(array('getSize'))
|
||||
->getMock()
|
||||
;
|
||||
|
@ -82,7 +82,7 @@ abstract class FileValidatorTest extends \PHPUnit_Framework_TestCase
|
||||
$this->context->expects($this->never())
|
||||
->method('addViolation');
|
||||
|
||||
$file = new UploadedFile($this->path, 'originalName');
|
||||
$file = new UploadedFile($this->path, 'originalName', null, null, null, true);
|
||||
$this->validator->validate($file, new File());
|
||||
}
|
||||
|
||||
|
Reference in New Issue
Block a user