Skip to content

Commit 5f95ad0

Browse files
TobionTobion
committed
Merge branch '4.4'
2 parents 3d5c827 + be5a53b commit 5f95ad0

10 files changed

+75
-63
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ CHANGELOG
1414
* made `Psr18Client` implement relevant PSR-17 factories and have streaming responses
1515
* added `TraceableHttpClient`, `HttpClientDataCollector` and `HttpClientPass` to integrate with the web profiler
1616
* allow enabling buffering conditionally with a Closure
17+
* allow option "buffer" to be a stream resource
1718

1819
4.3.0
1920
-----

‎CurlHttpClient.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,7 @@ final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface
3737
use HttpClientTrait;
3838
use LoggerAwareTrait;
3939

40-
private $defaultOptions = [
41-
'buffer' => null, // bool|\Closure - a boolean or a closure telling if the response should be buffered based on its headers
42-
] + self::OPTIONS_DEFAULTS + [
40+
private $defaultOptions = self::OPTIONS_DEFAULTS + [
4341
'auth_ntlm' => null, // array|string - an array containing the username as first value, and optionally the
4442
// password as the second one; or string like username:password - enabling NTLM auth
4543
];
@@ -64,7 +62,7 @@ public function __construct(array $defaultOptions = [], int $maxHostConnections
6462
throw new \LogicException('You cannot use the "Symfony\Component\HttpClient\CurlHttpClient" as the "curl" extension is not installed.');
6563
}
6664

67-
$this->defaultOptions['buffer'] = \Closure::fromCallable([__CLASS__, 'shouldBuffer']);
65+
$this->defaultOptions['buffer'] = $this->defaultOptions['buffer'] ?? \Closure::fromCallable([__CLASS__, 'shouldBuffer']);
6866

6967
if ($defaultOptions) {
7068
[, $this->defaultOptions] = self::prepareRequest(null, null, $defaultOptions, $this->defaultOptions);

‎HttpClientTrait.php

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,32 @@ private static function prepareRequest(?string $method, ?string $url, array $opt
4242

4343
$options = self::mergeDefaultOptions($options, $defaultOptions, $allowExtraOptions);
4444

45+
$buffer = $options['buffer'] ?? true;
46+
47+
if ($buffer instanceof \Closure) {
48+
$options['buffer'] = static function (array $headers) use ($buffer) {
49+
if (!\is_bool($buffer = $buffer($headers))) {
50+
if (!\is_array($bufferInfo = @stream_get_meta_data($buffer))) {
51+
throw new \LogicException(sprintf('The closure passed as option "buffer" must return bool or stream resource, got %s.', \is_resource($buffer) ? get_resource_type($buffer).' resource' : \gettype($buffer)));
52+
}
53+
54+
if (false === strpbrk($bufferInfo['mode'], 'acew+')) {
55+
throw new \LogicException(sprintf('The stream returned by the closure passed as option "buffer" must be writeable, got mode "%s".', $bufferInfo['mode']));
56+
}
57+
}
58+
59+
return $buffer;
60+
};
61+
} elseif (!\is_bool($buffer)) {
62+
if (!\is_array($bufferInfo = @stream_get_meta_data($buffer))) {
63+
throw new InvalidArgumentException(sprintf('Option "buffer" must be bool, stream resource or Closure, %s given.', \is_resource($buffer) ? get_resource_type($buffer).' resource' : \gettype($buffer)));
64+
}
65+
66+
if (false === strpbrk($bufferInfo['mode'], 'acew+')) {
67+
throw new InvalidArgumentException(sprintf('The stream in option "buffer" must be writeable, mode "%s" given.', $bufferInfo['mode']));
68+
}
69+
}
70+
4571
if (isset($options['json'])) {
4672
if (isset($options['body']) && '' !== $options['body']) {
4773
throw new InvalidArgumentException('Define either the "json" or the "body" option, setting both is not supported.');

‎NativeHttpClient.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,7 @@ final class NativeHttpClient implements HttpClientInterface, LoggerAwareInterfac
3535
use HttpClientTrait;
3636
use LoggerAwareTrait;
3737

38-
private $defaultOptions = [
39-
'buffer' => null, // bool|\Closure - a boolean or a closure telling if the response should be buffered based on its headers
40-
] + self::OPTIONS_DEFAULTS;
38+
private $defaultOptions = self::OPTIONS_DEFAULTS;
4139

4240
/** @var NativeClientState */
4341
private $multi;
@@ -50,7 +48,7 @@ final class NativeHttpClient implements HttpClientInterface, LoggerAwareInterfac
5048
*/
5149
public function __construct(array $defaultOptions = [], int $maxHostConnections = 6)
5250
{
53-
$this->defaultOptions['buffer'] = \Closure::fromCallable([__CLASS__, 'shouldBuffer']);
51+
$this->defaultOptions['buffer'] = $this->defaultOptions['buffer'] ?? \Closure::fromCallable([__CLASS__, 'shouldBuffer']);
5452

5553
if ($defaultOptions) {
5654
[, $this->defaultOptions] = self::prepareRequest(null, null, $defaultOptions, $this->defaultOptions);

‎Response/CurlResponse.php

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,21 @@ public function __construct(CurlClientState $multi, $ch, array $options = null,
6464
}
6565

6666
if (null === $content = &$this->content) {
67-
$content = null === $options || true === $options['buffer'] ? fopen('php://temp', 'w+') : null;
67+
$content = null === $options || true === $options['buffer'] ? fopen('php://temp', 'w+') : (\is_resource($options['buffer']) ? $options['buffer'] : null);
6868
} else {
6969
// Move the pushed response to the activity list
7070
$buffer = $options['buffer'];
7171

7272
if ('headers' !== curl_getinfo($ch, CURLINFO_PRIVATE)) {
7373
if ($options['buffer'] instanceof \Closure) {
74-
[$content, $buffer] = [null, $content];
75-
[$content, $buffer] = [$buffer, (bool) $options['buffer']($headers)];
74+
try {
75+
[$content, $buffer] = [null, $content];
76+
[$content, $buffer] = [$buffer, $options['buffer']($headers)];
77+
} catch (\Throwable $e) {
78+
$multi->handlesActivity[$id][] = null;
79+
$multi->handlesActivity[$id][] = $e;
80+
[$content, $buffer] = [$buffer, false];
81+
}
7682
}
7783

7884
if (ftell($content)) {
@@ -81,7 +87,9 @@ public function __construct(CurlClientState $multi, $ch, array $options = null,
8187
}
8288
}
8389

84-
if (true !== $buffer) {
90+
if (\is_resource($buffer)) {
91+
$content = $buffer;
92+
} elseif (true !== $buffer) {
8593
$content = null;
8694
}
8795
}
@@ -384,8 +392,8 @@ private static function parseHeaderLine($ch, string $data, array &$info, array &
384392
curl_setopt($ch, CURLOPT_PRIVATE, 'content');
385393

386394
try {
387-
if (!$content && $options['buffer'] instanceof \Closure && $options['buffer']($headers)) {
388-
$content = fopen('php://temp', 'w+');
395+
if (!$content && $options['buffer'] instanceof \Closure && $content = $options['buffer']($headers) ?: null) {
396+
$content = \is_resource($content) ? $content : fopen('php://temp', 'w+');
389397
}
390398
} catch (\Throwable $e) {
391399
$multi->handlesActivity[$id][] = null;

‎Response/MockResponse.php

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,16 +107,19 @@ public static function fromRequest(string $method, string $url, array $options,
107107
$response->id = ++self::$idSequence;
108108

109109
if (!($options['buffer'] ?? null) instanceof \Closure) {
110-
$response->content = true === ($options['buffer'] ?? true) ? fopen('php://temp', 'w+') : null;
110+
$response->content = true === ($options['buffer'] ?? true) ? fopen('php://temp', 'w+') : (\is_resource($options['buffer']) ? $options['buffer'] : null);
111111
}
112112
$response->initializer = static function (self $response) {
113113
if (null !== $response->info['error']) {
114114
throw new TransportException($response->info['error']);
115115
}
116116

117117
if (\is_array($response->body[0] ?? null)) {
118-
// Consume the first chunk if it's not yielded yet
119-
self::stream([$response])->current();
118+
foreach (self::stream([$response]) as $chunk) {
119+
if ($chunk->isFirst()) {
120+
break;
121+
}
122+
}
120123
}
121124
};
122125

@@ -183,9 +186,10 @@ protected static function perform(ClientState $multi, array &$responses): void
183186
$response->headers = $chunk[1]->getHeaders(false);
184187
self::readResponse($response, $chunk[0], $chunk[1], $offset);
185188
$multi->handlesActivity[$id][] = new FirstChunk();
189+
$buffer = $response->requestOptions['buffer'] ?? null;
186190

187-
if (($response->requestOptions['buffer'] ?? null) instanceof \Closure) {
188-
$response->content = $response->requestOptions['buffer']($response->headers) ? fopen('php://temp', 'w+') : null;
191+
if ($buffer instanceof \Closure && $response->content = $buffer($response->headers) ?: null) {
192+
$response->content = \is_resource($response->content) ? $response->content : fopen('php://temp', 'w+');
189193
}
190194
} catch (\Throwable $e) {
191195
$multi->handlesActivity[$id][] = null;

‎Response/NativeResponse.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public function __construct(NativeClientState $multi, $context, string $url, $op
5151
$this->info = &$info;
5252
$this->resolveRedirect = $resolveRedirect;
5353
$this->onProgress = $onProgress;
54-
$this->content = true === $options['buffer'] ? fopen('php://temp', 'w+') : null;
54+
$this->content = true === $options['buffer'] ? fopen('php://temp', 'w+') : (\is_resource($options['buffer']) ? $options['buffer'] : null);
5555
$this->shouldBuffer = $options['buffer'] instanceof \Closure ? $options['buffer'] : null;
5656

5757
// Temporary resources to dechunk/inflate the response stream
@@ -179,8 +179,8 @@ private function open(): void
179179
}
180180

181181
try {
182-
if (null !== $this->shouldBuffer && null === $this->content && ($this->shouldBuffer)($this->headers)) {
183-
$this->content = fopen('php://temp', 'w+');
182+
if (null !== $this->shouldBuffer && null === $this->content && $this->content = ($this->shouldBuffer)($this->headers) ?: null) {
183+
$this->content = \is_resource($this->content) ? $this->content : fopen('php://temp', 'w+');
184184
}
185185

186186
if (!$this->buffer) {

‎Tests/HttpClientTestCase.php

Lines changed: 16 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,10 @@
1212
namespace Symfony\Component\HttpClient\Tests;
1313

1414
use Symfony\Component\HttpClient\Exception\ClientException;
15-
use Symfony\Component\HttpClient\Exception\TransportException;
1615
use Symfony\Contracts\HttpClient\Test\HttpClientTestCase as BaseHttpClientTestCase;
1716

1817
abstract class HttpClientTestCase extends BaseHttpClientTestCase
1918
{
20-
public function testMaxDuration()
21-
{
22-
$this->markTestSkipped('Implemented as of version 4.4');
23-
}
24-
2519
public function testAcceptHeader()
2620
{
2721
$client = $this->getHttpClient(__FUNCTION__);
@@ -82,50 +76,33 @@ public function testToStream404()
8276
$stream = $response->toStream();
8377
}
8478

85-
public function testConditionalBuffering()
79+
public function testInfoOnCanceledResponse()
8680
{
87-
$client = $this->getHttpClient(__FUNCTION__);
88-
$response = $client->request('GET', 'http://localhost:8057');
89-
$firstContent = $response->getContent();
90-
$secondContent = $response->getContent();
91-
92-
$this->assertSame($firstContent, $secondContent);
81+
$this->markTestSkipped('Implemented as of version 4.4');
82+
}
9383

94-
$response = $client->request('GET', 'http://localhost:8057', ['buffer' => function () { return false; }]);
95-
$response->getContent();
84+
public function testBufferSink()
85+
{
86+
$this->markTestSkipped('Implemented as of version 4.4');
87+
}
9688

97-
$this->expectException(TransportException::class);
98-
$this->expectExceptionMessage('Cannot get the content of the response twice: buffering is disabled.');
99-
$response->getContent();
89+
public function testConditionalBuffering()
90+
{
91+
$this->markTestSkipped('Implemented as of version 4.4');
10092
}
10193

10294
public function testReentrantBufferCallback()
10395
{
104-
$client = $this->getHttpClient(__FUNCTION__);
105-
106-
$response = $client->request('GET', 'http://localhost:8057', ['buffer' => function () use (&$response) {
107-
$response->cancel();
108-
}]);
109-
110-
$this->assertSame(200, $response->getStatusCode());
111-
112-
$this->expectException(TransportException::class);
113-
$this->expectExceptionMessage('Response has been canceled.');
114-
$response->getContent();
96+
$this->markTestSkipped('Implemented as of version 4.4');
11597
}
11698

11799
public function testThrowingBufferCallback()
118100
{
119-
$client = $this->getHttpClient(__FUNCTION__);
120-
121-
$response = $client->request('GET', 'http://localhost:8057', ['buffer' => function () {
122-
throw new \Exception('Boo');
123-
}]);
124-
125-
$this->assertSame(200, $response->getStatusCode());
101+
$this->markTestSkipped('Implemented as of version 4.4');
102+
}
126103

127-
$this->expectException(TransportException::class);
128-
$this->expectExceptionMessage('Boo');
129-
$response->getContent();
104+
public function testMaxDuration()
105+
{
106+
$this->markTestSkipped('Implemented as of version 4.4');
130107
}
131108
}

‎Tests/MockHttpClientTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ protected function getHttpClient(string $testCase): HttpClientInterface
4848
return new MockHttpClient(function (string $method, string $url, array $options) use ($client) {
4949
try {
5050
// force the request to be completed so that we don't test side effects of the transport
51-
$response = $client->request($method, $url, $options);
51+
$response = $client->request($method, $url, ['buffer' => false] + $options);
5252
$content = $response->getContent(false);
5353

5454
return new MockResponse($content, $response->getInfo());

‎composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
"require": {
2323
"php": "^7.2.9",
2424
"psr/log": "^1.0",
25-
"symfony/http-client-contracts": "^1.1.7|^2",
25+
"symfony/http-client-contracts": "^1.1.8|^2",
2626
"symfony/polyfill-php73": "^1.11"
2727
},
2828
"require-dev": {

0 commit comments

Comments
 (0)