http-rs / surf

Fast and friendly HTTP client framework for async Rust
https://docs.rs/surf
Apache License 2.0
1.46k stars 119 forks source link

Enabling redirect middleware always sends two requests #307

Open jcrugzz opened 3 years ago

jcrugzz commented 3 years ago

Hey all, just started playing around with rust and ran into this issue. Even when there is no redirect code given in a response, I'm still seeing multiple requests being dispatched. Based on reading the code with my naive understanding, this might be expected currently? The middleware itself is generating a new request by cloning the original user defined request causing both to be dispatched.

Is this a limitation of the middleware and the ability to send partial requests as mentioned in the middleware? Is it assumed that the middleware itself is the only thing that should be dispatching requests and this is a bug? This happens regardless of there being a redirect code so I figure its related to the cloning of the request.

Just trying to understand a bit more about the machinery since it seems there needs to be a way to have response interception internally that isn't present in the middleware unless I'm still too naive.

Wishing you all well, thanks for all the good modules here in rust land.

Example code with some extra from my test code in case relevant:

use surf;
use cookie::{CookieJar,Cookie};

#[async_std::main]
async fn main() -> surf::Result<()> {
  let mut jar = CookieJar::new();
  let cook = Cookie::build("name", "value").finish();
  jar.add(cook);
  let cookies: String  = jar.iter().map(|cookie| cookie.to_string()).collect::<Vec<String>>().join("; ");

  // This currently sends multiple requests by default with the redirect
  // middleware enabled
  let req = surf::get("http://localhost:18000/auth/signin").header("cookie", cookies).build();
  let client = surf::client().with(surf::middleware::Redirect::default());
  let mut res = client.send(req).await?;

  dbg!(res.body_string().await?);
  Ok(())
}
Fishrock123 commented 3 years ago

Hey Jarrett, yeah this is expected right now. It is an unfortunate side effect of body streaming as well as some things regarding ownership. I'm not quite sure how other clients handle this but it could probably be improved.

the key things is we need to determine somehow how to either have the body stream be content addressable or know how much to wait for a 3XX before sending the body.

jesterfraud commented 1 year ago

I've been trying to debug failing OAuth2 authorization token requests, and this would be why. The caveat documented on the Redirect middleware doesn't highlight this as well as I'd like — my mind went to something more like CORS preflighting, where the first request won't cause side effects.

If possible, I'd like to have a version of this middleware that sacrifices maintaining the request body in favour of not issuing duplicate requests. It'd also be nice if there's a way for the logging middleware to indicate the source of the duplicate request, it would make debugging much easier.