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

Enabling redirect middleware always sends two requests #307

Open
jcrugzz opened this issue Jun 19, 2021 · 2 comments
Open

Enabling redirect middleware always sends two requests #307

jcrugzz opened this issue Jun 19, 2021 · 2 comments

Comments

@jcrugzz
Copy link

jcrugzz commented Jun 19, 2021

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

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.

rjframe added a commit to rjframe/b2-client that referenced this issue Oct 14, 2021
Surf is still sending requests to B2 even with surf-vcr intercepting
them; this is likely upstream's issue 307[1], but I need to debug to be
certain.

This commit works around the problem by using localhost as our server.
The strange thing is that surf does not appear to always send the
requests, and I have not been able to observe any requests when
listening via netcat.

[1]: http-rs/surf#307
@jesterfraud
Copy link

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.

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

No branches or pull requests

3 participants