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

client/redirect: expose previous and next request methods #2430

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lucab
Copy link
Contributor

@lucab lucab commented Sep 23, 2024

This augments redirect::Attempt in order to expose two relevant HTTP methods during redirects:

  • the methods for the previous request.
  • the method to be performed on the next request upon following a redirection.

The two methods can different, notably when redirecting POST requests:
https://en.wikipedia.org/wiki/Post/Redirect/Get

Towards #1777 (comment).

This augments `redirect::Attempt` in order to expose two
relevant HTTP methods during redirects:
 - the methods for the previous request.
 - the method to be performed on the next request upon
   following a redirection.

The two methods can different, notably when redirecting `POST`
requests.

Ref: https://en.wikipedia.org/wiki/Post/Redirect/Get
@seanmonstar
Copy link
Owner

This is certainly an interesting idea! I'm not sure which way I lean here, I suppose this could be a way to mutate the next method?

I'd more quickly merge a fix to only change the method from POST, as the spec seems to outline.

@lucab
Copy link
Contributor Author

lucab commented Sep 25, 2024

@seanmonstar oh sorry, I see now that my PR description is probably too confusing.
This is only meant to cover the comment from @konstin at #1777 (comment) in order to provide a way to stop following such redirects. The "previous method" is required for such a decision (at least), and I thought that adding the "next method" could be useful too (but I can drop that if you prefer, it can be re-computed from the response code). This doesn't need to be a mutating hook, the current follow/stop policy is enough.

This is not going to close the whole #1777, only tackling one specific follow-up comment.
I don't have any urgent need to fix the rest of the ticket, but if you are ok with following the fetch spec I can try to find some time to assemble a PR to align reqwest with that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants