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

Forwarding status of errors on streams #3303

Closed

Conversation

kazuho
Copy link
Member

@kazuho kazuho commented Dec 17, 2019

Clarifies that a tunnel might intentionally use an incomplete DATA frame to forward a malformed response, while preserving the partial response that has been transferred.

closes #3300.

@martinthomson martinthomson added -http design An issue that affects the design of the protocol; resolution requires consensus. labels Dec 17, 2019
@kazuho kazuho changed the title clarify the use of an incomplete DATA frame to forward a malformed partial message Clarify that tunnels can send malformed HTTP message, and it is not a connection-level error Dec 18, 2019
@kazuho
Copy link
Member Author

kazuho commented Dec 18, 2019

Seeing distaste for endorsing an intermediary to send a corrupt frame intentionally, I have changed the text to be a clarification of what we already have.

The paragraph that precedes the new paragraph being proposed in this PR states that, quote: "Intermediaries that process HTTP requests or responses (i.e., any intermediary
not acting as a tunnel) MUST NOT forward a malformed request or response."

The flip-side is that tunnels are allowed to send malformed HTTP messages. As a tunnel might be coalescing messages from multiple endpoints onto a single connection, receipt of a malformed HTTP message cannot be considered as a connection-level issue.

This PR clarifies that.

Copy link
Contributor

@janaiyengar janaiyengar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Editorial suggestion, but this new scoping helps -- thanks!

kazuho and others added 2 commits December 18, 2019 14:01
Co-Authored-By: Jana Iyengar <jri.ietf@gmail.com>
@martinthomson
Copy link
Member

Please fix the PR title.

@kazuho
Copy link
Member Author

kazuho commented Dec 20, 2019

@martinthomson Does something like "Receipt of malformed message is a stream-level error, as a tunnel might forward them" sounds OK to you?

@janaiyengar
Copy link
Contributor

@kazuho: I'll suggest "Clarify behavior for HTTP tunnels" as a better title

@martinthomson
Copy link
Member

Either works. "Forwarding status of errors on streams"

@kazuho kazuho changed the title Clarify that tunnels can send malformed HTTP message, and it is not a connection-level error Forwarding status of errors on streams Dec 23, 2019
@kazuho
Copy link
Member Author

kazuho commented Dec 23, 2019

@martinthomson Thank you for the suggestion. Done.

Copy link
Contributor

@ianswett ianswett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The more I look at this, the more I wonder what the full impact is. It ends up precluding a wide set of errors from becoming connection level errors in order to deal more gracefully with buggy implementations or inadvertent disconnects.

How about scoping this down only to cases when the stream is abruptly terminated(ie: reset) and not all the data was transferred.

@MikeBishop
Copy link
Contributor

@ianswett, that case is already covered by the current text. A stream can be reset mid-frame, and that's fine. The problem is that the contents might be invalid, or the intermediary might want to reliably deliver the terminated payload.

@martinthomson
Copy link
Member

I'm with Ian on this one. The new "MUST NOT" would force client implementations to bury a whole class of errors.

@kazuho
Copy link
Member Author

kazuho commented Jan 2, 2020

@ianswett @martinthomson Am I correct in assuming that you are arguing for prohibiting tunnels to forward malformed HTTP messages (e.g., those contain prohibited header fields)?

While I can see the beauty of such design, I am uncertain if that's a good approach. Use of an HTTP header field, which is end-to-end information, might make an HTTP message malformed. I wonder if an intermediary would be capable of detecting all such misuses. One extreme case that I can think of is an HTTP extension defining a new HTTP header field, that must not be sent as a trailer. Do we want an HTTP/3 intermediary to understand all the header fields that it forwards?

@martinthomson
Copy link
Member

I'm not suggesting that tunnels are forced to check what they forward, but that they assume responsibility for every byte they produce, forwarded or not.

An intermediary can forwards in the way you describe in that case. But if the source produces junk, the intermediary can wear the consequences. That doesn't mean that you have to check everything you get, but that's a call you would have to take on yourself.

The current text passes this cost on to clients in a way that will make it much harder to enforce protocol correctness at clients. That's not healthy.

@MikeBishop
Copy link
Contributor

I think our best path forward on this is to say that these errors MUST be treated as stream errors, and we retain the general policy that a stream error MAY be treated as a connection error.

@janaiyengar
Copy link
Contributor

I'm starting to like what @MikeBishop says here. There still is a problem that we can't restrict what a client ultimately does. If we could describe the cost of killing a connection, instead of requiring limiting the endpoint response, that might go a long way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-http design An issue that affects the design of the protocol; resolution requires consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Forwarding upstream errors, and the implications
6 participants