diff --git a/.changes/nextrelease/fix-objectuploader-stream.json b/.changes/nextrelease/fix-objectuploader-stream.json new file mode 100644 index 0000000000..1925fc8379 --- /dev/null +++ b/.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." + } +] diff --git a/src/S3/ObjectUploader.php b/src/S3/ObjectUploader.php index b73b7b1235..26c3713a27 100644 --- a/src/S3/ObjectUploader.php +++ b/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 diff --git a/tests/S3/ObjectUploaderTest.php b/tests/S3/ObjectUploaderTest.php index d032235d30..ef2e2db0de 100644 --- a/tests/S3/ObjectUploaderTest.php +++ b/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); + } }