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

Transfer-Encoding value is case-insensitive by RFC. #10041

Closed
veshij opened this issue Feb 13, 2020 · 4 comments · Fixed by #10055
Closed

Transfer-Encoding value is case-insensitive by RFC. #10041

veshij opened this issue Feb 13, 2020 · 4 comments · Fixed by #10055
Labels
area/http beginner Good starter issues! bug help wanted Needs help!

Comments

@veshij
Copy link
Contributor

veshij commented Feb 13, 2020

Envoy rejects requests having "Transfer-Encoding: Chunked" with unsupported transfer encoding error.

Per RFC7230: Hypertext Transfer Protocol (HTTP/1.1): Message Syntax and Routing transfer-coding names are case-insensitive:

All transfer-coding names are case-insensitive and ought to be
registered within the HTTP Transfer Coding registry, as defined in
Section 8.4. They are used in the TE (Section 4.3) and
Transfer-Encoding (Section 3.3.1) header

Expected behavior: all transfer-encoding comparisons are case-insensitive.

Logs:

D0213 02:23:06.031513 33976 http [external/envoy/source/common/http/conn_manager_impl.cc:278] [C516843051] dispatch error: http/1.1 protocol error: unsupported transfer encoding
@zuercher zuercher added area/http beginner Good starter issues! bug help wanted Needs help! labels Feb 13, 2020
@veshij
Copy link
Contributor Author

veshij commented Feb 13, 2020

I can try to take a look how to fix that.

@mattklein123
Copy link
Member

Trivial fix here:

if (current_headers.TransferEncoding()) {
absl::string_view encoding = current_headers.TransferEncoding()->value().getStringView();
if (Runtime::runtimeFeatureEnabled(
"envoy.reloadable_features.reject_unsupported_transfer_encodings") &&
encoding != Headers::get().TransferEncodingValues.Identity &&
encoding != Headers::get().TransferEncodingValues.Chunked) {
error_code_ = Http::Code::NotImplemented;
sendProtocolError();
throw CodecProtocolException("http/1.1 protocol error: unsupported transfer encoding");
}
}

@veshij
Copy link
Contributor Author

veshij commented Feb 13, 2020

@mattklein123 should we normalize transfer-encoding while parsing headers - otherwise it's possible to hit the same issue in other codepaths?

@mattklein123
Copy link
Member

We haven't normalized header values so far to my knowledge. I'm not sure if it's worth it but I could go either way. @alyssawilk any thoughts?

veshij added a commit to veshij/envoy that referenced this issue Feb 13, 2020
jenkins-stash-sync bot pushed a commit to Cray-HPE/cray-istio that referenced this issue Apr 30, 2021
The charts are updated for Istio 1.7.8. Here's a summary of the
changes.

* cray-istio-operator

I updated the charts/istio-operator subchart from the 1.7.8
distribution.

I got rid of the wait-jobs hook because this isn't necessary since Helm
takes care of making sure the CRD is ready.

On a related note, I found that when I upgraded from Istio 1.6.13 that
Helm deleted the IstioOperator CRD. To work around this, there's an
upgrade hook that recreates the IstioOperator CRD if it doens't exist.

* cray-istio-deploy

Just changed the default image tags and removed things from the README
that weren't accurate.

* cray-istio

I updated the charts/istio and charts/ingressgatewayhmn subcharts with
the latest version from the 1.7.8 distribution.

I removed the transfer encoding workaround ( CASMPET-3079 ). The
upstream bug is fixed: envoyproxy/envoy#10041

I removed the tcp-stats-filter-1.6 memory leak workaround ( CASMPET-4026 ).
Ths upstream bug is fixed: istio/istio#24720

The istio-ingressgateway now needs `runAsRoot: true`, see
https://istio.io/latest/news/releases/1.7.x/announcing-1.7/upgrade-notes/#gateways-run-as-non-root .
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/http beginner Good starter issues! bug help wanted Needs help!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants