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

Add Proxy support to Client #531

Closed
seanmonstar opened this issue May 15, 2015 · 23 comments
Closed

Add Proxy support to Client #531

seanmonstar opened this issue May 15, 2015 · 23 comments
Assignees
Labels
A-client Area: client.

Comments

@seanmonstar
Copy link
Member

I imagine some sort of Proxy type being added a property of Client, with a corresponding set_proxy(Option<Proxy>) method.

Use of a proxy can have different requirements: all requests, only https requests, only http... So it would probably mean Proxy would be an enum... and the proxy would be consulted in the RequestBuilder.send method.

@messense
Copy link
Contributor

messense commented Jul 2, 2015

It would be awesome to have this feature!

@shaleh
Copy link

shaleh commented Sep 16, 2015

Yes please. Some of us need this to use Rust at work.

@shaleh
Copy link

shaleh commented Sep 16, 2015

Something like:

let mut client = Client::new();
// after checking config, env for HTTP_PROXY, whatever
let proxy = Proxy::new(server, port, scheme);  // AuthProxy::new for the more complicated cases
client.add_proxy(proxy);  // set_proxy, which ever. Bike shed as needed
// now a request using 'scheme' will be proxied
client.get(....)

@mattnenterprise
Copy link
Contributor

I would like to give this a try.

@mattnenterprise
Copy link
Contributor

@seanmonstar Could you give more information on how you think this should be implemented?

@seanmonstar
Copy link
Member Author

@mattnenterprise an api suggested by @shaleh looks fine. It'd likely require adding a method to Request to change the RequestUri, since the full url is needed instead of just the path.

@shaleh
Copy link

shaleh commented Sep 23, 2015

Note the requirement for a list of proxies. When the final HTTP call is made it should check the request's scheme against defined proxies for a matching scheme.

@shaleh
Copy link

shaleh commented Sep 23, 2015

Since Rust does not support optional arguments I used AuthProxy in my example for the case where the proxy needs authentication. If this is too cumbersome using Option for the credentials is probably OK.

@shaleh
Copy link

shaleh commented Sep 23, 2015

I think using a container for the proxies avoids the need for Option<Proxy>. Just walk it and compare schemes. Since the list will only every be a few items long it is sane to walk it. No need for a hash or other lookup table.

@seanmonstar
Copy link
Member Author

There's a PR for this at #639, if others interested in this feature could help looking it over.

@winding-lines
Copy link
Contributor

Hello everybody, I have been working on the proxy implementation for a while (on and off) but I missed this conversation :-) I will evolve my PR based on this feedback.

@kikokikok
Copy link

Hi All,
I've just been tracking down the reason why multirust-rs is not able to work behind a proxy, It seems like the Http Client used in multirust is the Hyper one which leads me to this discussion and the associated PR.

Do you have an ETA on this one ? it seems like it hasn't evolved since October. Any chance to see this merged or is it still not mature enough.
I see a lot of value in having this functionality built-in Hyper as it serves a lot of tools/libs in the ecosystem and could clearly help to drive Enterprise adoption.

Thanks !

@mitsuhiko
Copy link
Contributor

I'm in the same boat. I really need proxy support. Is there any chance I can help out with this?

@mhristache
Copy link

Any updates on this?

@seanmonstar
Copy link
Member Author

My focus is entirely on the async branch, which makes this easy:

struct ProxiedHandler(hyper::Method, &'static str);

impl Handler for ProxiedHandler {
    fn on_request(&mut self, req: &mut Request) -> Next {
        req.set_method(self.0);
        req.set_uri(RequestUri::AbsoluteUri(self.1.parse().unwrap());
        Next::read()
    }
    // ...
}
client.request("http://proxy.server", ProxiedHandler(GET, "http://target.domain/path")); 

I can try to add in some support to the sync branch, such that if the Request.url does not match the Host header, then assume it is a proxied request and send the full URI...

seanmonstar added a commit that referenced this issue Apr 25, 2016
This works by configuring proxy options on a `Client`, such as
`client.set_proxy("http", "127.0.0.1", "8018")`.

Closes #531
seanmonstar added a commit that referenced this issue Apr 25, 2016
This works by configuring proxy options on a `Client`, such as
`client.set_proxy("http", "127.0.0.1", "8018")`.

Closes #531
@seanmonstar seanmonstar self-assigned this Apr 25, 2016
@seanmonstar
Copy link
Member Author

See #771 to add simple support to the sync branch. This will be released in hyper 0.9.2.

@shaleh this doesn't add support for lists, or Proxy-Authorization or the like. I believe those can be handled outside of hyper. The parts fixed in this PR were things that could not be done without hyper's help.

@shaleh
Copy link

shaleh commented Apr 25, 2016

No worries, I do not need either lists or Proxy-Authorization.

@JeffBelgum
Copy link

JeffBelgum commented Feb 24, 2017

@seanmonstar I am in need of Proxy-Authorization for a project at work. In a comment above, you suggest that it could be handled outside of hyper. Would you be able to provide some guidance as to how to do this? Would it involve creating a new type implementing NetworkConnector or some other approach?

@seanmonstar
Copy link
Member Author

@JeffBelgum possibly. When exactly should the header be sent, as part of the normal request, or as part of the CONNECT tunnel request?

@JeffBelgum
Copy link

It's send as part of the CONNECT tunnel request.

@seanmonstar
Copy link
Member Author

Then it seems that would require a new NetworkConnector, as the current Proxy doesn't have a way to customize the headers sent: https://github.com/hyperium/hyper/blob/0.10.x/src/client/proxy.rs#L47

It probably should, and I think it can be done in a backwards compatible way, by adding a method to ProxyConfig, and then making use of that in the Proxy.

@JeffBelgum
Copy link

JeffBelgum commented Feb 24, 2017

Great, thanks for the help! Would you be open to a PR against the 10.x branch that implements proxy authorization in the current Proxy struct?

@seanmonstar
Copy link
Member Author

@JeffBelgum for increased flexibility, I'd probably suggest allowing the ProxyConfig to accept a Headers struct, that is applied to each CONNECT request.

Some proxies use different headers for auth, and may or may not require other headers like User-Agent. Best to just let people configure any header.

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

No branches or pull requests

9 participants