-
Notifications
You must be signed in to change notification settings - Fork 54
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
DecoderPlugin - cURL 8.1.0 issue with HTTP/2 #230
Comments
Is this issue present in curl 8.1.1? |
According to curl/curl#11175 (comment) 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:
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 |
We worked around this issue by enforcing HTTP/1.1 for now. |
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. |
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 getsentry/sentry-php#1537 and curl/curl#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
and before is was ok
But still php http client should not add invalid headers as curl seems to be more strict(?)
The text was updated successfully, but these errors were encountered: