Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changes/nextrelease/fix-objectuploader-stream.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[
{
"type": "bugfix",
"category": "S3",
"description": "Prevents resources provided to `ObjectUploader` from being closed by Guzzle."
}
]
30 changes: 29 additions & 1 deletion src/S3/ObjectUploader.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
use GuzzleHttp\Promise\PromiseInterface;
use GuzzleHttp\Promise\PromisorInterface;
use GuzzleHttp\Psr7;
use GuzzleHttp\Psr7\FnStream;
use Psr\Http\Message\StreamInterface;

/**
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
66 changes: 66 additions & 0 deletions tests/S3/ObjectUploaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Loading