Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HttpClient] fix support for 103 Early Hints and other informational status codes #33391

Merged
merged 1 commit into from Sep 3, 2019
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -20,8 +20,8 @@
*/
class DataChunk implements ChunkInterface
{
private $offset;
private $content;
private $offset = 0;
private $content = '';
This conversation was marked as resolved by nicolas-grekas

This comment has been minimized.

Copy link
@ntzm

ntzm Aug 30, 2019

what's the benefit of adding these when they are in the constructor?

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Aug 30, 2019

Author Member

not having to call the constructor in child classes if defaults are appropriate, see InformationalChunk

This comment has been minimized.

Copy link
@ntzm

ntzm Aug 30, 2019

makes sense 👍


public function __construct(int $offset = 0, string $content = '')
{
@@ -53,6 +53,14 @@ public function isLast(): bool
return false;
}

/**
* {@inheritdoc}
*/
public function getInformationalStatus(): ?array
{
return null;
}

/**
* {@inheritdoc}
*/
@@ -65,6 +65,15 @@ public function isLast(): bool
throw new TransportException($this->errorMessage, 0, $this->error);
}

/**
* {@inheritdoc}
*/
public function getInformationalStatus(): ?array
{
$this->didThrow = true;
throw new TransportException($this->errorMessage, 0, $this->error);
}

/**
* {@inheritdoc}
*/
@@ -0,0 +1,35 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\HttpClient\Chunk;

/**
* @author Nicolas Grekas <p@tchwork.com>
*
* @internal
*/
class InformationalChunk extends DataChunk
{
private $status;

public function __construct(int $statusCode, array $headers)
{
$this->status = [$statusCode, $headers];
}

/**
* {@inheritdoc}
*/
public function getInformationalStatus(): ?array
{
return $this->status;
}
}
@@ -13,6 +13,7 @@

use Psr\Log\LoggerInterface;
use Symfony\Component\HttpClient\Chunk\FirstChunk;
use Symfony\Component\HttpClient\Chunk\InformationalChunk;
use Symfony\Component\HttpClient\Exception\TransportException;
use Symfony\Component\HttpClient\Internal\CurlClientState;
use Symfony\Contracts\HttpClient\ResponseInterface;
@@ -311,8 +312,11 @@ private static function parseHeaderLine($ch, string $data, array &$info, array &
return \strlen($data);
}

// End of headers: handle redirects and add to the activity list
// End of headers: handle informational responses, redirects, etc.

if (200 > $statusCode = curl_getinfo($ch, CURLINFO_RESPONSE_CODE)) {
$multi->handlesActivity[$id][] = new InformationalChunk($statusCode, $headers);

return \strlen($data);
}

@@ -339,7 +343,7 @@ private static function parseHeaderLine($ch, string $data, array &$info, array &

if ($statusCode < 300 || 400 <= $statusCode || curl_getinfo($ch, CURLINFO_REDIRECT_COUNT) === $options['max_redirects']) {
// Headers and redirects completed, time to get the response's body
$multi->handlesActivity[$id] = [new FirstChunk()];
$multi->handlesActivity[$id][] = new FirstChunk();

if ('destruct' === $waitFor) {
return 0;
@@ -45,7 +45,7 @@ class MockResponse implements ResponseInterface
public function __construct($body = '', array $info = [])
{
$this->body = is_iterable($body) ? $body : (string) $body;
$this->info = $info + $this->info;
$this->info = $info + ['http_code' => 200] + $this->info;

if (!isset($info['response_headers'])) {
return;
@@ -59,7 +59,8 @@ public function __construct($body = '', array $info = [])
}
}

$this->info['response_headers'] = $responseHeaders;
$this->info['response_headers'] = [];
self::addResponseHeaders($responseHeaders, $this->info, $this->headers);
}

/**
@@ -17,8 +17,6 @@

/**
* @author Nicolas Grekas <p@tchwork.com>
*
* @internal
*/
final class ResponseStream implements ResponseStreamInterface
{
@@ -15,6 +15,8 @@
use Symfony\Component\HttpClient\MockHttpClient;
use Symfony\Component\HttpClient\NativeHttpClient;
use Symfony\Component\HttpClient\Response\MockResponse;
use Symfony\Component\HttpClient\Response\ResponseStream;
use Symfony\Contracts\HttpClient\ChunkInterface;
use Symfony\Contracts\HttpClient\HttpClientInterface;
use Symfony\Contracts\HttpClient\ResponseInterface;

@@ -122,6 +124,41 @@ protected function getHttpClient(string $testCase): HttpClientInterface
$body = ['<1>', '', '<2>'];
$responses[] = new MockResponse($body, ['response_headers' => $headers]);
break;

case 'testInformationalResponseStream':
$client = $this->createMock(HttpClientInterface::class);
$response = new MockResponse('Here the body', ['response_headers' => [
'HTTP/1.1 103 ',
'Link: </style.css>; rel=preload; as=style',
'HTTP/1.1 200 ',
'Date: foo',
'Content-Length: 13',
]]);
$client->method('request')->willReturn($response);
$client->method('stream')->willReturn(new ResponseStream((function () use ($response) {
$chunk = $this->createMock(ChunkInterface::class);
$chunk->method('getInformationalStatus')
->willReturn([103, ['link' => ['</style.css>; rel=preload; as=style', '</script.js>; rel=preload; as=script']]]);

yield $response => $chunk;

$chunk = $this->createMock(ChunkInterface::class);
$chunk->method('isFirst')->willReturn(true);

yield $response => $chunk;

$chunk = $this->createMock(ChunkInterface::class);
$chunk->method('getContent')->willReturn('Here the body');

yield $response => $chunk;

$chunk = $this->createMock(ChunkInterface::class);
$chunk->method('isLast')->willReturn(true);

yield $response => $chunk;
})()));

return $client;
}

return new MockHttpClient($responses);
@@ -20,4 +20,9 @@ protected function getHttpClient(string $testCase): HttpClientInterface
{
return new NativeHttpClient();
}

public function testInformationalResponseStream()
{
$this->markTestSkipped('NativeHttpClient doesn\'t support informational status codes.');
}
}
@@ -21,7 +21,7 @@
"require": {
"php": "^7.1.3",
"psr/log": "^1.0",
"symfony/http-client-contracts": "^1.1.6",
"symfony/http-client-contracts": "^1.1.7",
"symfony/polyfill-php73": "^1.11"
},
"require-dev": {
@@ -47,6 +47,13 @@ public function isFirst(): bool;
*/
public function isLast(): bool;

/**
* Returns a [status code, headers] tuple when a 1xx status code was just received.
*
* @throws TransportExceptionInterface on a network error or when the idle timeout is reached
*/
public function getInformationalStatus(): ?array;

/**
* Returns the content of the response chunk.
*
@@ -754,6 +754,27 @@ public function testInformationalResponse()
$this->assertSame(200, $response->getStatusCode());
}

public function testInformationalResponseStream()
{
$client = $this->getHttpClient(__FUNCTION__);
$response = $client->request('GET', 'http://localhost:8057/103');

$chunks = [];
foreach ($client->stream($response) as $chunk) {
$chunks[] = $chunk;
}

$this->assertSame(103, $chunks[0]->getInformationalStatus()[0]);
$this->assertSame(['</style.css>; rel=preload; as=style', '</script.js>; rel=preload; as=script'], $chunks[0]->getInformationalStatus()[1]['link']);
$this->assertTrue($chunks[1]->isFirst());
$this->assertSame('Here the body', $chunks[2]->getContent());
$this->assertTrue($chunks[3]->isLast());
$this->assertNull($chunks[3]->getInformationalStatus());

$this->assertSame(['date', 'content-length'], array_keys($response->getHeaders()));
$this->assertContains('Link: </style.css>; rel=preload; as=style', $response->getInfo('response_headers'));
}

/**
* @requires extension zlib
*/
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.