Skip to content

Commit

Permalink
bug #35573 [HttpClient] make response stream functionality consistent…
Browse files Browse the repository at this point in the history
… (kbond)

This PR was squashed before being merged into the 5.1-dev branch.

Discussion
----------

[HttpClient] make response stream functionality consistent

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | n/a
| License       | MIT
| Doc PR        | n/a

There are three ways of creating a stream from a response:

1. Calling `$response->toStream()` (if the response supports this)
2. Calling `StreamWrapper::createResource($response)`
3. Calling `StreamWrapper::createResource($response, $httpClient)` (note the second argument)

Currently, the 3rd method creates a stream that is not rewindable (the other two are). The first commit adds tests showing the inconsistencies (1 test fails). The second commit is a fix to make the 3 ways consistent.

See https://twitter.com/nicolasgrekas/status/1224047079422599168 for reference.

Commits
-------

64f9111 [HttpClient] make response stream functionality consistent
  • Loading branch information
nicolas-grekas committed Feb 3, 2020
2 parents 12ca646 + 64f9111 commit ff4892b
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 1 deletion.
Expand Up @@ -49,7 +49,7 @@ class StreamWrapper
*/
public static function createResource(ResponseInterface $response, HttpClientInterface $client = null)
{
if (null === $client && \is_callable([$response, 'toStream']) && isset(class_uses($response)[ResponseTrait::class])) {
if (\is_callable([$response, 'toStream']) && isset(class_uses($response)[ResponseTrait::class])) {
$stack = debug_backtrace(DEBUG_BACKTRACE_PROVIDE_OBJECT | DEBUG_BACKTRACE_IGNORE_ARGS, 2);

if ($response !== ($stack[1]['object'] ?? null)) {
Expand Down
37 changes: 37 additions & 0 deletions src/Symfony/Component/HttpClient/Tests/HttpClientTestCase.php
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\HttpClient\Tests;

use Symfony\Component\HttpClient\Exception\ClientException;
use Symfony\Component\HttpClient\Response\StreamWrapper;
use Symfony\Contracts\HttpClient\Test\HttpClientTestCase as BaseHttpClientTestCase;

abstract class HttpClientTestCase extends BaseHttpClientTestCase
Expand Down Expand Up @@ -91,4 +92,40 @@ public function testNonBlockingStream()
$this->assertSame('', fread($stream, 8192));
$this->assertTrue(feof($stream));
}

public function testResponseStreamRewind()
{
$client = $this->getHttpClient(__FUNCTION__);
$response = $client->request('GET', 'http://localhost:8057/103');

$stream = $response->toStream();

$this->assertSame('Here the body', stream_get_contents($stream));
rewind($stream);
$this->assertSame('Here the body', stream_get_contents($stream));
}

public function testStreamWrapperStreamRewind()
{
$client = $this->getHttpClient(__FUNCTION__);
$response = $client->request('GET', 'http://localhost:8057/103');

$stream = StreamWrapper::createResource($response);

$this->assertSame('Here the body', stream_get_contents($stream));
rewind($stream);
$this->assertSame('Here the body', stream_get_contents($stream));
}

public function testStreamWrapperWithClientStreamRewind()
{
$client = $this->getHttpClient(__FUNCTION__);
$response = $client->request('GET', 'http://localhost:8057/103');

$stream = StreamWrapper::createResource($response, $client);

$this->assertSame('Here the body', stream_get_contents($stream));
rewind($stream);
$this->assertSame('Here the body', stream_get_contents($stream));
}
}

0 comments on commit ff4892b

Please sign in to comment.