Closed 1ed closed 4 years ago
Can you try using the http_version option with an http URL? How does it behave?
I tried using dev-master
for symfony/http-client
$client = new AmpHttpClient(['http_version' => '2.0']);
$client = new CurlHttpClient(['http_version' => '2.0']);
$options = [
'headers' => [
'Connection' => 'Upgrade, HTTP2-Settings',
'Upgrade' => 'h2c',
'HTTP2-settings' => 'AAMAAABkAARAAAAAAAIAAAAA',
]
];
$response = $client->request('GET', 'http://nghttp2.org');
$response->getStatusCode();
dd($response->getInfo());
Without the headers in $options
itt falls back to HTTP/1.1 for both clients:
"response_headers" => array:14 [
0 => "HTTP/1.1 200 OK"
1 => "Date: Sat, 11 Apr 2020 06:38:35 GMT"
2 => "Content-Type: text/html"
3 => "Last-Modified: Fri, 15 Nov 2019 14:36:38 GMT"
4 => "Etag: "5dceb7f6-19d8""
5 => "Accept-Ranges: bytes"
6 => "Content-Length: 6616"
7 => "X-Backend-Header-Rtt: 0.003255"
8 => "Server: nghttpx"
9 => "Via: 2 nghttpx"
10 => "alt-svc: h3-23=":4433"; ma=3600"
11 => "x-frame-options: SAMEORIGIN"
12 => "x-xss-protection: 1; mode=block"
13 => "x-content-type-options: nosniff"
]
// ...
If I add the headers (curl does the same with --http2) so tell the server to upgrade to HTTP/2 amp client get an error
PHP Fatal error: Uncaught Error: Call to a member function close() on null in .../vendor/amphp/http-client/src/Connection/Http1Connection.php:229
Stack trace:
#0 [internal function]: Amp\Http\Client\Connection\Http1Connection->Amp\Http\Client\Connection\{closure}()
#1 .../vendor/amphp/amp/lib/Coroutine.php(118): Generator->send()
#2 .../vendor/amphp/amp/lib/Internal/Placeholder.php(149): Amp\Coroutine->Amp\{closure}()
#3 .../vendor/amphp/amp/lib/Deferred.php(52): class@anonymous->resolve()
#4 .../vendor/amphp/byte-stream/lib/ResourceInputStream.php(101): Amp\Deferred->resolve()
#5 .../vendor/amphp/amp/lib/Loop/NativeDriver.php(201): Amp\ByteStream\ResourceInputStream::Amp\ByteStream\{closure}()
#6 .../vendor/a in .../vendor/amphp/http-client/src/Connection/Http1Connection.php on line 229
but curl hangs for a while but got back the response but didn't upgrade
"response_headers" => array:3 [
0 => "HTTP/1.1 101 Switching Protocols"
1 => "Connection: Upgrade"
2 => "Upgrade: h2c"
]
// ...
In curl due to this https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpClient/CurlHttpClient.php#L146 HTTP/1.1 will be forced for 'http:' if I remove that the connection upgrades to HTTP/2
"response_headers" => array:17 [
0 => "HTTP/1.1 101 Switching Protocols"
1 => "Connection: Upgrade"
2 => "Upgrade: h2c"
3 => "HTTP/2 200 "
4 => "date: Sat, 11 Apr 2020 06:46:42 GMT"
5 => "content-type: text/html"
6 => "last-modified: Fri, 15 Nov 2019 14:36:38 GMT"
7 => "etag: "5dceb7f6-19d8""
8 => "accept-ranges: bytes"
9 => "content-length: 6616"
10 => "x-backend-header-rtt: 0.002924"
11 => "server: nghttpx"
12 => "via: 2 nghttpx"
13 => "alt-svc: h3-23=":4433"; ma=3600"
14 => "x-frame-options: SAMEORIGIN"
15 => "x-xss-protection: 1; mode=block"
16 => "x-content-type-options: nosniff"
]
// ...
If I hardcode $curlopts[CURLOPT_HTTP_VERSION] = CURL_HTTP_VERSION_2_PRIOR_KNOWLEDGE;
(the same as adding --http2-prior-knowledge
to the curl cli) I got HTTP/2 even without addig the $toptions
"response_headers" => array:14 [
0 => "HTTP/2 200 "
1 => "date: Sat, 11 Apr 2020 06:50:14 GMT"
2 => "content-type: text/html"
3 => "last-modified: Fri, 15 Nov 2019 14:36:38 GMT"
4 => "etag: "5dceb7f6-19d8""
5 => "accept-ranges: bytes"
6 => "content-length: 6616"
7 => "x-backend-header-rtt: 0.00356"
8 => "server: nghttpx"
9 => "via: 2 nghttpx"
10 => "alt-svc: h3-23=":4433"; ma=3600"
11 => "x-frame-options: SAMEORIGIN"
12 => "x-xss-protection: 1; mode=block"
13 => "x-content-type-options: nosniff"
]
// ...
Amp doesn't support h2 on http afaik, you should report there if you want to confirm. That L146: how does a second request look like after the first responded with an upgrade?
I don't know exactly it's done by curl internally I think it sends the same request again, but with http/2 without the upgrade headers.
Sorry, I meant is any request after the first one using h2 directly? If yes is this enough a fix?
Otherwise I'd suggest a new syntax to force a specific version. e.g. !2
Up for up PR, one way or another (or both)?
I did some digging, but it seems to me it does not work the way it should. It seems it does not reuse the connection.
When I create multiple requests with CLI curl it seems like this:
It does the upgrade once and reuses the connection.
But the same with this code:
$client = new CurlHttpClient(['http_version' => '2.0']);
$options = [
'headers' => [
'Connection' => 'Upgrade, HTTP2-Settings',
'Upgrade' => 'h2c',
'HTTP2-settings' => 'AAMAAABkAARAAAAAAAIAAAAA',
]
];
$responses = [];
foreach (['robots.txt', 'humans.txt'] as $i => $file) {
$uri = "http://nghttp2.org/$file";
$responses[] = $client->request('GET', $uri, 0 === $i ? $options : []);
}
/** @var ResponseInterface[] $responses */
foreach ($responses as $response) {
$response->getStatusCode();
dump($response->getInfo());
}
Is this code should work the same way as the former? I think yes, but maybe I'm missing something or doing it wrong.
But as I can't see a connection id or something like that so I checked with wireshark.
This is for the curl cli
the steam index is the same for all frames.
But for php it seems like this
the stream index is different so it creates 2 different connections for the requests.
OK, thanks, fix is in #36422
Actually, this works, but the log is blurred by multi-connections support.
You'll notice by creating the client with new CurlHttpClient(['http_version' => 2], 1)
, the 1
asking to open only one connection per host.
Is this good enough? On non-SSL stream, there is no network overhead related to the SSL handshake, so that the multi-connections behavior is fine I think. The h2 negotiation adds no overhead, isn't it?
Oh, thank you! The patch works great. I don't really know about the overhead, so you mean by that it does not worth it to add support for CURL_HTTP_VERSION_2_PRIOR_KNOWLEDGE
via !2
?
I think it's not worth it until proven otherwise yes.
Ok, fair enough. Thank you very much for your help and the patch!
@1ed Can you still reproduce the error you got with AmpHttpClient
or was this with an earlier version? Currently I get Amp\Http\Client\HttpException : CONNECT or upgrade request made without upgrade handler callback
, but with amphp/http-client
directly.
Nope, I get the same as you with the code I've posted earlier, after updating to
#...
- Updating symfony/http-client-contracts (v2.0.1 => v2.1.1): Loading from cache
- Updating symfony/polyfill-php80 (v1.15.0 => v1.17.0): Loading from cache
- Updating amphp/amp (v2.4.2 => v2.4.4): Loading from cache
- Updating amphp/sync (v1.3.0 => v1.4.0): Loading from cache
- Updating amphp/http-client (v4.2.2 => v4.3.1): Loading from cache
- Updating symfony/http-client dev-master (5052db2 => c530027)
# ...
@kelunik that means with an upgrade handler amphp can sand h2c requests too? Do you have an example?
@1ed Not sure whether we'll support upgrades by default, but I've just opened a PR to allow for h2c if HTTP/2 is the only set protocol version, which requires only a small change: https://github.com/amphp/http-client/pull/271
Description
As far as I know it the HTTP Client component does not support HTTP/2 without TLS (aka. h2c). It could be useful for calling private endpoints without the need of certificate management but use the benefits of HTTP/2.
There are 2 methods to do it:
HTTP/2 200 date: Fri, 10 Apr 2020 23:53:42 GMT content-type: text/html last-modified: Fri, 15 Nov 2019 14:36:38 GMT etag: "5dceb7f6-19d8" accept-ranges: bytes content-length: 6616 x-backend-header-rtt: 0.001454 server: nghttpx via: 2 nghttpx alt-svc: h3-23=":4433"; ma=3600 x-frame-options: SAMEORIGIN x-xss-protection: 1; mode=block x-content-type-options: nosniff