Skip to content

Commit

Permalink
feature #32565 [HttpClient] Allow enabling buffering conditionally wi…
Browse files Browse the repository at this point in the history
…th 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
-------

f705ac9 [HttpClient] Allow enabling buffering conditionally with a Closure
  • Loading branch information
nicolas-grekas committed Sep 9, 2019
2 parents 312cbf9 + f705ac9 commit e9f524a
Show file tree
Hide file tree
Showing 10 changed files with 68 additions and 14 deletions.
1 change: 1 addition & 0 deletions src/Symfony/Component/HttpClient/CHANGELOG.md
Expand Up @@ -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
-----
Expand Down
3 changes: 1 addition & 2 deletions src/Symfony/Component/HttpClient/CachingHttpClient.php
Expand Up @@ -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);
}

Expand Down
8 changes: 6 additions & 2 deletions src/Symfony/Component/HttpClient/CurlHttpClient.php
Expand Up @@ -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
];
Expand All @@ -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();
Expand Down
11 changes: 11 additions & 0 deletions src/Symfony/Component/HttpClient/HttpClientTrait.php
Expand Up @@ -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);
}
}
8 changes: 6 additions & 2 deletions src/Symfony/Component/HttpClient/NativeHttpClient.php
Expand Up @@ -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;
Expand All @@ -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();
Expand Down
14 changes: 9 additions & 5 deletions src/Symfony/Component/HttpClient/Response/CurlResponse.php
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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']));
Expand Down
7 changes: 6 additions & 1 deletion src/Symfony/Component/HttpClient/Response/MockResponse.php
Expand Up @@ -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']);
Expand Down
10 changes: 9 additions & 1 deletion src/Symfony/Component/HttpClient/Response/NativeResponse.php
Expand Up @@ -35,6 +35,7 @@ final class NativeResponse implements ResponseInterface
private $inflate;
private $multi;
private $debugBuffer;
private $shouldBuffer;

/**
* @internal
Expand All @@ -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+');
Expand Down Expand Up @@ -92,6 +94,8 @@ public function getInfo(string $type = null)

public function __destruct()
{
$this->shouldBuffer = null;

try {
$this->doDestruct();
} finally {
Expand Down Expand Up @@ -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'];
}
Expand Down
Expand Up @@ -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;
Expand Down
18 changes: 18 additions & 0 deletions src/Symfony/Component/HttpClient/Tests/HttpClientTestCase.php
Expand Up @@ -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
Expand All @@ -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();
}
}

0 comments on commit e9f524a

Please sign in to comment.