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

Customizable Redirect Policy #24

Closed
seanmonstar opened this issue Nov 22, 2016 · 4 comments
Closed

Customizable Redirect Policy #24

seanmonstar opened this issue Nov 22, 2016 · 4 comments

Comments

@seanmonstar
Copy link
Owner

No description provided.

@Arnavion
Copy link

@Arnavion mind adding exactly what you'd need to the issue?

Well, currently reqwest effectively has a redirect policy of FollowAll because it sets the underlying request policy to FollowNone and has a loop to always follow redirects. I want to be able to control which redirects get followed and which don't, which hyper does by letting me set a FollowIf(fn(&Url) -> bool) policy.

My crate interfaces with a web API that sometimes sends a redirect to a login page instead of a 401/403 response, so by preventing that redirect I'm able to report an equivalent error to the caller of my function instead of trying to deserialize an HTML page as JSON.

I also plan in the future to prevent redirects from taking me to hostnames outside of a whitelist to prevent abuse.

@seanmonstar
Copy link
Owner Author

I've felt that the RedirectPolicy in hyper was too limited, and wanted to use this opportunity to make a better one. To start, the struct won't have it's variants exposed, so new ones can be added. Also, other problems with RedirectPolicy::FollowIf are:

  • It must be a standard function, so setting configuration variables at startup and using them is not possible
  • It is not possible to check for infinite redirects
  • It is not possible to set a number limit

Looking at other implementations for inspiration, there is this in golang's http package:

CheckRedirect func(req *Request, via []*Request) error

This allows a user to a) count the number of previous requests, b) check for duplicate URLs, among other things. If I understand Go enough, I believe that also means the CheckRedirect function could even modify the *Request, but I don't know if that's something desirable or not.


So, the design I've been thinking of looks something like this:

trait Redirect {
    fn redirect(&self, next: &Url, previous: &[Url]) -> bool;
}

impl Client {
    pub fn set_redirect_policy<T: Redirect>(&mut self, policy: T) {
        self.redirect_policy = Box::new(policy);
    }
}

The Redirect trait could perhaps receive Request instead of Urls, if there is use in inspecting the Headers also.

There could be a couple built-ins, such as:

  • RedirectPolicy::default() - max redirects 10, look for loops
  • RedirectPolicy::none() - always return false

It would also allow custom policies like this:

let max_redirects = config.load('max_redirects').unwrap_or(5);
client.set_redirect_policy(move |_next, previous| {
    previous.len() >= max_redirects    
})

@Arnavion
Copy link

That sounds good. Maybe you can also add operators to combine redirect policies but that can be done in user code as well.

@seanmonstar
Copy link
Owner Author

I have an implementation in #29.

I ended up not having a trait Redirect, because the compiler had issues infer lifetimes of the &Url and &[Url] arguments passed to closures. Otherwise, it does what was suggested in my previous comment.

repi pushed a commit to EmbarkStudios/reqwest that referenced this issue Dec 20, 2019
Fix broken examples on Readme.md, add examples
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

2 participants