diff --git a/Storage/src/Connection/Rest.php b/Storage/src/Connection/Rest.php index 2db6ee7546bb..3e9ba793b3b6 100644 --- a/Storage/src/Connection/Rest.php +++ b/Storage/src/Connection/Rest.php @@ -306,6 +306,7 @@ public function downloadObject(array $args = []) $transcodedObj = true; } }; + $attempt = null; $requestOptions['restRetryListener'] = function ( \Exception $e, $retryAttempt, @@ -313,7 +314,8 @@ public function downloadObject(array $args = []) ) use ( $resultStream, $requestedBytes, - $invocationId + $invocationId, + &$attempt, ) { // if the exception has a response for us to use if ($e instanceof RequestException && $e->hasResponse()) { @@ -331,6 +333,9 @@ public function downloadObject(array $args = []) // modify the range headers to fetch the remaining data $arguments[1]['headers']['Range'] = sprintf('bytes=%s-%s', $startByte, $endByte); $arguments[0] = $this->modifyRequestForRetry($arguments[0], $retryAttempt, $invocationId); + + // Copy the final result to the end of the stream + $attempt = $retryAttempt; } }; @@ -339,6 +344,13 @@ public function downloadObject(array $args = []) $requestOptions )->getBody(); + // If no retry attempt was made, then we can return the stream as is. + // This is important in the case where downloadObject is called to open + // the file but not to read from it yet. + if ($attempt === null) { + return $fetchedStream; + } + // If our object is a transcoded object, then Range headers are not honoured. // That means even if we had a partial download available, the final obj // that was fetched will contain the complete object. So, we don't need to copy diff --git a/Storage/tests/Unit/StorageObjectTest.php b/Storage/tests/Unit/StorageObjectTest.php index 7a5c4630b100..787ff11f23e0 100644 --- a/Storage/tests/Unit/StorageObjectTest.php +++ b/Storage/tests/Unit/StorageObjectTest.php @@ -635,6 +635,31 @@ public function provideDownloadAsStreamRetryHeaders() ]; } + public function testDownloadAsStreamShouldNotReadFromStream() + { + $stream = $this->prophesize(StreamInterface::class); + $stream->eof()->shouldNotBeCalled(); + $stream->read(Argument::any())->shouldNotBeCalled(); + + $httpHandler = function ($request, $options) use ($stream) { + return new Response(200, [], $stream->reveal()); + }; + + $rest = new Rest([ + 'httpHandler' => $httpHandler, + // Mock the authHttpHandler so it doesn't make a real request + 'authHttpHandler' => function () { + return new Response(200, [], '{"access_token": "abc"}'); + } + ]); + + $object = new StorageObject($rest, 'object', 'bucket'); + $stream = $object->downloadAsStream(); + + // assert the resulting stream looks like we'd expect + $this->assertInstanceOf(StreamInterface::class, $stream); + } + public function testGetsInfo() { $objectInfo = [