Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merge release 2.11.3 into 2.12.x #99

Merged
merged 5 commits into from Jul 6, 2022
Merged

Merge release 2.11.3 into 2.12.x #99

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5773e76
fixed UploadedFile::moveTo doesn't work by cli. Original file leave i…
k2rn Jul 4, 2022
197f195
cleanup file regardless of the stream
k2rn Jul 4, 2022
091487a
Cleaned up `UploadedFileTest` by providing return types and parameter…
Ocramius Jul 6, 2022
1f97b0c
Merge pull request #98 from k2rn/fixed_UploadedFile_moveTo
Ocramius Jul 6, 2022
198cd9a
Merge branch '2.12.x' into 2.11.x-merge-up-into-2.12.x_vi6yjNRm
Ocramius Jul 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 1 addition & 45 deletions psalm-baseline.xml
View file
Open in desktop
Expand Up @@ -1164,54 +1164,10 @@
</MissingReturnType>
</file>
<file src="test/UploadedFileTest.php">
<MissingParamType occurrences="6">
<code>$path</code>
<code>$status</code>
<code>$status</code>
<code>$status</code>
<code>$status</code>
<code>$streamOrFile</code>
</MissingParamType>
<MissingPropertyType occurrences="1">
<code>$tmpFile</code>
</MissingPropertyType>
<MissingReturnType occurrences="24">
<code>errorConstantsAndMessages</code>
<code>invalidErrorStatuses</code>
<code>invalidMovePaths</code>
<code>invalidStreams</code>
<code>nonOkErrorStatus</code>
<code>testCannotRetrieveStreamAfterMove</code>
<code>testConstructorDoesNotRaiseExceptionForInvalidStreamWhenErrorStatusPresent</code>
<code>testGetStreamRaisesExceptionWhenErrorStatusPresent</code>
<code>testGetStreamRaisesExceptionWithAppropriateMessageWhenUploadErrorDetected</code>
<code>testGetStreamReturnsOriginalStreamObject</code>
<code>testGetStreamReturnsStreamForFile</code>
<code>testGetStreamReturnsWrappedPhpStream</code>
<code>testMoveCannotBeCalledMoreThanOnce</code>
<code>testMoveRaisesExceptionForInvalidPath</code>
<code>testMoveToCreatesStreamIfOnlyAFilenameWasProvided</code>
<code>testMoveToRaisesExceptionWhenErrorStatusPresent</code>
<code>testMoveToRaisesExceptionWithAppropriateMessageWhenUploadErrorDetected</code>
<code>testMovesFileToDesignatedPath</code>
<code>testRaisesExceptionOnInvalidErrorStatus</code>
<code>testRaisesExceptionOnInvalidStreamOrFile</code>
<code>testValidClientFilename</code>
<code>testValidClientMediaType</code>
<code>testValidNullClientFilename</code>
<code>testValidSize</code>
</MissingReturnType>
<MixedArgument occurrences="6">
<MixedArgument occurrences="2">
<code>$path</code>
<code>$status</code>
<code>$status</code>
<code>$status</code>
<code>$status</code>
<code>$streamOrFile</code>
</MixedArgument>
<UndefinedThisPropertyAssignment occurrences="1">
<code>$this-&gt;tmpfile</code>
</UndefinedThisPropertyAssignment>
</file>
<file src="test/UriTest.php">
<InvalidScalarArgument occurrences="2">
Expand Down
7 changes: 7 additions & 0 deletions src/UploadedFile.php
View file
Open in desktop
Expand Up @@ -187,6 +187,13 @@ public function moveTo($targetPath) : void
case (empty($sapi) || 0 === strpos($sapi, 'cli') || 0 === strpos($sapi, 'phpdbg') || ! $this->file):
// Non-SAPI environment, or no filename present
$this->writeFile($targetPath);

if ($this->stream instanceof StreamInterface) {
$this->stream->close();
}
if (is_string($this->file) && file_exists($this->file)) {
unlink($this->file);
}
break;
default:
// SAPI environment, with file present
Expand Down
113 changes: 72 additions & 41 deletions test/UploadedFileTest.php
View file
Open in desktop
Expand Up @@ -30,23 +30,33 @@
use const UPLOAD_ERR_OK;
use const UPLOAD_ERR_PARTIAL;

class UploadedFileTest extends TestCase
final class UploadedFileTest extends TestCase
{
protected $tmpFile;
/** @var false|null|string */
private $orgFile;

/** @var mixed */
private $tmpFile;

protected function setUp() : void
{
$this->tmpFile = null;
$this->orgFile = null;
}

protected function tearDown() : void
{
if (is_string($this->tmpFile) && file_exists($this->tmpFile)) {
unlink($this->tmpFile);
}

if (is_string($this->orgFile) && file_exists($this->orgFile)) {
unlink($this->orgFile);
}
}

public function invalidStreams()
/** @return non-empty-array<non-empty-string, array{mixed}> */
public function invalidStreams(): array
{
return [
'null' => [null],
Expand All @@ -65,22 +75,24 @@ public function invalidStreams()

/**
* @dataProvider invalidStreams
* @param mixed $streamOrFile
*/
public function testRaisesExceptionOnInvalidStreamOrFile($streamOrFile)
public function testRaisesExceptionOnInvalidStreamOrFile($streamOrFile): void
{
$this->expectException(InvalidArgumentException::class);

new UploadedFile($streamOrFile, 0, UPLOAD_ERR_OK);
}

public function testValidSize()
public function testValidSize(): void
{
$uploaded = new UploadedFile(fopen('php://temp', 'wb+'), 123, UPLOAD_ERR_OK);

$this->assertSame(123, $uploaded->getSize());
}

public function invalidErrorStatuses()
/** @return non-empty-array<non-empty-string, array{int}> */
public function invalidErrorStatuses(): array
{
return [
'negative' => [-1],
Expand All @@ -91,48 +103,48 @@ public function invalidErrorStatuses()
/**
* @dataProvider invalidErrorStatuses
*/
public function testRaisesExceptionOnInvalidErrorStatus($status)
public function testRaisesExceptionOnInvalidErrorStatus(int $status): void
{
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('status');

new UploadedFile(fopen('php://temp', 'wb+'), 0, $status);
}

public function testValidClientFilename()
public function testValidClientFilename(): void
{
$file = new UploadedFile(fopen('php://temp', 'wb+'), 0, UPLOAD_ERR_OK, 'boo.txt');
$this->assertSame('boo.txt', $file->getClientFilename());
}

public function testValidNullClientFilename()
public function testValidNullClientFilename(): void
{
$file = new UploadedFile(fopen('php://temp', 'wb+'), 0, UPLOAD_ERR_OK, null);
$this->assertSame(null, $file->getClientFilename());
}

public function testValidClientMediaType()
public function testValidClientMediaType(): void
{
$file = new UploadedFile(fopen('php://temp', 'wb+'), 0, UPLOAD_ERR_OK, 'foobar.baz', 'mediatype');
$this->assertSame('mediatype', $file->getClientMediaType());
}

public function testGetStreamReturnsOriginalStreamObject()
public function testGetStreamReturnsOriginalStreamObject(): void
{
$stream = new Stream('php://temp');
$upload = new UploadedFile($stream, 0, UPLOAD_ERR_OK);
$this->assertSame($stream, $upload->getStream());
}

public function testGetStreamReturnsWrappedPhpStream()
public function testGetStreamReturnsWrappedPhpStream(): void
{
$stream = fopen('php://temp', 'wb+');
$upload = new UploadedFile($stream, 0, UPLOAD_ERR_OK);
$uploadStream = $upload->getStream()->detach();
$this->assertSame($stream, $uploadStream);
}

public function testGetStreamReturnsStreamForFile()
public function testGetStreamReturnsStreamForFile(): void
{
$this->tmpFile = $stream = tempnam(sys_get_temp_dir(), 'diac');
$upload = new UploadedFile($stream, 0, UPLOAD_ERR_OK);
Expand All @@ -142,20 +154,22 @@ public function testGetStreamReturnsStreamForFile()
$this->assertSame($stream, $r->getValue($uploadStream));
}

public function testMovesFileToDesignatedPath()
public function testMovesFileToDesignatedPath(): void
{
$originalContents = 'Foo bar!';
$stream = new Stream('php://temp', 'wb+');
$stream->write('Foo bar!');
$stream->write($originalContents);
$upload = new UploadedFile($stream, 0, UPLOAD_ERR_OK);

$this->tmpFile = $to = tempnam(sys_get_temp_dir(), 'diac');
$upload->moveTo($to);
$this->assertTrue(file_exists($to));
$contents = file_get_contents($to);
$this->assertSame($stream->__toString(), $contents);
$this->assertSame($originalContents, $contents);
}

public function invalidMovePaths()
/** @return non-empty-array<non-empty-string, array{mixed}> */
public function invalidMovePaths(): array
{
return [
'null' => [null],
Expand All @@ -171,8 +185,10 @@ public function invalidMovePaths()

/**
* @dataProvider invalidMovePaths
*
* @param mixed $path
*/
public function testMoveRaisesExceptionForInvalidPath($path)
public function testMoveRaisesExceptionForInvalidPath($path): void
{
$stream = new Stream('php://temp', 'wb+');
$stream->write('Foo bar!');
Expand All @@ -186,7 +202,7 @@ public function testMoveRaisesExceptionForInvalidPath($path)
$upload->moveTo($path);
}

public function testMoveCannotBeCalledMoreThanOnce()
public function testMoveCannotBeCalledMoreThanOnce(): void
{
$stream = new Stream('php://temp', 'wb+');
$stream->write('Foo bar!');
Expand All @@ -202,7 +218,7 @@ public function testMoveCannotBeCalledMoreThanOnce()
$upload->moveTo($to);
}

public function testCannotRetrieveStreamAfterMove()
public function testCannotRetrieveStreamAfterMove(): void
{
$stream = new Stream('php://temp', 'wb+');
$stream->write('Foo bar!');
Expand All @@ -218,7 +234,8 @@ public function testCannotRetrieveStreamAfterMove()
$upload->getStream();
}

public function nonOkErrorStatus()
/** @return non-empty-array<non-empty-string, array{positive-int}> */
public function nonOkErrorStatus(): array
{
return [
'UPLOAD_ERR_INI_SIZE' => [ UPLOAD_ERR_INI_SIZE ],
Expand All @@ -235,7 +252,7 @@ public function nonOkErrorStatus()
* @dataProvider nonOkErrorStatus
* @group 60
*/
public function testConstructorDoesNotRaiseExceptionForInvalidStreamWhenErrorStatusPresent($status)
public function testConstructorDoesNotRaiseExceptionForInvalidStreamWhenErrorStatusPresent(int $status): void
{
$uploadedFile = new UploadedFile('not ok', 0, $status);
$this->assertSame($status, $uploadedFile->getError());
Expand All @@ -245,7 +262,7 @@ public function testConstructorDoesNotRaiseExceptionForInvalidStreamWhenErrorSta
* @dataProvider nonOkErrorStatus
* @group 60
*/
public function testMoveToRaisesExceptionWhenErrorStatusPresent($status)
public function testMoveToRaisesExceptionWhenErrorStatusPresent(int $status): void
{
$uploadedFile = new UploadedFile('not ok', 0, $status);

Expand All @@ -259,7 +276,7 @@ public function testMoveToRaisesExceptionWhenErrorStatusPresent($status)
* @dataProvider nonOkErrorStatus
* @group 60
*/
public function testGetStreamRaisesExceptionWhenErrorStatusPresent($status)
public function testGetStreamRaisesExceptionWhenErrorStatusPresent(int $status): void
{
$uploadedFile = new UploadedFile('not ok', 0, $status);

Expand All @@ -272,20 +289,24 @@ public function testGetStreamRaisesExceptionWhenErrorStatusPresent($status)
/**
* @group 82
*/
public function testMoveToCreatesStreamIfOnlyAFilenameWasProvided()
public function testMoveToCreatesStreamIfOnlyAFilenameWasProvided(): void
{
$this->orgFile = tempnam(sys_get_temp_dir(), 'ORG');
$this->tmpFile = tempnam(sys_get_temp_dir(), 'DIA');
file_put_contents($this->orgFile, 'Hello');

$original = file_get_contents($this->orgFile);

$uploadedFile = new UploadedFile(__FILE__, 100, UPLOAD_ERR_OK, basename(__FILE__), 'text/plain');
$uploadedFile = new UploadedFile($this->orgFile, 100, UPLOAD_ERR_OK, basename($this->orgFile), 'text/plain');
$uploadedFile->moveTo($this->tmpFile);

$original = file_get_contents(__FILE__);
$test = file_get_contents($this->tmpFile);
$contents = file_get_contents($this->tmpFile);

$this->assertSame($original, $test);
$this->assertSame($original, $contents);
}

public function errorConstantsAndMessages()
/** @return iterable<int, array{int, non-empty-string}> */
public function errorConstantsAndMessages(): iterable
{
foreach (UploadedFile::ERROR_MESSAGES as $constant => $message) {
if ($constant === UPLOAD_ERR_OK) {
Expand All @@ -295,13 +316,11 @@ public function errorConstantsAndMessages()
}
}

/**
* @dataProvider errorConstantsAndMessages
* @param int $constant Upload error constant
* @param string $message Associated error message
*/
public function testGetStreamRaisesExceptionWithAppropriateMessageWhenUploadErrorDetected($constant, $message)
{
/** @dataProvider errorConstantsAndMessages */
public function testGetStreamRaisesExceptionWithAppropriateMessageWhenUploadErrorDetected(
int $constant,
string $message
): void {
$uploadedFile = new UploadedFile(__FILE__, 100, $constant);
$this->expectException(RuntimeException::class);
$this->expectExceptionMessage($message);
Expand All @@ -310,14 +329,26 @@ public function testGetStreamRaisesExceptionWithAppropriateMessageWhenUploadErro

/**
* @dataProvider errorConstantsAndMessages
* @param int $constant Upload error constant
* @param string $message Associated error message
*/
public function testMoveToRaisesExceptionWithAppropriateMessageWhenUploadErrorDetected($constant, $message)
{
public function testMoveToRaisesExceptionWithAppropriateMessageWhenUploadErrorDetected(
int $constant,
string $message
): void {
$uploadedFile = new UploadedFile(__FILE__, 100, $constant);
$this->expectException(RuntimeException::class);
$this->expectExceptionMessage($message);
$uploadedFile->moveTo('/tmp/foo');
}

public function testMoveToInCLIShouldRemoveOriginalFile(): void
{
$this->orgFile = tempnam(sys_get_temp_dir(), 'ORG');
file_put_contents($this->orgFile, 'Hello');
$upload = new UploadedFile($this->orgFile, 0, UPLOAD_ERR_OK);

$this->tmpFile = $to = tempnam(sys_get_temp_dir(), 'diac');
$upload->moveTo($to);
$this->assertFalse(file_exists($this->orgFile));
$this->assertTrue(file_exists($to));
}
}