[HttpClient] fix management of shorter-than-requested timeouts with AmpHttpClient

Author: nicolas-grekas

Response/AmpResponse.php

@@ -42,6 +42,8 @@ final class AmpResponse implements ResponseInterface
     private $canceller;
     private $onProgress;
 
+    private static $delay;
+
     /**
      * @internal
      */
@@ -171,18 +173,21 @@ private static function perform(ClientState $multi, array &$responses = null): v
      */
     private static function select(ClientState $multi, float $timeout): int
     {
-        $selected = 1;
-        $delay = Loop::delay(1000 * $timeout, static function () use (&$selected) {
-            $selected = 0;
-            Loop::stop();
-        });
-        Loop::run();
+        $start = microtime(true);
+        $remaining = $timeout;
 
-        if ($selected) {
-            Loop::cancel($delay);
-        }
+        while (true) {
+            self::$delay = Loop::delay(1000 * $remaining, [Loop::class, 'stop']);
+            Loop::run();
 
-        return $selected;
+            if (null === self::$delay) {
+                return 1;
+            }
+
+            if (0 >= $remaining = $timeout - microtime(true) + $start) {
+                return 0;
+            }
+        }
     }
 
     private static function generateResponse(Request $request, AmpClientState $multi, string $id, array &$info, array &$headers, CancellationTokenSource $canceller, array &$options, \Closure $onProgress, &$handle, ?LoggerInterface $logger)
@@ -192,7 +197,7 @@ private static function generateResponse(Request $request, AmpClientState $multi
         $request->setInformationalResponseHandler(static function (Response $response) use (&$activity, $id, &$info, &$headers) {
             self::addResponseHeaders($response, $info, $headers);
             $activity[$id][] = new InformationalChunk($response->getStatus(), $response->getHeaders());
-            Loop::defer([Loop::class, 'stop']);
+            self::stopLoop();
         });
 
         try {
@@ -210,7 +215,7 @@ private static function generateResponse(Request $request, AmpClientState $multi
             if ('HEAD' === $response->getRequest()->getMethod() || \in_array($info['http_code'], [204, 304], true)) {
                 $activity[$id][] = null;
                 $activity[$id][] = null;
-                Loop::defer([Loop::class, 'stop']);
+                self::stopLoop();
 
                 return;
             }
@@ -222,7 +227,7 @@ private static function generateResponse(Request $request, AmpClientState $multi
             $body = $response->getBody();
 
             while (true) {
-                Loop::defer([Loop::class, 'stop']);
+                self::stopLoop();
 
                 if (null === $data = yield $body->read()) {
                     break;
@@ -241,7 +246,7 @@ private static function generateResponse(Request $request, AmpClientState $multi
             $info['download_content_length'] = $info['size_download'];
         }
 
-        Loop::defer([Loop::class, 'stop']);
+        self::stopLoop();
     }
 
     private static function followRedirects(Request $originRequest, AmpClientState $multi, array &$info, array &$headers, CancellationTokenSource $canceller, array $options, \Closure $onProgress, &$handle, ?LoggerInterface $logger)
@@ -402,4 +407,14 @@ private static function getPushedResponse(Request $request, AmpClientState $mult
             return $response;
         }
     }
+
+    private static function stopLoop(): void
+    {
+        if (null !== self::$delay) {
+            Loop::cancel(self::$delay);
+            self::$delay = null;
+        }
+
+        Loop::defer([Loop::class, 'stop']);
+    }
 }

Tests/HttpClientTest.php

@@ -12,18 +12,15 @@
 namespace Symfony\Component\HttpClient\Tests;
 
 use PHPUnit\Framework\TestCase;
-use Symfony\Component\HttpClient\AmpHttpClient;
-use Symfony\Component\HttpClient\CurlHttpClient;
 use Symfony\Component\HttpClient\HttpClient;
+use Symfony\Component\HttpClient\NativeHttpClient;
+use Symfony\Contracts\HttpClient\HttpClientInterface;
 
 class HttpClientTest extends TestCase
 {
     public function testCreateClient()
     {
-        if (\extension_loaded('curl') && ('\\' !== \DIRECTORY_SEPARATOR || ini_get('curl.cainfo') || ini_get('openssl.cafile') || ini_get('openssl.capath')) && 0x073d00 <= curl_version()['version_number']) {
-            $this->assertInstanceOf(CurlHttpClient::class, HttpClient::create());
-        } else {
-            $this->assertInstanceOf(AmpHttpClient::class, HttpClient::create());
-        }
+        $this->assertInstanceOf(HttpClientInterface::class, HttpClient::create());
+        $this->assertNotInstanceOf(NativeHttpClient::class, HttpClient::create());
     }
 }