bug #35014 [HttpClient] make pushed responses retry-able (nicolas-gre…

…kas) This PR was merged into the 4.3 branch. Discussion ---------- [HttpClient] make pushed responses retry-able | Q | A | ------------- | --- | Branch? | 4.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | https://github.com/orgs/symfony/projects/1#card-30499375 | License | MIT | Doc PR | - This moves the PUSH matching logic down so that the curl handle of pushed responses can be properly configured. This should make pushed requests retry-able when they fail just after the push-promise frame. Commits ------- c2864f6 [HttpClient] make pushed responses retry-able
symfony · Dec 18, 2019 · bbf7ed1 · bbf7ed1
2 parents 026d57e + c2864f6
commit bbf7ed1
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 21 deletions.
diff --git a/src/Symfony/Component/HttpClient/CurlHttpClient.php b/src/Symfony/Component/HttpClient/CurlHttpClient.php
@@ -116,23 +116,6 @@ public function request(string $method, string $url, array $options = []): Respo
             $options['normalized_headers']['user-agent'][] = $options['headers'][] = 'User-Agent: Symfony HttpClient/Curl';
         }
 
-        if ($pushedResponse = $this->multi->pushedResponses[$url] ?? null) {
-            unset($this->multi->pushedResponses[$url]);
-
-            if (self::acceptPushForRequest($method, $options, $pushedResponse)) {
-                $this->logger && $this->logger->debug(sprintf('Accepting pushed response: "%s %s"', $method, $url));
-
-                // Reinitialize the pushed response with request's options
-                $pushedResponse->response->__construct($this->multi, $url, $options, $this->logger);
-
-                return $pushedResponse->response;
-            }
-
-            $this->logger && $this->logger->debug(sprintf('Rejecting pushed response: "%s".', $url));
-        }
-
-        $this->logger && $this->logger->info(sprintf('Request: "%s %s"', $method, $url));
-
         $curlopts = [
             CURLOPT_URL => $url,
             CURLOPT_TCP_NODELAY => true,
@@ -267,7 +250,26 @@ public function request(string $method, string $url, array $options = []): Respo
             $curlopts[file_exists($options['bindto']) ? CURLOPT_UNIX_SOCKET_PATH : CURLOPT_INTERFACE] = $options['bindto'];
         }
 
-        $ch = curl_init();
+        if ($pushedResponse = $this->multi->pushedResponses[$url] ?? null) {
+            unset($this->multi->pushedResponses[$url]);
+
+            if (self::acceptPushForRequest($method, $options, $pushedResponse)) {
+                $this->logger && $this->logger->debug(sprintf('Accepting pushed response: "%s %s"', $method, $url));
+
+                // Reinitialize the pushed response with request's options
+                $ch = $pushedResponse->handle;
+                $pushedResponse = $pushedResponse->response;
+                $pushedResponse->__construct($this->multi, $url, $options, $this->logger);
+            } else {
+                $this->logger && $this->logger->debug(sprintf('Rejecting pushed response: "%s".', $url));
+                $pushedResponse = null;
+            }
+        }
+
+        if (!$pushedResponse) {
+            $ch = curl_init();
+            $this->logger && $this->logger->info(sprintf('Request: "%s %s"', $method, $url));
+        }
 
         foreach ($curlopts as $opt => $value) {
             if (null !== $value && !curl_setopt($ch, $opt, $value) && CURLOPT_CERTINFO !== $opt) {
@@ -279,7 +281,7 @@ public function request(string $method, string $url, array $options = []): Respo
             }
         }
 
-        return new CurlResponse($this->multi, $ch, $options, $this->logger, $method, self::createRedirectResolver($options, $host));
+        return $pushedResponse ?? new CurlResponse($this->multi, $ch, $options, $this->logger, $method, self::createRedirectResolver($options, $host));
     }
 
     /**
@@ -369,7 +371,7 @@ private static function handlePush($parent, $pushed, array $requestHeaders, Curl
         $url .= $headers[':path'][0];
         $logger && $logger->debug(sprintf('Queueing pushed response: "%s"', $url));
 
-        $multi->pushedResponses[$url] = new PushedResponse(new CurlResponse($multi, $pushed), $headers, $multi->openHandles[(int) $parent][1] ?? []);
+        $multi->pushedResponses[$url] = new PushedResponse(new CurlResponse($multi, $pushed), $headers, $multi->openHandles[(int) $parent][1] ?? [], $pushed);
 
         return CURL_PUSH_OK;
     }

diff --git a/src/Symfony/Component/HttpClient/Internal/PushedResponse.php b/src/Symfony/Component/HttpClient/Internal/PushedResponse.php
@@ -29,10 +29,13 @@ final class PushedResponse
 
     public $parentOptions = [];
 
-    public function __construct(CurlResponse $response, array $requestHeaders, array $parentOptions)
+    public $handle;
+
+    public function __construct(CurlResponse $response, array $requestHeaders, array $parentOptions, $handle)
     {
         $this->response = $response;
         $this->requestHeaders = $requestHeaders;
         $this->parentOptions = $parentOptions;
+        $this->handle = $handle;
     }
 }