Skip to content

Commit

Permalink
bug #33982 [HttpClient] improve StreamWrapper (nicolas-grekas)
Browse files Browse the repository at this point in the history 
This PR was merged into the 4.4 branch.

Discussion
----------

[HttpClient] improve StreamWrapper

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | symfony/symfony-docs#12389

Spotted while working on the linked doc PR.

Commits
-------

ea52d1c [HttpClient] improve StreamWrapper
  • Loading branch information
@nicolas-grekas
nicolas-grekas committed Oct 16, 2019
2 parents cbc4efc + ea52d1c commit c4b39b7
Show file tree
Hide file tree
Showing 7 changed files with 128 additions and 18 deletions.
24 changes: 22 additions & 2 deletions src/Symfony/Component/HttpClient/Response/CurlResponse.php
View file
Expand Up @@ -227,6 +227,13 @@ public function __destruct()
*/
private function close(): void
{
if (self::$performing) {
$this->multi->handlesActivity[$this->id][] = null;
$this->multi->handlesActivity[$this->id][] = new TransportException('Response has been canceled.');

return;
}

unset($this->multi->openHandles[$this->id], $this->multi->handlesActivity[$this->id]);
curl_multi_remove_handle($this->multi->handle, $this->handle);
curl_setopt_array($this->handle, [
Expand Down Expand Up @@ -264,6 +271,12 @@ private static function schedule(self $response, array &$runningResponses): void
private static function perform(CurlClientState $multi, array &$responses = null): void
{
if (self::$performing) {
if ($responses) {
$response = current($responses);
$multi->handlesActivity[(int) $response->handle][] = null;
$multi->handlesActivity[(int) $response->handle][] = new TransportException(sprintf('Userland callback cannot use the client nor the response while processing "%s".', curl_getinfo($response->handle, CURLINFO_EFFECTIVE_URL)));
}

return;
}

Expand Down Expand Up @@ -370,8 +383,15 @@ private static function parseHeaderLine($ch, string $data, array &$info, array &

curl_setopt($ch, CURLOPT_PRIVATE, 'content');

if (!$content && $options['buffer'] instanceof \Closure && $options['buffer']($headers)) {
$content = fopen('php://temp', 'w+');
try {
if (!$content && $options['buffer'] instanceof \Closure && $options['buffer']($headers)) {
$content = fopen('php://temp', 'w+');
}
} catch (\Throwable $e) {
$multi->handlesActivity[$id][] = null;
$multi->handlesActivity[$id][] = $e;

return 0;
}
} elseif (null !== $info['redirect_url'] && $logger) {
$logger->info(sprintf('Redirecting: "%s %s"', $info['http_code'], $info['redirect_url']));
Expand Down
8 changes: 5 additions & 3 deletions src/Symfony/Component/HttpClient/Response/MockResponse.php
View file
Expand Up @@ -105,9 +105,7 @@ public static function fromRequest(string $method, string $url, array $options,
$response->requestOptions = $options;
$response->id = ++self::$idSequence;

if (($options['buffer'] ?? null) instanceof \Closure) {
$response->content = $options['buffer']($mock->getHeaders(false)) ? fopen('php://temp', 'w+') : null;
} else {
if (!($options['buffer'] ?? null) instanceof \Closure) {
$response->content = true === ($options['buffer'] ?? true) ? fopen('php://temp', 'w+') : null;
}
$response->initializer = static function (self $response) {
Expand Down Expand Up @@ -184,6 +182,10 @@ protected static function perform(ClientState $multi, array &$responses): void
$response->headers = $chunk[1]->getHeaders(false);
self::readResponse($response, $chunk[0], $chunk[1], $offset);
$multi->handlesActivity[$id][] = new FirstChunk();

if (($response->requestOptions['buffer'] ?? null) instanceof \Closure) {
$response->content = $response->requestOptions['buffer']($response->headers) ? fopen('php://temp', 'w+') : null;
}
} catch (\Throwable $e) {
$multi->handlesActivity[$id][] = null;
$multi->handlesActivity[$id][] = $e;
Expand Down
21 changes: 17 additions & 4 deletions src/Symfony/Component/HttpClient/Response/NativeResponse.php
View file
Expand Up @@ -160,10 +160,6 @@ private function open(): void
stream_set_blocking($h, false);
$this->context = $this->resolveRedirect = null;

if (null !== $this->shouldBuffer && null === $this->content && ($this->shouldBuffer)($this->headers)) {
$this->content = fopen('php://temp', 'w+');
}

if (isset($context['ssl']['peer_certificate_chain'])) {
$this->info['peer_certificate_chain'] = $context['ssl']['peer_certificate_chain'];
}
Expand All @@ -182,6 +178,23 @@ private function open(): void
$this->inflate = null;
}

try {
if (null !== $this->shouldBuffer && null === $this->content && ($this->shouldBuffer)($this->headers)) {
$this->content = fopen('php://temp', 'w+');
}

if (!$this->buffer) {
throw new TransportException('Response has been canceled.');
}
} catch (\Throwable $e) {
$this->close();
$this->multi->handlesActivity[$this->id] = [new FirstChunk()];
$this->multi->handlesActivity[$this->id][] = null;
$this->multi->handlesActivity[$this->id][] = $e;

return;
}

$this->multi->openHandles[$this->id] = [$h, $this->buffer, $this->inflate, $this->content, $this->onProgress, &$this->remaining, &$this->info];
$this->multi->handlesActivity[$this->id] = [new FirstChunk()];
}
Expand Down
10 changes: 6 additions & 4 deletions src/Symfony/Component/HttpClient/Response/ResponseTrait.php
View file
Expand Up @@ -57,7 +57,7 @@ trait ResponseTrait
/** @var resource */
private $handle;
private $id;
private $timeout;
private $timeout = 0;
private $finalInfo;
private $offset = 0;
private $jsonData;
Expand Down Expand Up @@ -185,7 +185,7 @@ public function cancel(): void
/**
* Casts the response to a PHP stream resource.
*
* @return resource|null
* @return resource
*
* @throws TransportExceptionInterface When a network error occurs
* @throws RedirectionExceptionInterface On a 3xx when $throw is true and the "max_redirects" option has been reached
Expand All @@ -194,8 +194,10 @@ public function cancel(): void
*/
public function toStream(bool $throw = true)
{
// Ensure headers arrived
$this->getHeaders($throw);
if ($throw) {
// Ensure headers arrived
$this->getHeaders($throw);
}

return StreamWrapper::createResource($this, null, $this->content, $this->handle && 'stream' === get_resource_type($this->handle) ? $this->handle : null);
}
Expand Down
32 changes: 29 additions & 3 deletions src/Symfony/Component/HttpClient/Response/StreamWrapper.php
View file
Expand Up @@ -73,6 +73,11 @@ public static function createResource(ResponseInterface $response, HttpClientInt
}
}

public function getResponse(): ResponseInterface
{
return $this->response;
}

public function stream_open(string $path, string $mode, int $options): bool
{
if ('r' !== $mode) {
Expand Down Expand Up @@ -107,7 +112,9 @@ public function stream_read(int $count)
// Empty the internal activity list
foreach ($this->client->stream([$this->response], 0) as $chunk) {
try {
$chunk->isTimeout();
if (!$chunk->isTimeout() && $chunk->isFirst()) {
$this->response->getStatusCode(); // ignore 3/4/5xx
}
} catch (ExceptionInterface $e) {
trigger_error($e->getMessage(), E_USER_WARNING);

Expand Down Expand Up @@ -146,6 +153,10 @@ public function stream_read(int $count)
$this->eof = !$chunk->isTimeout();
$this->eof = $chunk->isLast();

if ($chunk->isFirst()) {
$this->response->getStatusCode(); // ignore 3/4/5xx
}

if ('' !== $data = $chunk->getContent()) {
if (\strlen($data) > $count) {
if (null === $this->content) {
Expand Down Expand Up @@ -192,6 +203,10 @@ public function stream_seek(int $offset, int $whence = SEEK_SET): bool
if (SEEK_END === $whence || $size < $offset) {
foreach ($this->client->stream([$this->response]) as $chunk) {
try {
if ($chunk->isFirst()) {
$this->response->getStatusCode(); // ignore 3/4/5xx
}

// Chunks are buffered in $this->content already
$size += \strlen($chunk->getContent());

Expand Down Expand Up @@ -231,6 +246,13 @@ public function stream_cast(int $castAs)

public function stream_stat(): array
{
try {
$headers = $this->response->getHeaders(false);
} catch (ExceptionInterface $e) {
trigger_error($e->getMessage(), E_USER_WARNING);
$headers = [];
}

return [
'dev' => 0,
'ino' => 0,
Expand All @@ -239,12 +261,16 @@ public function stream_stat(): array
'uid' => 0,
'gid' => 0,
'rdev' => 0,
'size' => (int) ($this->response->getHeaders(false)['content-length'][0] ?? 0),
'size' => (int) ($headers['content-length'][0] ?? 0),
'atime' => 0,
'mtime' => strtotime($this->response->getHeaders(false)['last-modified'][0] ?? '') ?: 0,
'mtime' => strtotime($headers['last-modified'][0] ?? '') ?: 0,
'ctime' => 0,
'blksize' => 0,
'blocks' => 0,
];
}

private function __construct()
{
}
}
49 changes: 47 additions & 2 deletions src/Symfony/Component/HttpClient/Tests/HttpClientTestCase.php
View file
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Component\HttpClient\Tests;

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

Expand Down Expand Up @@ -52,9 +53,7 @@ public function testAcceptHeader()
public function testToStream()
{
$client = $this->getHttpClient(__FUNCTION__);

$response = $client->request('GET', 'http://localhost:8057');

$stream = $response->toStream();

$this->assertSame("{\n \"SER", fread($stream, 10));
Expand All @@ -67,6 +66,22 @@ public function testToStream()
$this->assertTrue(feof($stream));
}

public function testToStream404()
{
$client = $this->getHttpClient(__FUNCTION__);
$response = $client->request('GET', 'http://localhost:8057/404');
$stream = $response->toStream(false);

$this->assertSame("{\n \"SER", fread($stream, 10));
$this->assertSame('VER_PROTOCOL', fread($stream, 12));
$this->assertSame($response, stream_get_meta_data($stream)['wrapper_data']->getResponse());
$this->assertSame(404, $response->getStatusCode());

$this->expectException(ClientException::class);
$response = $client->request('GET', 'http://localhost:8057/404');
$stream = $response->toStream();
}

public function testConditionalBuffering()
{
$client = $this->getHttpClient(__FUNCTION__);
Expand All @@ -83,4 +98,34 @@ public function testConditionalBuffering()
$this->expectExceptionMessage('Cannot get the content of the response twice: buffering is disabled.');
$response->getContent();
}

public function testReentrantBufferCallback()
{
$client = $this->getHttpClient(__FUNCTION__);

$response = $client->request('GET', 'http://localhost:8057', ['buffer' => function () use (&$response) {
$response->cancel();
}]);

$this->assertSame(200, $response->getStatusCode());

$this->expectException(TransportException::class);
$this->expectExceptionMessage('Response has been canceled.');
$response->getContent();
}

public function testThrowingBufferCallback()
{
$client = $this->getHttpClient(__FUNCTION__);

$response = $client->request('GET', 'http://localhost:8057', ['buffer' => function () {
throw new \Exception('Boo');
}]);

$this->assertSame(200, $response->getStatusCode());

$this->expectException(TransportException::class);
$this->expectExceptionMessage('Boo');
$response->getContent();
}
}
2 changes: 2 additions & 0 deletions src/Symfony/Component/HttpClient/Tests/MockHttpClientTest.php
View file
Expand Up @@ -103,6 +103,8 @@ protected function getHttpClient(string $testCase): HttpClientInterface
case 'testBadRequestBody':
case 'testOnProgressCancel':
case 'testOnProgressError':
case 'testReentrantBufferCallback':
case 'testThrowingBufferCallback':
$responses[] = new MockResponse($body, ['response_headers' => $headers]);
break;

Expand Down

0 comments on commit c4b39b7

Please sign in to comment.