From 6d1657b720fd12363286cf4a857fd8052bc36486 Mon Sep 17 00:00:00 2001
From: Nicolas Grekas <nicolas.grekas@gmail.com>
Date: Tue, 11 Feb 2020 14:51:01 +0100
Subject: [PATCH] [HttpClient] fix getting response content after its
 destructor throwed an HttpExceptionInterface

---
 .../HttpClient/Response/CurlResponse.php      | 23 +++++++++----------
 .../HttpClient/Response/NativeResponse.php    | 10 ++++++--
 .../HttpClient/Response/ResponseTrait.php     |  2 ++
 .../HttpClient/Test/HttpClientTestCase.php    | 14 +++++++++++
 4 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/src/Symfony/Component/HttpClient/Response/CurlResponse.php b/src/Symfony/Component/HttpClient/Response/CurlResponse.php
index c497cfc4fdd1..6a1be2e08a5a 100644
--- a/src/Symfony/Component/HttpClient/Response/CurlResponse.php
+++ b/src/Symfony/Component/HttpClient/Response/CurlResponse.php
@@ -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;
 
 /**
@@ -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
@@ -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) {
@@ -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
         }
 
@@ -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;
diff --git a/src/Symfony/Component/HttpClient/Response/NativeResponse.php b/src/Symfony/Component/HttpClient/Response/NativeResponse.php
index 7abe18abab2e..f7f458a47cb0 100644
--- a/src/Symfony/Component/HttpClient/Response/NativeResponse.php
+++ b/src/Symfony/Component/HttpClient/Response/NativeResponse.php
@@ -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;
 
 /**
@@ -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
diff --git a/src/Symfony/Component/HttpClient/Response/ResponseTrait.php b/src/Symfony/Component/HttpClient/Response/ResponseTrait.php
index 8793a45184c6..000da5344dcf 100644
--- a/src/Symfony/Component/HttpClient/Response/ResponseTrait.php
+++ b/src/Symfony/Component/HttpClient/Response/ResponseTrait.php
@@ -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();
diff --git a/src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php b/src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php
index aad712e3e8dc..851997c087bd 100644
--- a/src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php
+++ b/src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php
@@ -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__);