`Proxy::try_from_env` should prefer more specific environment variables

[niklasmohrin]

I have a concern that came up in tealdeer here: tealdeer-rs/tealdeer#450 (comment)

In Proxy::try_from_env, the environment variables all_proxy, https_proxy, and http_proxy are checked, in this order:

ureq/src/proxy.rs

Lines 211 to 218 in 428bba8

	const TRY_ENV: &[&str] = &[
	"ALL_PROXY",
	"all_proxy",
	"HTTPS_PROXY",
	"https_proxy",
	"HTTP_PROXY",
	"http_proxy",
	];

However, doesn't it make more sense to prefer the more specific https_proxy variable over the general all_proxy one? Additionally, should https_proxy and http_proxy even be used for for HTTP and HTTPS requests respectively?

For reference, the described behavior is what happens in hyper-util, and thus, reqwest: https://github.com/hyperium/hyper-util/blob/d91ea8efe6f3b09cc2fd6cc9e303566bd887a3ea/src/client/proxy/matcher.rs#L315-L319

Thanks for the library!

[algesten]

However, doesn't it make more sense to prefer the more specific https_proxy variable over the general all_proxy one? Additionally, should https_proxy and http_proxy even be used for for HTTP and HTTPS requests respectively?

Proxy environment variables doesn't have a spec, and it's a bit wild west out there.

ureq is a bit weird here because it treats all these variables as equals. And they (probably) are not. This is just a carry-over from the first implementation, and since I personally never been in a situation where I need to rely on these, I simply can't guide how it should work.

Order changes aside, the question is whether there are other oddities that should be corrected.

[niklasmohrin]

Fair enough. Changing the order is a breaking change anyways, so maybe it's just something to keep in mind. I personally don't use a proxy, so I don't have much input on it, but the way hyper does it seems more right to me. From what was said in the linked issue, users that regularly use proxies also found the behavior unexpected.

From a security perspective, the use of http_proxy at all when performing https requests is a bit scary, but again, I am not sure how relevant it actually is.

@Jisu-Woniu can you provide some input?

[Jisu-Woniu]

the use of http_proxy at all when performing https requests is a bit scary

Actually, no. HTTPS is encrypted stream instead of plain text, this means the client and the server are safe to exchange data stream, and the proxy cannot get much information from the stream.

Making HTTP connection prefer http_proxy -> all_proxy, and HTTPS prefer https_proxy -> http_proxy -> all_proxy is OK. Or HTTPS prefer https_proxy -> all_proxy is also acceptable. I think all_proxy is a last resort if no other proxy is found (for example some niche protocols that is not based on HTTP.

My proxy envs are configured as this (in Bash syntax), a local proxy to bypass censorship:

export https_proxy=http://127.0.0.1:7897
export http_proxy=http://127.0.0.1:7897
export all_proxy=socks5://127.0.0.1:7897
export no_proxy=localhost,*.local,127.*.*.*,127.0.0.1,192.168.*.*,10.*.*.*,::1

[Jisu-Woniu]

Proxy environment variables doesn't have a spec, and it's a bit wild west out there.

That's true, but the most widely used tool in UNIX world that defines a proxy env standard is curl / libcurl. And curl does have a explaination about proxy strings in its manpage:

       -x, --proxy <[protocol://]host[:port]>
              Use the specified proxy.

              The  proxy  string  can  be  specified  with  a  protocol://  prefix. No protocol specified or http:// it is treated as an HTTP proxy. Use socks4://, socks4a://, socks5:// or
              socks5h:// to request a specific SOCKS version to be used.

              Unix domain sockets are supported for socks proxy. Set localhost for the host part. e.g. socks5h://localhost/path/to/socket.sock

              HTTPS proxy support works with the https:// protocol prefix for OpenSSL and GnuTLS. It also works for mbedTLS, Rustls, Schannel and wolfSSL (added in 7.87.0).

              Unrecognized and unsupported proxy protocol schemes cause an error.

              If the port number is not specified in the proxy string, it is assumed to be 1080.

              This option overrides existing environment variables that set the proxy to use. If there is an environment variable setting a proxy, you can set proxy to "" to override it.

              All operations that are performed over an HTTP proxy are transparently converted to HTTP. It means that certain protocol specific operations might not be available.  This  is
              not the case if you can tunnel through the proxy, as one with the --proxytunnel option.

              User  and password that might be provided in the proxy string are URL decoded by curl. This allows you to pass in special characters such as @ by using %40 or pass in a colon
              with %3a.

              The proxy host can be specified the same way as the proxy environment variables, including the protocol prefix (http://) and the embedded user + password.

              When a proxy is used, the active FTP mode as set with --ftp-port, cannot be used.

              Doing FTP over an HTTP proxy without --proxytunnel makes curl do HTTP with an FTP URL over the proxy. For such transfers, common FTP specific options do not  work,  including
              --ssl-reqd and --ftp-ssl-control.

              If --proxy is provided several times, the last set value is used.

              Example:
              curl --proxy http://proxy.example https://example.com

              See also --socks5 and --proxy-basic.

[JPDye]

Making HTTP connection prefer http_proxy -> all_proxy, and HTTPS prefer https_proxy -> http_proxy -> all_proxy is OK. Or HTTPS prefer https_proxy -> all_proxy is also acceptable. I think all_proxy is a last resort if no other proxy is found (for example some niche protocols that is not based on HTTP.

I think http -> all and https -> all.

I don't think https -> http -> all makes sense unless I am missing a common tool that does things that way. With cURL I don't see any fallback behaviour and that makes sense to me, given the historical context of these environment variables.

I focus on servers, not clients, so can't comment too much on any other peculiarities that might need to be added/removed to align with other tools.