Redirect callback

[carloseltuerto]

For HTTP redirect, Cronet invokes a "User" callback method for each leg, and waits for the User to allow continuation (or cancel).

At first sight, Envoy-Mobile does not expose such mechanism. Here is the Cronet API for this:

https://chromium.googlesource.com/chromium/src/+/master/components/cronet/android/api/src/org/chromium/net/UrlRequest.java#133

[carloseltuerto]

Maybe implementing a Redirect Callback inside Envoy-Mobile is not necessary. The Cronvoy implementation could simply forbid any redirect internally, but start a new stream if the user accepts the redirect. At first sight, trying to hack router.cc accordingly seems rather heavy.

[carloseltuerto]

It turns out that https://github.com/envoyproxy/envoy-mobile/pull/1462 is an expedient solution. The native Chromium implementation of Cronet does leave the C++ layer handle the redirection.

And most callers do invoke "cancel" when they reject the redirection. Still, there is at least one exception in Google codebase - "cancel" is not invoked in such case. But I guess that the outcome will be similar with Cronet or Envoy-Mobile: the Engine will be dangling for a while, and then will clean up itself upon a timeout.

Conclusion: Envoy-Mobile will need to offer a redirection mechanism that is similar to the one offered by Cronet. FYI, here is how this is done with Cronet:

General rules:

• While waiting for a caller decision, the Engine can't invoke any callback.
• Before the Engine can invoke the onHeader() callback method, redirection(s) must have been solved.

State transition: edge: network msg -or- caller decision

                  ||    /========\
    First 30X ->  ||    ||      ||
                  \/    \/      ||
Engine-onRedirectReceived()     ||
  ||                    ||      ||   <- Subsequent 30X
  \/                    \/      ||
Caller-cancel()     Caller-followRedirect() 
                        ||          ||           
       20X, 40X, 50X -> ||          || <- bad DNS, internal failure, etc
                        \/          \/
            Engine-onHeader()    Engine-onError()

[alyssawilk]

So from an envoy perspective, I think we don't want to handle redirects in a vacuum, we want to consider how we're doing retries as well.

Envoy has a mechanism for doing redirects, where a filter (router or otherwise) can recreateStream(), and that new stream will handle the redirect, recreating and passing through all the various filters along the way. It'll only do this if the full request is buffered, and the buffer limits are the same for retries and redirects, which is why I think we want to consider them together. Server side, we tend to have pretty tight limits on how much we'll buffer for retries and redirects (like 2-4k) where client side I assume (though possibly incorrectly) we're going to want to buffer more data in the Envoy layers, to handle things like QUIC 0-rtt handshake failures, and some level of 5xx handling, but that's based on the assumption that Chrome/Cronet does some amount of retries on its own and that may be a mistaken assumption.

@carloseltuerto / @DavidSchinazi / @RyanTheOptimist can you share a bit of info cronet retries to confirm/deny that?

[alyssawilk]

FWIW I totally think it's fine to land https://github.com/envoyproxy/envoy-mobile/pull/1462 and just leave this as an open issue to address in the future to avoid double buffering for retries/redirects.

[RyanTheOptimist]

I'm not familiar enough with the nuances/details of Cronet's redirect handling to have a useful opinion here. But I'll bet @efimki might.

[efimki]

AFAIR Cronet does NOT do any retries outside of the normal retries that are done by underlying Chromium net stack. For the request buffering Cronet UploadDataProvider API has a 'Rewind()' method that is called on redirect to rewind the data.