reqwest icon indicating copy to clipboard operation
reqwest copied to clipboard

client/redirect: expose previous and next request methods

Open lucab opened this issue 1 year ago • 2 comments

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 https://github.com/seanmonstar/reqwest/issues/1777#issuecomment-2363279906.

lucab avatar Sep 23 '24 10:09 lucab

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.

seanmonstar avatar Sep 25 '24 14:09 seanmonstar

@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 https://github.com/seanmonstar/reqwest/issues/1777#issuecomment-2363279906 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 https://github.com/seanmonstar/reqwest/issues/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.

lucab avatar Sep 25 '24 16:09 lucab

Hey @seanmonstar, I was doing some spring cleaning and noticed that I still have this old PR pending. I've rebased it to current master, can we decide to either merge it or to close it?

lucab avatar Apr 15 '25 09:04 lucab

Thanks for the reminder. With #2576, providing something like this would need to be in tower-http. I can't say whether it should... I suppose if we think of redirects as a form of retry, then access to changing the next request sent is in the retry middleware...

seanmonstar avatar Apr 15 '25 18:04 seanmonstar

Ack, thanks for feedback. I'm thus closing this PR on hyper side.

I suppose if we think of redirects as a form of retry, then access to changing the next request sent is in the retry middleware...

Just for reference, what is the "retry middleware" you are referencing here? Are you maybe talking about the follow_redirect one at https://docs.rs/tower-http/latest/tower_http/follow_redirect/index.html?

lucab avatar Apr 16 '25 08:04 lucab