Skip to content
Draft
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-trace-middleware-debug-stream.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[
{
"type": "bugfix",
"category": "TraceMiddleware",
"description": "Defer closing the `@http.debug` resource on streamed response bodies so Guzzle's stream_notification callback can keep writing to it while the body is being read. Fixes \"fprintf(): supplied resource is not a valid stream resource\" warnings when using the S3 stream wrapper with debug enabled."
}
]
62 changes: 62 additions & 0 deletions src/DebugResourceBoundStream.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
<?php
namespace Aws;

use GuzzleHttp\Psr7\StreamDecoratorTrait;
use Psr\Http\Message\StreamInterface;

/**
* PSR-7 stream decorator that owns a secondary "debug" resource and keeps it
* open for the lifetime of the decorated body.
*
* {@see TraceMiddleware} hands the resource it allocated for HTTP-level debug
* output to this decorator whenever the response body is streamed
* (`@http.stream = true`). Guzzle's StreamHandler captures that resource in a
* stream_notification callback that fires on every read of the body, so the
* resource must outlive the middleware chain — closing it eagerly triggered
* "fprintf(): supplied resource is not a valid stream resource"
*
* The resource is closed exactly once when the body is closed or destroyed.
*
* @internal
*/
final class DebugResourceBoundStream implements StreamInterface
{
use StreamDecoratorTrait;

/** @var StreamInterface */
private $stream;

/** @var resource|null */
private $debugResource;

/**
* @param StreamInterface $stream Underlying response body.
* @param resource $debugResource Open php://temp resource still
* referenced by the HTTP adapter's
* stream notification callback.
*/
public function __construct(StreamInterface $stream, $debugResource)
{
$this->stream = $stream;
$this->debugResource = $debugResource;
}

public function close(): void
{
$this->stream->close();
$this->releaseDebugResource();
}

public function __destruct()
{
$this->releaseDebugResource();
}

private function releaseDebugResource(): void
{
if (is_resource($this->debugResource)) {
@fclose($this->debugResource);
}
$this->debugResource = null;
}
}
37 changes: 29 additions & 8 deletions src/TraceMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public function __invoke($step, $name)

return $next($command, $request)->then(
function ($value) use ($step, $name, $command, $start) {
$this->flushHttpDebug($command);
$value = $this->flushHttpDebug($command, $value);
$this->stepOutput($start, [
'step' => $step,
'name' => $name,
Expand Down Expand Up @@ -284,16 +284,37 @@ private function createHttpDebug(CommandInterface $command)
}
}

private function flushHttpDebug(CommandInterface $command)
private function flushHttpDebug(CommandInterface $command, $value = null)
{
if ($res = $command['@http']['debug']) {
if (is_resource($res)) {
rewind($res);
$this->write(stream_get_contents($res));
fclose($res);
$res = $command['@http']['debug'] ?? null;
if (!is_resource($res)) {
if ($res !== null) {
$command['@http']['debug'] = null;
}
$command['@http']['debug'] = null;
return $value;
}

rewind($res);
$this->write(stream_get_contents($res));
$command['@http']['debug'] = null;

// When the response body is streamed, Guzzle's StreamHandler has
// captured $res in a stream_notification callback that fires on every
// read of the body. Closing it here would raise warnings once
// the caller (e.g. S3 stream wrapper) reads the body. Transfer
// ownership to the body so the resource closes with it.
if (
!empty($command['@http']['stream'])
&& $value instanceof ResultInterface
&& ($body = $value['Body'] ?? null) instanceof StreamInterface
&& !$body instanceof DebugResourceBoundStream
) {
$value['Body'] = new DebugResourceBoundStream($body, $res);
return $value;
}

fclose($res);
return $value;
}

private function write($value)
Expand Down
154 changes: 154 additions & 0 deletions tests/DebugResourceBoundStreamTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
<?php
namespace Aws\Test;

use Aws\DebugResourceBoundStream;
use GuzzleHttp\Psr7\Utils;
use PHPUnit\Framework\Attributes\CoversClass;
use Psr\Http\Message\StreamInterface;
use Yoast\PHPUnitPolyfills\TestCases\TestCase;

#[CoversClass(DebugResourceBoundStream::class)]
class DebugResourceBoundStreamTest extends TestCase
{
public function testDelegatesReadsToInnerStream()
{
$inner = Utils::streamFor('hello world');
$debug = fopen('php://temp', 'w+');
$stream = new DebugResourceBoundStream($inner, $debug);

$this->assertSame('hello', $stream->read(5));
$this->assertSame(' world', $stream->read(6));
$this->assertSame('', $stream->read(1)); // past end → triggers EOF
$this->assertTrue($stream->eof());
$this->assertTrue(is_resource($debug), 'reads must not close the debug FD');

$stream->close();
}

public function testToStringDelegates()
{
$inner = Utils::streamFor('payload');
$debug = fopen('php://temp', 'w+');
$stream = new DebugResourceBoundStream($inner, $debug);

$this->assertSame('payload', (string) $stream);

$stream->close();
}

public function testGetSizeAndMetadataDelegate()
{
$inner = Utils::streamFor('abcdef');
$debug = fopen('php://temp', 'w+');
$stream = new DebugResourceBoundStream($inner, $debug);

$this->assertSame(6, $stream->getSize());
$this->assertIsArray($stream->getMetadata());

$stream->close();
}

public function testCloseReleasesDebugResource()
{
$inner = Utils::streamFor('data');
$debug = fopen('php://temp', 'w+');
$stream = new DebugResourceBoundStream($inner, $debug);

$this->assertTrue(is_resource($debug));
$stream->close();
$this->assertFalse(
is_resource($debug),
'close() must fclose the debug FD'
);
}

public function testCloseAlsoClosesInnerStream()
{
$innerHandle = fopen('php://temp', 'w+');
fwrite($innerHandle, 'x');
rewind($innerHandle);
$inner = Utils::streamFor($innerHandle);

$debug = fopen('php://temp', 'w+');
$stream = new DebugResourceBoundStream($inner, $debug);

$stream->close();
$this->assertFalse(is_resource($innerHandle), 'inner stream must also close');
$this->assertFalse(is_resource($debug));
}

public function testDestructReleasesDebugResource()
{
$inner = Utils::streamFor('abc');
$debug = fopen('php://temp', 'w+');
$stream = new DebugResourceBoundStream($inner, $debug);

unset($stream);
$this->assertFalse(
is_resource($debug),
'__destruct must fclose the debug FD'
);
}

public function testDoubleCloseIsSafe()
{
$inner = Utils::streamFor('abc');
$debug = fopen('php://temp', 'w+');
$stream = new DebugResourceBoundStream($inner, $debug);

$stream->close();
// Second close must not warn or throw on the already-closed FD.
$stream->close();

$this->assertFalse(is_resource($debug));
}

public function testCloseFollowedByDestructIsSafe()
{
$inner = Utils::streamFor('abc');
$debug = fopen('php://temp', 'w+');
$stream = new DebugResourceBoundStream($inner, $debug);

$stream->close();
// Destructor should see a null debug resource and do nothing.
unset($stream);

$this->assertFalse(is_resource($debug));
}

public function testAlreadyClosedDebugResourceIsToleratedAtDestruct()
{
$inner = Utils::streamFor('abc');
$debug = fopen('php://temp', 'w+');

$stream = new DebugResourceBoundStream($inner, $debug);

// Simulate a caller who closed the FD out of band — this is the
// scenario that originally caused the regression (#2856).
fclose($debug);
$this->assertFalse(is_resource($debug));

$caught = [];
set_error_handler(static function ($severity, $message) use (&$caught) {
$caught[] = $message;
return true;
});
try {
unset($stream);
} finally {
restore_error_handler();
}

$this->assertSame([], $caught);
}

public function testImplementsStreamInterface()
{
$stream = new DebugResourceBoundStream(
Utils::streamFor(''),
fopen('php://temp', 'w+')
);
$this->assertInstanceOf(StreamInterface::class, $stream);
$stream->close();
}
}
Loading