feature #32565 [HttpClient] Allow enabling buffering conditionally with a Closure (rjwebdev)

This PR was merged into the 4.4 branch.

Discussion
----------

[HttpClient] Allow enabling buffering conditionally with a Closure

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #31883
| License       | MIT
| Doc PR        | symfony/symfony-docs#12043

With this PR, responses can be buffered automatically from a closure passed to the `buffer` option.

```php
$resp = $client->request('GET', $url, [
    'buffer' => function (array $headers): bool { return true/false; },
]);
```

When no option is provided, buffering is now enabled only for json, xml and text/* content types.

Commits
-------

f705ac9dc4 [HttpClient] Allow enabling buffering conditionally with a Closure

Author: nicolas-grekas

CHANGELOG.md

@@ -11,6 +11,7 @@ CHANGELOG
  * added `$response->toStream()` to cast responses to regular PHP streams
  * made `Psr18Client` implement relevant PSR-17 factories and have streaming responses
  * added `TraceableHttpClient`, `HttpClientDataCollector` and `HttpClientPass` to integrate with the web profiler
+ * allow enabling buffering conditionally with a Closure
 
 4.3.0
 -----

CachingHttpClient.php

@@ -68,9 +68,8 @@ public function request(string $method, string $url, array $options = []): Respo
     {
         [$url, $options] = $this->prepareRequest($method, $url, $options, $this->defaultOptions, true);
         $url = implode('', $url);
-        $options['extra']['no_cache'] = $options['extra']['no_cache'] ?? !$options['buffer'];
 
-        if (!empty($options['body']) || $options['extra']['no_cache'] || !\in_array($method, ['GET', 'HEAD', 'OPTIONS'])) {
+        if (!empty($options['body']) || !empty($options['extra']['no_cache']) || !\in_array($method, ['GET', 'HEAD', 'OPTIONS'])) {
             return $this->client->request($method, $url, $options);
         }
 

CurlHttpClient.php

@@ -37,7 +37,9 @@ final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface
     use HttpClientTrait;
     use LoggerAwareTrait;
 
-    private $defaultOptions = self::OPTIONS_DEFAULTS + [
+    private $defaultOptions = [
+        'buffer' => null, // bool|\Closure - a boolean or a closure telling if the response should be buffered based on its headers
+    ] + self::OPTIONS_DEFAULTS + [
         'auth_ntlm' => null, // array|string - an array containing the username as first value, and optionally the
                              //   password as the second one; or string like username:password - enabling NTLM auth
     ];
@@ -62,8 +64,10 @@ public function __construct(array $defaultOptions = [], int $maxHostConnections
             throw new \LogicException('You cannot use the "Symfony\Component\HttpClient\CurlHttpClient" as the "curl" extension is not installed.');
         }
 
+        $this->defaultOptions['buffer'] = \Closure::fromCallable([__CLASS__, 'shouldBuffer']);
+
         if ($defaultOptions) {
-            [, $this->defaultOptions] = self::prepareRequest(null, null, $defaultOptions, self::OPTIONS_DEFAULTS);
+            [, $this->defaultOptions] = self::prepareRequest(null, null, $defaultOptions, $this->defaultOptions);
         }
 
         $this->multi = $multi = new CurlClientState();

HttpClientTrait.php

@@ -503,4 +503,15 @@ private static function mergeQueryString(?string $queryString, array $queryArray
 
         return implode('&', $replace ? array_replace($query, $queryArray) : ($query + $queryArray));
     }
+
+    private static function shouldBuffer(array $headers): bool
+    {
+        $contentType = $headers['content-type'][0] ?? null;
+
+        if (false !== $i = strpos($contentType, ';')) {
+            $contentType = substr($contentType, 0, $i);
+        }
+
+        return $contentType && preg_match('#^(?:text/|application/(?:.+\+)?(?:json|xml)$)#i', $contentType);
+    }
 }

NativeHttpClient.php

@@ -35,7 +35,9 @@ final class NativeHttpClient implements HttpClientInterface, LoggerAwareInterfac
     use HttpClientTrait;
     use LoggerAwareTrait;
 
-    private $defaultOptions = self::OPTIONS_DEFAULTS;
+    private $defaultOptions = [
+        'buffer' => null, // bool|\Closure - a boolean or a closure telling if the response should be buffered based on its headers
+    ] + self::OPTIONS_DEFAULTS;
 
     /** @var NativeClientState */
     private $multi;
@@ -48,8 +50,10 @@ final class NativeHttpClient implements HttpClientInterface, LoggerAwareInterfac
      */
     public function __construct(array $defaultOptions = [], int $maxHostConnections = 6)
     {
+        $this->defaultOptions['buffer'] = \Closure::fromCallable([__CLASS__, 'shouldBuffer']);
+
         if ($defaultOptions) {
-            [, $this->defaultOptions] = self::prepareRequest(null, null, $defaultOptions, self::OPTIONS_DEFAULTS);
+            [, $this->defaultOptions] = self::prepareRequest(null, null, $defaultOptions, $this->defaultOptions);
         }
 
         $this->multi = new NativeClientState();

Response/CurlResponse.php

@@ -64,18 +64,18 @@ public function __construct(CurlClientState $multi, $ch, array $options = null,
         }
 
         if (null === $content = &$this->content) {
-            $content = ($options['buffer'] ?? true) ? fopen('php://temp', 'w+') : null;
+            $content = true === $options['buffer'] ? fopen('php://temp', 'w+') : null;
         } else {
             // Move the pushed response to the activity list
             if (ftell($content)) {
                 rewind($content);
                 $multi->handlesActivity[$id][] = stream_get_contents($content);
             }
-            $content = ($options['buffer'] ?? true) ? $content : null;
+            $content = true === $options['buffer'] ? $content : null;
         }
 
-        curl_setopt($ch, CURLOPT_HEADERFUNCTION, static function ($ch, string $data) use (&$info, &$headers, $options, $multi, $id, &$location, $resolveRedirect, $logger): int {
-            return self::parseHeaderLine($ch, $data, $info, $headers, $options, $multi, $id, $location, $resolveRedirect, $logger);
+        curl_setopt($ch, CURLOPT_HEADERFUNCTION, static function ($ch, string $data) use (&$info, &$headers, $options, $multi, $id, &$location, $resolveRedirect, $logger, &$content): int {
+            return self::parseHeaderLine($ch, $data, $info, $headers, $options, $multi, $id, $location, $resolveRedirect, $logger, $content);
         });
 
         if (null === $options) {
@@ -278,7 +278,7 @@ private static function select(CurlClientState $multi, float $timeout): int
     /**
      * Parses header lines as curl yields them to us.
      */
-    private static function parseHeaderLine($ch, string $data, array &$info, array &$headers, ?array $options, CurlClientState $multi, int $id, ?string &$location, ?callable $resolveRedirect, ?LoggerInterface $logger): int
+    private static function parseHeaderLine($ch, string $data, array &$info, array &$headers, ?array $options, CurlClientState $multi, int $id, ?string &$location, ?callable $resolveRedirect, ?LoggerInterface $logger, &$content = null): int
     {
         if (!\in_array($waitFor = @curl_getinfo($ch, CURLINFO_PRIVATE), ['headers', 'destruct'], true)) {
             return \strlen($data); // Ignore HTTP trailers
@@ -349,6 +349,10 @@ private static function parseHeaderLine($ch, string $data, array &$info, array &
                 return 0;
             }
 
+            if ($options['buffer'] instanceof \Closure && !$content && $options['buffer']($headers)) {
+                $content = fopen('php://temp', 'w+');
+            }
+
             curl_setopt($ch, CURLOPT_PRIVATE, 'content');
         } elseif (null !== $info['redirect_url'] && $logger) {
             $logger->info(sprintf('Redirecting: "%s %s"', $info['http_code'], $info['redirect_url']));

Response/MockResponse.php

@@ -104,7 +104,12 @@ public static function fromRequest(string $method, string $url, array $options,
         $response = new self([]);
         $response->requestOptions = $options;
         $response->id = ++self::$idSequence;
-        $response->content = ($options['buffer'] ?? true) ? fopen('php://temp', 'w+') : null;
+
+        if (($options['buffer'] ?? null) instanceof \Closure) {
+            $response->content = $options['buffer']($mock->getHeaders(false)) ? fopen('php://temp', 'w+') : null;
+        } else {
+            $response->content = true === ($options['buffer'] ?? true) ? fopen('php://temp', 'w+') : null;
+        }
         $response->initializer = static function (self $response) {
             if (null !== $response->info['error']) {
                 throw new TransportException($response->info['error']);

Response/NativeResponse.php

@@ -35,6 +35,7 @@ final class NativeResponse implements ResponseInterface
     private $inflate;
     private $multi;
     private $debugBuffer;
+    private $shouldBuffer;
 
     /**
      * @internal
@@ -50,7 +51,8 @@ public function __construct(NativeClientState $multi, $context, string $url, $op
         $this->info = &$info;
         $this->resolveRedirect = $resolveRedirect;
         $this->onProgress = $onProgress;
-        $this->content = $options['buffer'] ? fopen('php://temp', 'w+') : null;
+        $this->content = true === $options['buffer'] ? fopen('php://temp', 'w+') : null;
+        $this->shouldBuffer = $options['buffer'] instanceof \Closure ? $options['buffer'] : null;
 
         // Temporary resources to dechunk/inflate the response stream
         $this->buffer = fopen('php://temp', 'w+');
@@ -92,6 +94,8 @@ public function getInfo(string $type = null)
 
     public function __destruct()
     {
+        $this->shouldBuffer = null;
+
         try {
             $this->doDestruct();
         } finally {
@@ -152,6 +156,10 @@ private function open(): void
         stream_set_blocking($h, false);
         $this->context = $this->resolveRedirect = null;
 
+        if (null !== $this->shouldBuffer && null === $this->content && ($this->shouldBuffer)($this->headers)) {
+            $this->content = fopen('php://temp', 'w+');
+        }
+
         if (isset($context['ssl']['peer_certificate_chain'])) {
             $this->info['peer_certificate_chain'] = $context['ssl']['peer_certificate_chain'];
         }

Response/ResponseTrait.php

@@ -117,7 +117,7 @@ public function getContent(bool $throw = true): string
             }
 
             if (null === $content) {
-                throw new TransportException('Cannot get the content of the response twice: the request was issued with option "buffer" set to false.');
+                throw new TransportException('Cannot get the content of the response twice: buffering is disabled.');
             }
 
             return $content;

Tests/HttpClientTestCase.php

@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\HttpClient\Tests;
 
+use Symfony\Component\HttpClient\Exception\TransportException;
 use Symfony\Contracts\HttpClient\Test\HttpClientTestCase as BaseHttpClientTestCase;
 
 abstract class HttpClientTestCase extends BaseHttpClientTestCase
@@ -37,4 +38,21 @@ public function testToStream()
         $this->assertSame('', fread($stream, 1));
         $this->assertTrue(feof($stream));
     }
+
+    public function testConditionalBuffering()
+    {
+        $client = $this->getHttpClient(__FUNCTION__);
+        $response = $client->request('GET', 'http://localhost:8057');
+        $firstContent = $response->getContent();
+        $secondContent = $response->getContent();
+
+        $this->assertSame($firstContent, $secondContent);
+
+        $response = $client->request('GET', 'http://localhost:8057', ['buffer' => function () { return false; }]);
+        $response->getContent();
+
+        $this->expectException(TransportException::class);
+        $this->expectExceptionMessage('Cannot get the content of the response twice: buffering is disabled.');
+        $response->getContent();
+    }
 }