bugfix: prevent resources provided to ObjectUploader from being closed by Guzzle

Author: Sean O'Brien

.changes/nextrelease/fix-objectuploader-stream.json

@@ -0,0 +1,7 @@
+[
+  {
+    "type": "bugfix",
+    "category": "S3",
+    "description": "Prevents resources provided to `ObjectUploader` from being closed by Guzzle."
+  }
+]

src/S3/ObjectUploader.php

@@ -4,6 +4,7 @@
 use GuzzleHttp\Promise\PromiseInterface;
 use GuzzleHttp\Promise\PromisorInterface;
 use GuzzleHttp\Psr7;
+use GuzzleHttp\Psr7\FnStream;
 use Psr\Http\Message\StreamInterface;
 
 /**
@@ -57,7 +58,7 @@ public function __construct(
         $this->client = $client;
         $this->bucket = $bucket;
         $this->key = $key;
-        $this->body = Psr7\Utils::streamFor($body);
+        $this->body = $this->createStream($body);
         $this->acl = $acl;
         $this->options = $options + self::$defaults;
         // Handle "add_content_md5" option.
@@ -100,6 +101,33 @@ public function upload()
         return $this->promise()->wait();
     }
 
+    /**
+     * Creates a stream from the provided body, ensuring that user-provided
+     * PHP stream resources are not closed when the stream is destructed.
+     *
+     * @param mixed $body
+     *
+     * @return StreamInterface
+     */
+    private function createStream($body): StreamInterface
+    {
+        $stream = Psr7\Utils::streamFor($body);
+
+        // When the user provides a raw PHP resource, we should not take
+        // ownership of it (i.e., we should not fclose it on destruct).
+        // Decorate with a no-op close to prevent the underlying Stream
+        // from closing the user's resource handle.
+        if (is_resource($body)) {
+            return FnStream::decorate($stream, [
+                'close' => function () use ($stream) {
+                    $stream->detach();
+                },
+            ]);
+        }
+
+        return $stream;
+    }
+
     /**
      * Determines if the body should be uploaded using PutObject or the
      * Multipart Upload System. It also modifies the passed-in $body as needed

tests/S3/ObjectUploaderTest.php

@@ -381,4 +381,70 @@ public function testAddContentMd5EmitsDeprecationNotice()
             restore_error_handler();
         }
     }
+
+    /**
+     * Tests that a user-provided PHP resource handle is NOT closed by
+     * ObjectUploader after the upload completes (PutObject path).
+     *
+     * Regression test for: https://github.com/aws/aws-sdk-php/issues/XXXX
+     * The cyclic reference fix (PR #3290) caused Stream::__destruct() to fire
+     * promptly, which closed user-provided resource handles.
+     */
+    public function testUploadDoesNotCloseUserProvidedResourceHandle()
+    {
+        $client = $this->getTestClient('S3');
+        $this->addMockResults($client, [new Result()]);
+
+        $fp = fopen('php://memory', 'r+');
+        fwrite($fp, 'Hello, World!');
+        fseek($fp, 0);
+
+        (new ObjectUploader($client, 'bucket', 'key', $fp))->upload();
+
+        $this->assertTrue(
+            is_resource($fp),
+            'User-provided resource handle should remain open after upload()'
+        );
+
+        // Clean up
+        fclose($fp);
+    }
+
+    /**
+     * Tests that a user-provided PHP resource handle is NOT closed by
+     * ObjectUploader after the upload completes (multipart path).
+     */
+    public function testMultipartUploadDoesNotCloseUserProvidedResourceHandle()
+    {
+        $client = $this->getTestClient('S3');
+        $this->addMockResults($client, [
+            new Result(['UploadId' => 'foo']),
+            new Result(['ETag' => 'bar']),
+            new Result(['ETag' => 'bar']),
+            new Result(['Location' => 'https://bucket.s3.amazonaws.com/key']),
+        ]);
+
+        // Create a resource with content larger than the multipart threshold
+        $fp = fopen('php://temp', 'r+');
+        $data = str_repeat('x', 6 * self::MB);
+        fwrite($fp, $data);
+        fseek($fp, 0);
+
+        (new ObjectUploader(
+            $client,
+            'bucket',
+            'key',
+            $fp,
+            'private',
+            ['mup_threshold' => 4 * self::MB]
+        ))->upload();
+
+        $this->assertTrue(
+            is_resource($fp),
+            'User-provided resource handle should remain open after multipart upload()'
+        );
+
+        // Clean up
+        fclose($fp);
+    }
 }