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 HTTP proxy support #401

Closed
brson opened this issue May 6, 2016 · 10 comments
Closed

Add HTTP proxy support #401

brson opened this issue May 6, 2016 · 10 comments

Comments

@brson
Copy link
Contributor

brson commented May 6, 2016

With hyper 0.9.2 you can call Client::with_http_proxy(host, port) to set up the HTTP proxy.
I'm not sure how I want to implement this yet, whether there are standard ways to indicate a proxy should be used or not. Whatever the solution it needs to work during the initial install as well as after installation.

cc #344
cc @seanmonstar

@xen0n
Copy link
Contributor

xen0n commented May 8, 2016

Considering the typical rustup invocation is done the curl | sh way, I think the environment variables http_proxy and/or https_proxy are preferable as the same settings are honored by curl as well. Plus it's the de facto standard way on Linux at least, so it's intuitive for the user.

We just need to pull the config from the environment and parse it, as a reference here's how curl does it.

@lilianmoraru
Copy link

This won't be in the initial release?

@alexcrichton
Copy link
Member

Since the switch to curl, this is actually supported in the same standard method that curl supports it now (e.g. standard env vars)

@lilianmoraru
Copy link

Solved by #532?

@brson
Copy link
Contributor Author

brson commented Jul 19, 2016

@lilianmoraru I don't quite consider it solved yet. Two things:

  • I'd like to verify that the env vars used by hyper are the same used by curl, and a quick googling did not lead me to a description of how the curl env vars work
  • I'd like the proxy support documented in the README

Unfortunately I still don't know much myself about how to use this feature.

@seanmonstar
Copy link
Contributor

curl environment variables: https://curl.haxx.se/docs/manpage.html heading "ENVIRONMENT":

The environment variables can be specified in lower case or upper case. The lower case version has precedence. http_proxy is an exception as it is only available in lower case.

http_proxy [protocol://][:port]

Sets the proxy server to use for HTTP.

HTTPS_PROXY [protocol://][:port]

Sets the proxy server to use for HTTPS.

hyper doesn't look for any environment variables. rustup would need to look for them, and enable the proxy settings on the Client itself.

@brson
Copy link
Contributor Author

brson commented Jul 19, 2016

Thanks @seanmonstar. The hyper backend for rustup defines a function for the HTTPS_PROXY and ALL_PROXY environment variables (which looks to work for either HTTP or HTTPS). Perhaps we should remove ALL_PROXY and add HTTP_PROXY, for just plain HTTP.

@seanmonstar
Copy link
Contributor

Ah, well I didn't copy the whole section. There is an ALL_PROXY variable for curl. And they specifically say that HTTP_PROXY is not available, only http_proxy.

Here's the rest of the section:

[url-protocol]_PROXY [protocol://][:port]

Sets the proxy server to use for [url-protocol], where the protocol is a protocol that curl supports and as specified in a URL. FTP, FTPS, POP3, IMAP, SMTP, LDAP etc.

ALL_PROXY [protocol://][:port]

Sets the proxy server to use if no protocol-specific proxy is set.

NO_PROXY

list of host names that shouldn't go through any proxy. If set to a asterisk '*' only, it matches all hosts.

@brson
Copy link
Contributor Author

brson commented Jul 21, 2016

The hyper backend now uses the same env vars as curl. Just waiting on docs.

@brson
Copy link
Contributor Author

brson commented Jul 22, 2016

Done. Thanks @inejge !

@brson brson closed this as completed Jul 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants