envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
24.85k stars 4.78k forks source link

http: propagate correct reset codes #13349

Open kpr12 opened 4 years ago

kpr12 commented 4 years ago

Hi, we have the following situation: Envoy has received an HTTP request from the client and forwarded that request to the upstream server. While waiting for the HTTP respone from the server, Envoy receives from the client a RST_STREAM packet with error code "CANCEL". This triggers Envoy to send a RST_STREAM packet with a different error code "NO_ERROR" upstream to the server. My question: Why does Envoy not send the same error code to the upstream server that it had received from the client? And would it be possible to configure Envoy to send the same error code as received from the client?

mattklein123 commented 4 years ago

I know this has been asked for but I'm not sure if there is an existing issue tracking. The main issue is that we don't propagate reset reasons from downstream to upstream and vice versa today. I don't think this would be extremely difficult to do, but not sure if it's worth it or not. cc @alyssawilk

alyssawilk commented 4 years ago

Yeah, I think we can do better with reset codes in general, propagating where it makes sense and including details in logs. I think it's just a question of someone taking the time to do the work.

kanongil commented 3 years ago

This can definitely be improved. I have seen cases where a downstream client receives RST_STREAM with code 0 (NO_ERROR), when only part of the upstream data has been delivered. This is a serious issue, as without a content-length header, or application level checks (eg. checksum or structural checks), there is no way for a client to know that it received a partial response.

github-actions[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

kanongil commented 3 years ago

No one is interested in fixing this payload corruption issue?

kanongil commented 3 years ago

Btw. this is probably a worser issue than it appears, since he standard nghttp2 client does not report transmission errors for partial responses where the content-length is more than actually transferred, and the server closes the stream with RST_STREAM code 0. See https://github.com/nghttp2/nghttp2/pull/1508 for details and a potential client fix.