bug #59062 [HttpClient] Always set CURLOPT_CUSTOMREQUEST to the correct HTTP method in CurlHttpClient (KurtThiemann)

nicolas-grekas · nicolas-grekas · commit ffc4dc611c03 · 2024-12-02T18:53:07.000+01:00
This PR was submitted for the 7.2 branch but it was squashed and merged into the 6.4 branch instead. Discussion ---------- [HttpClient] Always set CURLOPT_CUSTOMREQUEST to the correct HTTP method in CurlHttpClient | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix #59043 | License | MIT Currently, when sending a `HEAD` request and using a closure as the request body using `CurlHttpClient`, a `PUT` request instead of a HEAD request will be sent. This is because `CURLOPT_NOBODY` is set to make a `HEAD` request, but is overwritten by `CURLOPT_UPLOAD`, which is necessary to make curl call `CURLOPT_READFUNCTION`, but also sets the HTTP method to `PUT`. The solution I am suggesting is to just always set `CURLOPT_CUSTOMREQUEST`, so the correct HTTP method will be used, no matter what other options are added. This is mainly an issue in the PSR-18 wrapper, since PSR requests always have a request body stream, even if the body is empty. Since #58856, `Psr18Client` always passes a closure as the request body to the underlying HTTP client instead of just reading the entire request body stream into a string, as that made it impossible to upload large files. While it would be possible to add an exception to `Psr18Client` to only add the request body if the method used commonly has a body, I think the better solution is to ensure that `CurlHttpClient` always actually sends the type of request that it is asked to send. Commits ------- bfdf5d9 [HttpClient] Always set CURLOPT_CUSTOMREQUEST to the correct HTTP method in CurlHttpClient
diff --git a/src/Symfony/Component/HttpClient/CurlHttpClient.php b/src/Symfony/Component/HttpClient/CurlHttpClient.php
@@ -197,13 +197,12 @@ public function request(string $method, string $url, array $options = []): Respo
             $curlopts[\CURLOPT_RESOLVE] = $resolve;
         }
 
+        $curlopts[\CURLOPT_CUSTOMREQUEST] = $method;
         if ('POST' === $method) {
             // Use CURLOPT_POST to have browser-like POST-to-GET redirects for 301, 302 and 303
             $curlopts[\CURLOPT_POST] = true;
         } elseif ('HEAD' === $method) {
             $curlopts[\CURLOPT_NOBODY] = true;
-        } else {
-            $curlopts[\CURLOPT_CUSTOMREQUEST] = $method;
         }
 
         if ('\\' !== \DIRECTORY_SEPARATOR && $options['timeout'] < 1) {
diff --git a/src/Symfony/Component/HttpClient/Tests/HttpClientTestCase.php b/src/Symfony/Component/HttpClient/Tests/HttpClientTestCase.php
@@ -651,4 +651,26 @@ public function testDefaultContentType()
 
         $this->assertSame(['abc' => 'def', 'content-type' => 'application/json', 'REQUEST_METHOD' => 'POST'], $response->toArray());
     }
+
+    public function testHeadRequestWithClosureBody()
+    {
+        $p = TestHttpServer::start(8067);
+
+        try {
+            $client = $this->getHttpClient(__FUNCTION__);
+
+            $response = $client->request('HEAD', 'http://localhost:8057/head', [
+                'body' => fn () => '',
+            ]);
+            $headers = $response->getHeaders();
+        } finally {
+            $p->stop();
+        }
+
+        $this->assertArrayHasKey('x-request-vars', $headers);
+
+        $vars = json_decode($headers['x-request-vars'][0], true);
+        $this->assertIsArray($vars);
+        $this->assertSame('HEAD', $vars['REQUEST_METHOD']);
+    }
 }
diff --git a/src/Symfony/Component/HttpClient/composer.json b/src/Symfony/Component/HttpClient/composer.json
@@ -25,7 +25,7 @@
         "php": ">=8.1",
         "psr/log": "^1|^2|^3",
         "symfony/deprecation-contracts": "^2.5|^3",
-        "symfony/http-client-contracts": "~3.4.3|^3.5.1",
+        "symfony/http-client-contracts": "~3.4.4|^3.5.2",
         "symfony/service-contracts": "^2.5|^3"
     },
     "require-dev": {
diff --git a/src/Symfony/Contracts/HttpClient/Test/Fixtures/web/index.php b/src/Symfony/Contracts/HttpClient/Test/Fixtures/web/index.php
@@ -42,6 +42,7 @@
         exit;
 
     case '/head':
+        header('X-Request-Vars: '.json_encode($vars, \JSON_UNESCAPED_SLASHES | \JSON_UNESCAPED_UNICODE));
         header('Content-Length: '.strlen($json), true);
         break;
 

Original file line number	Diff line number	Diff line change
`@@ -197,13 +197,12 @@ public function request(string $method, string $url, array $options = []): Respo`
`197`	`197`	`$curlopts[\CURLOPT_RESOLVE] = $resolve;`
`198`	`198`	`}`
`199`	`199`
	`200`	`+ $curlopts[\CURLOPT_CUSTOMREQUEST] = $method;`
`200`	`201`	`if ('POST' === $method) {`
`201`	`202`	`// Use CURLOPT_POST to have browser-like POST-to-GET redirects for 301, 302 and 303`
`202`	`203`	`$curlopts[\CURLOPT_POST] = true;`
`203`	`204`	`} elseif ('HEAD' === $method) {`
`204`	`205`	`$curlopts[\CURLOPT_NOBODY] = true;`
`205`		`- } else {`
`206`		`- $curlopts[\CURLOPT_CUSTOMREQUEST] = $method;`
`207`	`206`	`}`
`208`	`207`
`209`	`208`	`if ('\\' !== \DIRECTORY_SEPARATOR && $options['timeout'] < 1) {`