Skip to content

Commit

Permalink
bug #35674 [HttpClient] fix getting response content after its destru…
Browse files Browse the repository at this point in the history
…ctor throwed an HttpExceptionInterface (nicolas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[HttpClient] fix getting response content after its destructor throwed an HttpExceptionInterface

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Spotted by @B-Galati in #35659 (comment)

Commits
-------

6d1657b [HttpClient] fix getting response content after its destructor throwed an HttpExceptionInterface
  • Loading branch information
fabpot committed Feb 11, 2020
2 parents c895a40 + 6d1657b commit 5cf876f
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 14 deletions.
23 changes: 11 additions & 12 deletions src/Symfony/Component/HttpClient/Response/CurlResponse.php
Expand Up @@ -16,6 +16,7 @@
use Symfony\Component\HttpClient\Chunk\InformationalChunk;
use Symfony\Component\HttpClient\Exception\TransportException;
use Symfony\Component\HttpClient\Internal\CurlClientState;
use Symfony\Contracts\HttpClient\Exception\HttpExceptionInterface;
use Symfony\Contracts\HttpClient\ResponseInterface;

/**
Expand Down Expand Up @@ -113,7 +114,7 @@ public function __construct(CurlClientState $multi, $ch, array $options = null,
$this->initializer = static function (self $response) {
$waitFor = curl_getinfo($ch = $response->handle, CURLINFO_PRIVATE);

return 'H' === $waitFor[0] || 'D' === $waitFor[0];
return 'H' === $waitFor[0];
};

// Schedule the request in a non-blocking way
Expand Down Expand Up @@ -174,17 +175,15 @@ public function __destruct()
return; // Unused pushed response
}

$waitFor = curl_getinfo($this->handle, CURLINFO_PRIVATE);

if ('C' === $waitFor[0] || '_' === $waitFor[0]) {
$this->close();
} elseif ('H' === $waitFor[0]) {
$waitFor[0] = 'D'; // D = destruct
curl_setopt($this->handle, CURLOPT_PRIVATE, $waitFor);
}

$e = null;
$this->doDestruct();
} catch (HttpExceptionInterface $e) {
throw $e;
} finally {
if (null !== $e) {
throw $e;
}

$this->close();

if (!$this->multi->openHandles) {
Expand Down Expand Up @@ -304,7 +303,7 @@ private static function parseHeaderLine($ch, string $data, array &$info, array &
{
$waitFor = @curl_getinfo($ch, CURLINFO_PRIVATE) ?: '_0';

if ('H' !== $waitFor[0] && 'D' !== $waitFor[0]) {
if ('H' !== $waitFor[0]) {
return \strlen($data); // Ignore HTTP trailers
}

Expand Down Expand Up @@ -370,7 +369,7 @@ private static function parseHeaderLine($ch, string $data, array &$info, array &
// Headers and redirects completed, time to get the response's content
$multi->handlesActivity[$id][] = new FirstChunk();

if ('D' === $waitFor[0] || 'HEAD' === $info['http_method'] || \in_array($statusCode, [204, 304], true)) {
if ('HEAD' === $info['http_method'] || \in_array($statusCode, [204, 304], true)) {
$waitFor = '_0'; // no content expected
$multi->handlesActivity[$id][] = null;
$multi->handlesActivity[$id][] = null;
Expand Down
10 changes: 8 additions & 2 deletions src/Symfony/Component/HttpClient/Response/NativeResponse.php
Expand Up @@ -15,6 +15,7 @@
use Symfony\Component\HttpClient\Chunk\FirstChunk;
use Symfony\Component\HttpClient\Exception\TransportException;
use Symfony\Component\HttpClient\Internal\NativeClientState;
use Symfony\Contracts\HttpClient\Exception\HttpExceptionInterface;
use Symfony\Contracts\HttpClient\ResponseInterface;

/**
Expand Down Expand Up @@ -84,11 +85,16 @@ public function getInfo(string $type = null)

public function __destruct()
{
$this->shouldBuffer = null;

try {
$e = null;
$this->doDestruct();
} catch (HttpExceptionInterface $e) {
throw $e;
} finally {
if (null !== $e) {
throw $e;
}

$this->close();

// Clear the DNS cache when all requests completed
Expand Down
2 changes: 2 additions & 0 deletions src/Symfony/Component/HttpClient/Response/ResponseTrait.php
Expand Up @@ -294,6 +294,8 @@ private function checkStatusCode()
*/
private function doDestruct()
{
$this->shouldBuffer = true;

if ($this->initializer && null === $this->info['error']) {
self::initialize($this);
$this->checkStatusCode();
Expand Down
14 changes: 14 additions & 0 deletions src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php
Expand Up @@ -782,6 +782,20 @@ public function testDestruct()
$this->assertLessThan(4, $duration);
}

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

$start = microtime(true);

try {
$client->request('GET', 'http://localhost:8057/404');
$this->fail(ClientExceptionInterface::class.' expected');
} catch (ClientExceptionInterface $e) {
$this->assertSame('GET', $e->getResponse()->toArray(false)['REQUEST_METHOD']);
}
}

public function testProxy()
{
$client = $this->getHttpClient(__FUNCTION__);
Expand Down

0 comments on commit 5cf876f

Please sign in to comment.