envoyproxy / envoy

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

Proxy multiple 1xx responses #19044

Open alyssawilk opened 3 years ago

alyssawilk commented 3 years ago

See https://github.com/envoyproxy/envoy/issues/18844

zhxie commented 2 years ago

Hi, I would like to take a try.

As mentioned here, I would like to break the issue apart,

  1. Fix the missing 102/103 HTTP status code, since the Envoy returns 102 Unknown back to the downstream,
  2. Pass through multiple 1xx response, and
  3. Clear response timer every time when 1xx comes, since the Envoy will returns 504 Gateway Timeout even there is continuous 102 Processings coming back from the upstream.

You mentioned it will be a complicated work and we only have one slot for buffering 1xxs. Could you explain it a bit more?

alyssawilk commented 2 years ago

hm, this is very much not a beginner issue, but you're welcome to try.

The easy part is changing conn_manager_impl.cc continueHeader to return a vector of headers. The nasty part would be changing the filter_manager.* code to handle multiple 1xx headers. You can possibly crib off how metadata is handled there as I believe it correctly handles multiple entries.

once you've done both of those, you'd want an integration test of the "happy path", just sending 2 1xx headers through, and then all the filter stop and start cases which again you can probably crib off the metadata tests.

zhxie commented 2 years ago

There are many many assert and if barriers in the code to prevent proxying 1xx twice or encoding 1xx headers twice or resetting the informational headers though they are not been used other than encoding 1xx headers. It is easy to remove these barriers, and in my HTTP/1.1 test, Envoy works normally and proxy multiple 1xx reponses back successfully. But the most difficult part to me is that, as you said, this is a very complex region with quite a complicated code logic. I may overlook some points that might cause potential issues. There are a lot of discussions (like #11433) in the community before and the previous decision is to ignore multiple 1xx headers. So I am worried about that it may has a very large and complicated work to do, like the compatibility to existing filters. Do you have some considerations that can provide to me to avoid going down the wrong ways.

alyssawilk commented 2 years ago

right, it's pretty easy to proxy multiple 100s in the happy path, where there is no filter pausing, but if a filter pauses the chain, multiple 100s have to be cached and resumed, and that's where the complexity is. I'm inclined to think that doing a half solution (where it works when not backed up, but consolidates when backed up) is worse than clear lack of support but you could ask for a second opinion from Matt.

SuperOleg39 commented 1 year ago

This feature is important for Early Hints, because with Server-Side Rendering request to page HTML will usually has several sequential stages of processing, for example:

And usually you can have information about all important page resources only at the last stage - i.e. practically before the response, not so much benefits for 13 code.

But part of resources we know before and can send at early stages, and speed up page loading for seconds at client-side.

Of course this may all be a slightly specific example, but in any case it does not violate the early hints specification)

alyssawilk commented 1 year ago

totally makes sense, but Envoy features are largely driven by the features that contributing companies need, and so far there hasn't been enough demand for this that anyone has signed on to do the work. cc @ianswett as the only person at my company who might know of any internal demand for proper early hints support.

ianswett commented 1 year ago

There's definitely interest from my teams, including @diannahu on Early Hints, so I'd love to see this supported.

Now that HTTP/2 Server Push is deprecated, Early Hints is the suggested replacement, and Chrome supports it today: https://developer.chrome.com/blog/early-hints/

We've seen a few use cases where Early Hints could be very helpful in the last few years.

In the case of Early Hints, I believe consolidating the 100 responses in the case of a filter pause would be better than nothing, but I'm not sure if that's true for other 1xx use cases. Is it feasible to make this default disabled and if someone enables it, they need to understand the limitations of the feature?

alyssawilk commented 1 year ago

It's not just about filter pause, it's also about blocked downstream HCM - basically any time you don't immediately ship headers you need to handle multiple. I think if Google wants support we should just add support for multiple non-coalesced headers, using metadata as a model.

ianswett commented 1 year ago

Metadata does seem like it may be a good model in this case.

Nealsoni00 commented 5 months ago

Hey all! I was wondering if there was any thoughts around 104 informational responses being supported.

They are currently in discussions for use with resumable uploads ietf rfc. I currently have a use case that requires the 104 response and it seems like adding support is not too difficult.

If I imitated the introduced pattern in 19023 for 104 responses, would there be objections to it's introduction?