client-common
client-common copied to clipboard
php-http
DecoderPlugin - cURL 8.1.0 issue with HTTP/2
We got an issue reported over at https://github.com/getsentry/sentry-php, that cURL 8.1.0 and up no longer work as expected when using the DecoderPlugin.
See https://github.com/getsentry/sentry-php/issues/1537 and https://github.com/curl/curl/issues/11175.
From @krowinski
The problem is in \Http\Client\Common\Plugin\DecoderPlugin::handleRequest as is adding header 'TE'
$request = $request->withHeader('TE', $encodings);
with values
"TE" => array:3 [ 0 => "gzip" 1 => "deflate" 2 => "chunked" ]
And RFC for HTTP/2 specifies The only exception to this is the TE header field, which MAY be present in an HTTP/2 request; when it is, it MUST NOT contain any value other than "trailers"."
https://httpwg.org/specs/rfc9113.html#rfc.section.8.2.2
Its connected to curl lib as on 8.1.0 we got error
curl -X POST https://google.com -d '' -H "TE: gzip"
curl: (56) HTTP/2 stream 1 was reset
and before is was ok
curl -X POST https://google.com -d '' -H "TE: gzip"
<!DOCTYPE html>
But still php http client should not add invalid headers as curl seems to be more strict(?)
According to https://github.com/curl/curl/issues/11175#issuecomment-1559739707 it is
Hmmm. I am not so sure this is a bug here. We have an decoder plugin, but it is not used by default. Sentry are the ones who are adding it.
Looking at other repos that use this repo, they typically do not use this plugin.
We indeed use this plugin directly, https://github.com/getsentry/sentry-php/blob/master/src/HttpClient/HttpClientFactory.php#L102.
i am not too familiar with HTTP/2 nor the decoder plugin. the code happens here: https://github.com/php-http/client-common/blob/676f98fb3785d2c9e1b16aa24d9fcc39066f3f06/src/Plugin/DecoderPlugin.php#L54
if the HTTP/2 forbids this header, is the plugin still useful and needed? if not, maybe change the sentry setup to not include the plugin when HTTP/2 is used. if the plugin does useful things for HTTP/2, could we make the plugin detect if the request is a HTTP/2 request and in that case not add the header or with the trailers value if that is needed? at which point does the client know whether it is initiating a HTTP/2 request and not a 1.x request?
We worked around this issue by enforcing HTTP/1.1 for now. I still suggest finding a proper solution, as we will be one of many running into this issue.
sure, happy to merge a proper solution if somebody can find out how that solution should look like.
We are now seeing this issue within our Symfony app, how would one go about configuring sentry to not use the decoder.
Seems within symfony you can disable compression which will stop sentry from using this plugin.