8000 bug #33982 [HttpClient] improve StreamWrapper (nicolas-grekas) · symfony/symfony@c4b39b7 · GitHub
[go: up one dir, main page]
Skip to content

Commit c4b39b7

Browse files
nicolas-grekasnicolas-grekas
committed
bug #33982 [HttpClient] improve StreamWrapper (nicolas-grekas)
This PR was merged into the 4.4 branch. Discussion ---------- [HttpClient] improve StreamWrapper | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | symfony/symfony-docs#12389 Spotted while working on the linked doc PR. Commits ------- ea52d1c [HttpClient] improve StreamWrapper
2 parents cbc4efc + ea52d1c commit c4b39b7

File tree

7 files changed

+128
-18
lines changed

7 files changed

+128
-18
lines changed

‎src/Symfony/Component/HttpClient/Response/CurlResponse.php

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,13 @@ public function __destruct()
227227
*/
228228
private function close(): void
229229
{
230+
if (self::$performing) {
231+
$this->multi->handlesActivity[$this->id][] = null;
232+
$this->multi->handlesActivity[$this->id][] = new TransportException('Response has been canceled.');
233+
234+
return;
235+
}
236+
230237
unset($this->multi->openHandles[$this->id], $this->multi->handlesActivity[$this->id]);
231238
curl_multi_remove_handle($this->multi->handle, $this->handle);
232239
curl_setopt_array($this->handle, [
@@ -264,6 +271,12 @@ private static function schedule(self $response, array &$runningResponses): void
264271
private static function perform(CurlClientState $multi, array &$responses = null): void
265272
{
266273
if (self::$performing) {
274+
if ($responses) {
275+
$response = current($responses);
276+
$multi->handlesActivity[(int) $response->handle][] = null;
277+
$multi->handlesActivity[(int) $response->handle][] = new TransportException(sprintf('Userland callback cannot use the client nor the response while processing "%s".', curl_getinfo($response->handle, CURLINFO_EFFECTIVE_URL)));
278+
}
279+
267280
return;
268281
}
269282

@@ -370,8 +383,15 @@ private static function parseHeaderLine($ch, string $data, array &$info, array &
370383

371384
curl_setopt($ch, CURLOPT_PRIVATE, 'content');
372385

373-
if (!$content && $options['buffer'] instanceof \Closure && $options['buffer']($headers)) {
374-
$content = fopen('php://temp', 'w+');
386+
try {
387+
if (!$content && $options['buffer'] instanceof \Closure && $options['buffer']($headers)) {
388+
$content = fopen('php://temp', 'w+');
389+
}
390+
} catch (\Throwable $e) {
391+
$multi->handlesActivity[$id][] = null;
392+
$multi->handlesActivity[$id][] = $e;
393+
394+
return 0;
375395
}
376396
} elseif (null !== $info['redirect_url'] && $logger) {
377397
$logger->info(sprintf('Redirecting: "%s %s"', $info['http_code'], $info['redirect_url']));

‎src/Symfony/Component/HttpClient/Response/MockResponse.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,7 @@ public static function fromRequest(string $method, string $url, array $options,
105105
$response->requestOptions = $options;
106106
$response->id = ++self::$idSequence;
107107

108-
if (($options['buffer'] ?? null) instanceof \Closure) {
109-
$response->content = $options['buffer']($mock->getHeaders(false)) ? fopen('php://temp', 'w+') : null;
110-
} else {
108+
if (!($options['buffer'] ?? null) inst F438 anceof \Closure) {
111109
$response->content = true === ($options['buffer'] ?? true) ? fopen('php://temp', 'w+') : null;
112110
}
113111
$response->initializer = static function (self $response) {
@@ -184,6 +182,10 @@ protected static function perform(ClientState $multi, array &$responses): void
184182
$response->headers = $chunk[1]->getHeaders(false);
185183
self::readResponse($response, $chunk[0], $chunk[1], $offset);
186184
$multi->handlesActivity[$id][] = new FirstChunk();
185+
186+
if (($response->requestOptions['buffer'] ?? null) instanceof \Closure) {
187+
$response->content = $response->requestOptions['buffer']($response->headers) ? fopen('php://temp', 'w+') : null;
188+
}
187189
} catch (\Throwable $e) {
188190
$multi->handlesActivity[$id][] = null;
189191
$multi->handlesActivity[$id][] = $e;

‎src/Symfony/Component/HttpClient/Response/NativeResponse.php

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -160,10 +160,6 @@ private function open(): void
160160
stream_set_blocking($h, false);
161161
$this->context = $this->resolveRedirect = null;
162162

163-
if (null !== $this->shouldBuffer && null === $this->content && ($this->shouldBuffer)($this->headers)) {
164-
$this->content = fopen('php://temp', 'w+');
165-
}
166-
167163
if (isset($context['ssl']['peer_certificate_chain'])) {
168164
$this->info['peer_certificate_chain'] = $context['ssl']['peer_certificate_chain'];
169165
}
@@ -182,6 +178,23 @@ private function open(): void
182178
$this->inflate = null;
183179
}
184180

181+
try {
182+
if (null !== $this->shouldBuffer && null === $this->content && ($this->shouldBuffer)($this->headers)) {
183+
$this->content = fopen('php://temp', 'w+');
184+
}
185+
186+
if (!$this->buffer) {
187+
throw new TransportException('Response has been canceled.');
188+
}
189+
} catch (\Throwable $e) {
190+
$this->close();
191+
$this->multi->handlesActivity[$this->id] = [new FirstChunk()];
192+
$this->multi->handlesActivity[$this->id][] = null;
193+
$this->multi->handlesActivity[$this->id][] = $e;
194+
195+
return;
196+
}
197+
185198
$this->multi->openHandles[$this->id] = [$h, $this->buffer, $this->inflate, $this->content, $this->onProgress, &$this->remaining, &$this->info];
186199
$this->multi->handlesActivity[$this->id] = [new FirstChunk()];
187200
}

‎src/Symfony/Component/HttpClient/Response/ResponseTrait.php

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ trait ResponseTrait
5757
/** @var resource */
5858
private $handle;
5959
private $id;
60-
private $timeout;
60+
private $timeout = 0;
6161
private $finalInfo;
6262
private $offset = 0;
6363
private $jsonData;
@@ -185,7 +185,7 @@ public function cancel(): void
185185
/**
186186
* Casts the response to a PHP stream resource.
187187
*
188-
* @return resource|null
188+
* @return resource
189189
*
190190
* @throws TransportExceptionInterface When a network error occurs
191191
* @throws RedirectionExceptionInterface On a 3xx when $throw is true and the "max_redirects" option has been reached
@@ -194,8 +194,10 @@ public function cancel(): void
194194
*/
195195
public function toStream(bool $throw = true)
196196
{
197-
// Ensure headers arrived
198-
$this->getHeaders($throw);
197+
if ($throw) {
198+
// Ensure headers arrived
199+
$this->getHeaders($throw);
200+
}
199201

200202
return StreamWrapper::createResource($this, null, $this->content, $this->handle && 'stream' === get_resource_type($this->handle) ? $this->handle : null);
201203
}

‎src/Symfony/Component/HttpClient/Response/StreamWrapper.php

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,11 @@ public static function createResource(ResponseInterface $response, HttpClientInt
7373
}
7474
}
7575

76+
public function getResponse(): ResponseInterface
77+
{
78+
return $this->response;
79+
}
80+
7681
public function stream_open(string $path, string $mode, int $options): bool
7782
{
7883
if ('r' !== $mode) {
@@ -107,7 +112,9 @@ public function stream_read(int $count)
107112
// Empty the internal activity list
108113
foreach ($this->client->stream([$this->response], 0) as $chunk) {
109114
try {
110-
$chunk->isTimeout();
115+
if (!$chunk->isTimeout() && $chunk->isFirst()) {
116+
$this->response->getStatusCode(); // ignore 3/4/5xx
117+
}
111118
} catch (ExceptionInterface $e) {
112119
trigger_error($e->getMessage(), E_USER_WARNING);
113120

@@ -146,6 +153,10 @@ public function stream_read(int $count)
146153
$this->eof = !$chunk->isTimeout();
147154
$this->eof = $chunk->isLast();
148155

156+
if ($chunk->isFirst()) {
157+
$this->response->getStatusCode(); // ignore 3/4/5xx
158+
}
159+
149160
if ('' !== $data = $chunk->getContent()) {
150161
if (\strlen($data) > $count) {
151162
if (null === $this->content) {
@@ -192,6 +203,10 @@ public function stream_seek(int $offset, int $whence = SEEK_SET): bool
192203
if (SEEK_END === $whence || $size < $offset) {
193204
foreach ($this->client->stream([$this->response]) as $chunk) {
194205
try {
206+
if ($chunk->isFirst()) {
207+
$this->response->getStatusCode(); // ignore 3/4/5xx
208+
}
209+
195210
// Chunks are buffered in $this->content already
196211
$size += \strlen($chunk->getContent());
197212

@@ -231,6 +246,13 @@ public function stream_cast(int $castAs)
231246

232247
public function stream_stat(): array
233248
{
249+
try {
250+
$headers = $this->response->getHeaders(false);
251+
} catch (ExceptionInterface $e) {
252+
trigger_error($e->getMessage(), E_USER_WARNING);
253+
$headers = [];
254+
}
255+
234256
return [
235257
'dev' => 0,
236258
'ino' => 0,
@@ -239,12 +261,16 @@ public function stream_stat(): array
239261
'uid' => 0,
240262
'gid' => 0,
241263
'rdev' => 0,
242-
'size' => (int) ($this->response->getHeaders(false)['content-length'][0] ?? 0),
264+
'size' => (int) ($headers['content-length'][0] ?? 0),
243265
'atime' => 0,
244-
'mtime' => strtotime($this->response->getHeaders(false)['last-modified'][0] ?? '') ?: 0,
266+
'mtime' => strtotime($headers['last-modified'][0] ?? '') ?: 0,
245267
'ctime' => 0,
246268
'blksize' => 0,
247269
'blocks' => 0,
248270
];
249271
}
272+
273+
private function __construct()
274+
{
275+
}
250276
}

‎src/Symfony/Component/HttpClient/Tests/HttpClientTestCase.php

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\HttpClient\Tests;
1313

14+
use Symfony\Component\HttpClient\Exception\ClientException;
1415
use Symfony\Component\HttpClient\Exception\TransportException;
1516
use Symfony\Contracts\HttpClient\Test\HttpClientTestCase as BaseHttpClientTestCase;
1617

@@ -52,9 +53,7 @@ public function testAcceptHeader()
5253
public function testToStream()
5354
{
5455
$client = $this->getHttpClient(__FUNCTION__);
55-
5656
$response = $client->request('GET', 'http://localhost:8057');
57-
5857
$stream = $response->toStream();
5958

6059
$this->assertSame("{\n \"SER", fread($stream, 10));
@@ -67,6 +66,22 @@ public function testToStream()
6766
$this->assertTrue(feof($stream));
6867
}
6968

69+
public function testToStream404()
70+
{
71+
$client = $this->getHttpClient(__FUNCTION__);
72+
$response = $client->request('GET', 'http://localhost:8057/404');
73+
$stream = $response->toStream(false);
74+
75+
$this->assertSame("{\n \"SER", fread($stream, 10));
76+
$this->assertSame('VER_PROTOCOL', fread($stream, 12));
77+
$this->assertSame($response, stream_get_meta_data($stream)['wrapper_data']->getResponse());
78+
$this->assertSame(404, $response->getStatusCode());
79+
80+
$this->expectException(ClientException::class);
81+
$response = $client->request('GET', 'http://localhost:8057/404');
82+
$stream = $response->toStream();
83+
}
84+
7085
public function testConditionalBuffering()
7186
{
7287
$client = $this->getHttpClient(__FUNCTION__);
@@ -83,4 +98,34 @@ public function testConditionalBuffering()
8398
$this->expectExceptionMessage('Cannot get the content of the response twice: buffering is disabled.');
8499
$response->getContent();
85100
}
101+
102+
public function testReentrantBufferCallback()
103+
{
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();
115+
}
116+
117+
public function testThrowingBufferCallback()
118+
{
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());
126+
127+
$this->expectException(TransportException::class);
128+
$this->expectExceptionMessage('Boo');
129+
$response->getContent();
130+
}
86131
}

‎src/Symfony/Component/HttpClient/Tests/MockHttpClientTest.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ protected function getHttpClient(string $testCase): HttpClientInterface
103103
case 'testBadRequestBody':
104104
case 'testOnProgressCancel':
105105
case 'testOnProgressError':
106+
case 'testReentrantBufferCallback':
107+
case 'testThrowingBufferCallback':
106108
$responses[] = new MockResponse($body, ['response_headers' => $headers]);
107109
break;
108110

0 commit comments

Comments
 (0)
0