Merge branch '5.1' into 5.x

* 5.1:
  [HttpClient] fix reading the body after a ClientException
  [HttpClient] fix tests and merge

Author: nicolas-grekas

src/Symfony/Component/HttpClient/Internal/Canary.php

@@ -0,0 +1,40 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\HttpClient\Internal;
+
+/**
+ * @author Nicolas Grekas <[email protected]>
+ *
+ * @internal
+ */
+final class Canary
+{
+    private $canceller;
+
+    public function __construct(\Closure $canceller)
+    {
+        $this->canceller = $canceller;
+    }
+
+    public function cancel()
+    {
+        if (($canceller = $this->canceller) instanceof \Closure) {
+            $this->canceller = null;
+            $canceller();
+        }
+    }
+
+    public function __destruct()
+    {
+        $this->cancel();
+    }
+}

src/Symfony/Component/HttpClient/Response/AmpResponse.php

@@ -30,6 +30,7 @@
 use Symfony\Component\HttpClient\Internal\AmpBody;
 use Symfony\Component\HttpClient\Internal\AmpCanary;
 use Symfony\Component\HttpClient\Internal\AmpClientState;
+use Symfony\Component\HttpClient\Internal\Canary;
 use Symfony\Component\HttpClient\Internal\ClientState;
 use Symfony\Contracts\HttpClient\ResponseInterface;
 
@@ -127,6 +128,11 @@ public function __construct(AmpClientState $multi, Request $request, array $opti
 
         $multi->openHandles[$id] = $id;
         ++$multi->responseCount;
+
+        $this->canary = new Canary(static function () use ($canceller, $multi, $id) {
+            $canceller->cancel();
+            unset($multi->openHandles[$id], $multi->handlesActivity[$id]);
+        });
     }
 
     /**
@@ -142,29 +148,14 @@ public function __destruct()
         try {
             $this->doDestruct();
         } finally {
-            $multi = clone $this->multi;
-
-            $this->close();
-
             // Clear the DNS cache when all requests completed
             if (0 >= --$this->multi->responseCount) {
                 $this->multi->responseCount = 0;
                 $this->multi->dnsCache = [];
             }
-
-            $this->multi = $multi;
         }
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    private function close(): void
-    {
-        $this->canceller->cancel();
-        unset($this->multi->openHandles[$this->id], $this->multi->handlesActivity[$this->id]);
-    }
-
     /**
      * {@inheritdoc}
      */

src/Symfony/Component/HttpClient/Response/CurlResponse.php

@@ -15,6 +15,7 @@
 use Symfony\Component\HttpClient\Chunk\FirstChunk;
 use Symfony\Component\HttpClient\Chunk\InformationalChunk;
 use Symfony\Component\HttpClient\Exception\TransportException;
+use Symfony\Component\HttpClient\Internal\Canary;
 use Symfony\Component\HttpClient\Internal\ClientState;
 use Symfony\Component\HttpClient\Internal\CurlClientState;
 use Symfony\Contracts\HttpClient\ResponseInterface;
@@ -171,6 +172,31 @@ public function __construct(CurlClientState $multi, $ch, array $options = null,
         // Schedule the request in a non-blocking way
         $multi->openHandles[$id] = [$ch, $options];
         curl_multi_add_handle($multi->handle, $ch);
+
+        $this->canary = new Canary(static function () use ($ch, $multi, $id) {
+            unset($multi->pauseExpiries[$id], $multi->openHandles[$id], $multi->handlesActivity[$id]);
+            curl_setopt($ch, \CURLOPT_PRIVATE, '_0');
+
+            if (self::$performing) {
+                return;
+            }
+
+            curl_multi_remove_handle($multi->handle, $ch);
+            curl_setopt_array($ch, [
+                \CURLOPT_NOPROGRESS => true,
+                \CURLOPT_PROGRESSFUNCTION => null,
+                \CURLOPT_HEADERFUNCTION => null,
+                \CURLOPT_WRITEFUNCTION => null,
+                \CURLOPT_READFUNCTION => null,
+                \CURLOPT_INFILE => null,
+            ]);
+
+            if (!$multi->openHandles) {
+                // Schedule DNS cache eviction for the next request
+                $multi->dnsCache->evictions = $multi->dnsCache->evictions ?: $multi->dnsCache->removals;
+                $multi->dnsCache->removals = $multi->dnsCache->hostnames = [];
+            }
+        });
     }
 
     /**
@@ -221,48 +247,11 @@ public function getContent(bool $throw = true): string
 
     public function __destruct()
     {
-        try {
-            if (null === $this->timeout) {
-                return; // Unused pushed response
-            }
-
-            $this->doDestruct();
-        } finally {
-            $multi = clone $this->multi;
-
-            $this->close();
-
-            if (!$this->multi->openHandles) {
-                // Schedule DNS cache eviction for the next request
-                $this->multi->dnsCache->evictions = $this->multi->dnsCache->evictions ?: $this->multi->dnsCache->removals;
-                $this->multi->dnsCache->removals = $this->multi->dnsCache->hostnames = [];
-            }
-
-            $this->multi = $multi;
-        }
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    private function close(): void
-    {
-        unset($this->multi->pauseExpiries[$this->id], $this->multi->openHandles[$this->id], $this->multi->handlesActivity[$this->id]);
-        curl_setopt($this->handle, \CURLOPT_PRIVATE, '_0');
-
-        if (self::$performing) {
-            return;
+        if (null === $this->timeout) {
+            return; // Unused pushed response
         }
 
-        curl_multi_remove_handle($this->multi->handle, $this->handle);
-        curl_setopt_array($this->handle, [
-            \CURLOPT_NOPROGRESS => true,
-            \CURLOPT_PROGRESSFUNCTION => null,
-            \CURLOPT_HEADERFUNCTION => null,
-            \CURLOPT_WRITEFUNCTION => null,
-            \CURLOPT_READFUNCTION => null,
-            \CURLOPT_INFILE => null,
-        ]);
+        $this->doDestruct();
     }
 
     /**

src/Symfony/Component/HttpClient/Response/NativeResponse.php

@@ -14,6 +14,7 @@
 use Psr\Log\LoggerInterface;
 use Symfony\Component\HttpClient\Chunk\FirstChunk;
 use Symfony\Component\HttpClient\Exception\TransportException;
+use Symfony\Component\HttpClient\Internal\Canary;
 use Symfony\Component\HttpClient\Internal\ClientState;
 use Symfony\Component\HttpClient\Internal\NativeClientState;
 use Symfony\Contracts\HttpClient\ResponseInterface;
@@ -45,7 +46,7 @@ final class NativeResponse implements ResponseInterface, StreamableInterface
     public function __construct(NativeClientState $multi, $context, string $url, array $options, array &$info, callable $resolver, ?callable $onProgress, ?LoggerInterface $logger)
     {
         $this->multi = $multi;
-        $this->id = (int) $context;
+        $this->id = $id = (int) $context;
         $this->context = $context;
         $this->url = $url;
         $this->logger = $logger;
@@ -70,6 +71,13 @@ public function __construct(NativeClientState $multi, $context, string $url, arr
         $info['pause_handler'] = static function (float $duration) use (&$pauseExpiry) {
             $pauseExpiry = 0 < $duration ? microtime(true) + $duration : 0;
         };
+
+        $this->canary = new Canary(static function () use ($multi, $id) {
+            if (null !== ($host = $multi->openHandles[$id][6] ?? null) && 0 >= --$multi->hosts[$host]) {
+                unset($multi->hosts[$host]);
+            }
+            unset($multi->openHandles[$id], $multi->handlesActivity[$id]);
+        });
     }
 
     /**
@@ -95,17 +103,11 @@ public function __destruct()
         try {
             $this->doDestruct();
         } finally {
-            $multi = clone $this->multi;
-
-            $this->close();
-
             // Clear the DNS cache when all requests completed
             if (0 >= --$this->multi->responseCount) {
                 $this->multi->responseCount = 0;
                 $this->multi->dnsCache = [];
             }
-
-            $this->multi = $multi;
         }
     }
 
@@ -203,11 +205,8 @@ private function open(): void
      */
     private function close(): void
     {
-        if (null !== ($host = $this->multi->openHandles[$this->id][6] ?? null) && 0 >= --$this->multi->hosts[$host]) {
-            unset($this->multi->hosts[$host]);
-        }
-        unset($this->multi->openHandles[$this->id], $this->multi->handlesActivity[$this->id]);
-        $this->handle = $this->buffer = $this->onProgress = null;
+        $this->canary->cancel();
+        $this->handle = $this->buffer = $this->inflate = $this->onProgress = null;
     }
 
     /**

src/Symfony/Component/HttpClient/Response/TransportResponseTrait.php

@@ -41,6 +41,7 @@ trait TransportResponseTrait
     private $timeout = 0;
     private $inflate;
     private $finalInfo;
+    private $canary;
     private $logger;
 
     /**
@@ -81,6 +82,15 @@ public function cancel(): void
         $this->close();
     }
 
+    /**
+     * Closes the response and all its network handles.
+     */
+    protected function close(): void
+    {
+        $this->canary->cancel();
+        $this->inflate = null;
+    }
+
     /**
      * Adds pending responses to the activity list.
      */

src/Symfony/Component/HttpClient/Tests/HttpClientTestCase.php

@@ -319,6 +319,7 @@ public function testHandleIsRemovedOnException()
             // The response content-type mustn't be json as that calls getContent
             // @see src/Symfony/Component/HttpClient/Exception/HttpExceptionTrait.php:58
             $this->assertStringNotContainsString('json', $e->getResponse()->getHeaders(false)['content-type'][0] ?? '');
+            unset($e);
 
             $r = new \ReflectionProperty($client, 'multi');
             $r->setAccessible(true);

src/Symfony/Contracts/HttpClient/Test/Fixtures/web/index.php

@@ -66,6 +66,9 @@
     case '/404-gzipped':
         header('Content-Type: text/plain', true, 404);
         ob_start('ob_gzhandler');
+        @ob_flush();
+        flush();
+        usleep(300000);
         echo 'some text';
         exit;