Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit d50fbf6

Browse filesBrowse files
bug #47879 [HttpClient] Fix buffering after calling AsyncContext::passthru() (nicolas-grekas, lubo13)
This PR was merged into the 5.4 branch. Discussion ---------- [HttpClient] Fix buffering after calling AsyncContext::passthru() | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Hi, guys, I think there is an issue with the RetryableHttpClient, let me explain to you what I've seen. 1. Request is executed into AsyncResponse::__construct 2. The initializer Closure executes AsyncResponse::passthru, there the AsyncContext and a new stream are created 3. The next step is yielding from passthruStream and there the stream is checked - ```$r->stream->valid()```. Here the stream is not a valid iterator, because the last retry attempt has already been made and the generator is closed. 4. This leads the response body stream to be unseekable because the ```$r->openBuffer()``` is not called. The content is left with a value ```null``` and when ```AsyncResponse->toStream()``` is called from ```Psr18Client::110``` the content with value null is bound to the ```content of StreamWrapper```. 5. After that this newly generated resource is passed to the ```Psr17Factory->createStreamFromResource()``` and the resulted (unseekable body) stream is passed to the ```$response->withBody($body)``` onto ```Psr18Client::121``` 6. The result is that the Response body stream is not seekable and when I execute two times ```(string) $response->getBody()``` after the first attempt the body is an empty string. Let me know what you think. Thank you. Commits ------- b559f80 [HttpClient] Add test case for seeking into the content of RetryableHttpClient responses c1da0eb [HttpClient] Fix buffering after calling AsyncContext::passthru()
2 parents af6a777 + b559f80 commit d50fbf6
Copy full SHA for d50fbf6

File tree

Expand file treeCollapse file tree

2 files changed

+25
-1
lines changed
Filter options
Expand file treeCollapse file tree

2 files changed

+25
-1
lines changed

‎src/Symfony/Component/HttpClient/Response/AsyncContext.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpClient/Response/AsyncContext.php
+7-1Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,9 +181,15 @@ public function replaceResponse(ResponseInterface $response): ResponseInterface
181181

182182
/**
183183
* Replaces or removes the chunk filter iterator.
184+
*
185+
* @param ?callable(ChunkInterface, self): ?\Iterator $passthru
184186
*/
185187
public function passthru(callable $passthru = null): void
186188
{
187-
$this->passthru = $passthru;
189+
$this->passthru = $passthru ?? static function ($chunk, $context) {
190+
$context->passthru = null;
191+
192+
yield $chunk;
193+
};
188194
}
189195
}

‎src/Symfony/Component/HttpClient/Tests/RetryableHttpClientTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpClient/Tests/RetryableHttpClientTest.php
+18Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,4 +225,22 @@ public function log($level, $message, array $context = []): void
225225
$this->assertNotNull($delay);
226226
$this->assertSame((int) ($retryAfter * 1000), $delay);
227227
}
228+
229+
public function testRetryOnErrorAssertContent()
230+
{
231+
$client = new RetryableHttpClient(
232+
new MockHttpClient([
233+
new MockResponse('', ['http_code' => 500]),
234+
new MockResponse('Test out content', ['http_code' => 200]),
235+
]),
236+
new GenericRetryStrategy([500], 0),
237+
1
238+
);
239+
240+
$response = $client->request('GET', 'http://example.com/foo-bar');
241+
242+
self::assertSame(200, $response->getStatusCode());
243+
self::assertSame('Test out content', $response->getContent());
244+
self::assertSame('Test out content', $response->getContent(), 'Content should be buffered');
245+
}
228246
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.