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

FollowRedirect cannot pass Extensions #152

Open
quininer opened this issue Oct 17, 2021 · 4 comments
Open

FollowRedirect cannot pass Extensions #152

quininer opened this issue Oct 17, 2021 · 4 comments
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Experience needed to fix: Medium / intermediate

Comments

@quininer
Copy link
Contributor

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 davidpdrsn added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Nov 8, 2021
@davidpdrsn davidpdrsn added this to the 0.2.0 milestone Nov 8, 2021
@davidpdrsn davidpdrsn removed this from the 0.2.0 milestone Nov 25, 2021
@davidpdrsn
Copy link
Member

From #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 davidpdrsn added E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Experience needed to fix: Medium / intermediate labels Dec 2, 2022
@davidpdrsn davidpdrsn added this to the 0.4 milestone Dec 2, 2022
@davidpdrsn
Copy link
Member

davidpdrsn commented Dec 2, 2022

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
Copy link
Contributor

tesaguri commented Dec 2, 2022

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
Copy link
Member

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 (hyperium/http#574) so maybe we should wait?

@davidpdrsn davidpdrsn removed this from the 0.4 milestone Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Experience needed to fix: Medium / intermediate
Projects
None yet
Development

No branches or pull requests

3 participants