fix(Storage): ensure streams arent copied by default (#7495)

Storage/src/Connection/Rest.php

@@ -306,14 +306,16 @@ public function downloadObject(array $args = [])
                 $transcodedObj = true;
             }
         };
+        $attempt = null;
         $requestOptions['restRetryListener'] = function (
             \Exception $e,
             $retryAttempt,
             &$arguments
         ) 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

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 = [