Skip to content

Commit

Permalink
Disallow cloning Stream objects
Browse files Browse the repository at this point in the history
Streams hold a reference to the stateful resource handle for their actual
contents. Cloning a Stream will not actually clone the underlying resource,
thus both streams would still refer to the same resource after cloning and any
changes in one stream object would be reflected in the other object. This
violates user expectations after a cloning operation.

Disallow cloning entirely as the safe default choice. Alternatively a new
stream could be created and attached and the contents could be copied over.
This can get expensive with larger or infinite streams, though.

Signed-off-by: Tim Düsterhus <duesterhus@woltlab.com>
  • Loading branch information
TimWolla committed Apr 13, 2023
1 parent 13f45e5 commit 81de28d
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 0 deletions.
7 changes: 7 additions & 0 deletions src/Stream.php
Expand Up @@ -364,4 +364,11 @@ private function isValidStreamResourceType(mixed $resource): bool

return false;
}

/**
* Disallow stream cloning.
*/
private function __clone(): void

Check failure on line 371 in src/Stream.php

View workflow job for this annotation

GitHub Actions / ci / QA Checks (Psalm [8.0, locked], ubuntu-latest, laminas/laminas-continuous-integration-action@v1, ...

MethodSignatureMustOmitReturnType

src/Stream.php:371:5: MethodSignatureMustOmitReturnType: Method __clone must not declare a return type (see https://psalm.dev/168)
{
}
}
14 changes: 14 additions & 0 deletions test/StreamTest.php
Expand Up @@ -5,6 +5,7 @@
namespace LaminasTest\Diactoros;

use CurlHandle;
use Error;
use GdImage;
use InvalidArgumentException;
use Laminas\Diactoros\Stream;
Expand Down Expand Up @@ -679,4 +680,17 @@ public function testSizeReportsNullForPhpInputStreams(): void
$stream = new Stream($resource);
$this->assertNull($stream->getSize());
}

public function testStreamsAreUnclonable(): void
{
$stream = new Stream(\fopen('php://temp', 'r+'));

Check failure on line 686 in test/StreamTest.php

View workflow job for this annotation

GitHub Actions / ci / QA Checks (PHPCodeSniffer [8.0, locked], ubuntu-latest, laminas/laminas-continuous-integration-ac...

Function \fopen() should not be referenced via a fully qualified name, but via a use statement.
$stream->write('foo');

$this->assertSame('foo', $stream->__toString());

$this->expectException(Error::class);
$this->expectExceptionMessage('private Laminas\Diactoros\Stream::__clone()');

clone $stream;

Check failure on line 694 in test/StreamTest.php

View workflow job for this annotation

GitHub Actions / ci / QA Checks (Psalm [8.0, locked], ubuntu-latest, laminas/laminas-continuous-integration-action@v1, ...

InvalidClone

test/StreamTest.php:694:9: InvalidClone: Cannot clone Laminas\Diactoros\Stream (see https://psalm.dev/069)
}
}

0 comments on commit 81de28d

Please sign in to comment.