search

tower-rs / tower-http

HTTP specific Tower utilities.
710 stars 165 forks source link

`FollowRedirect` cannot pass `Extensions` #152

Open quininer opened 3 years ago

quininer commented 3 years ago

Feature Request

Motivation

The currently redirected Request will not carry the Extensions of original Request, which may cause the http client to lose some context.

Proposal

Maybe we should modify the Policy trait so that clone_body becomes clone_request. Allow users to deal with Extensions that need to be inherited.

Alternatives

In the current implementation, users can take out Extensions from the first on_request and set it in the subsequent on_request. But this seems strange, and there is no docs to guarantee the stability of this behavior.

davidpdrsn commented 2 years ago

From https://github.com/tower-rs/tower-http/issues/125

@Nehliin @MarcusGrass and I (all work at embark) and have talked about this a bit. We think given the number of unanswered questions here not gonna include this in tower-http 0.2.

It feels having some answer to cloning requests would be useful for this as well, and also useful in general. So at some point we should research that.

davidpdrsn commented 1 year ago

I think changing clone_body to clone_request sounds look a good approach for now since general request cloning isn't really possible, and probably wont be for some time.

tesaguri commented 1 year ago

Changing clone_body to clone_request means that implementors of Policy will have to handle cloning of method, uri, version and headers, which seems a little burdensome for them. We may be able to provide convenience methods in PolicyExt that handles that task, but it's still unclear what the middleware should do if the policy modified, say, version of the request. Should the middleware restore original version? Or should we just trust the user not to do weird things on the request?

davidpdrsn commented 1 year ago

Hm good point. Maybe instead we could have a new method just for cloning the extensions you need.

This likely going to change in http 1.0 regardless (https://github.com/hyperium/http/pull/574) so maybe we should wait?