feat(proxy): client proxy for http and https

[winding-lines]

This is a proof of concept for supporting client side proxies. The tests
are not passing yet, I am just soliciting feedback on the architecture.

The use cases that I see are the following

• when proxy is configured by default all connections go to the proxy
• additionally there is a small list of local wan/localhost which bypass
  the proxy

So once a connection to a host is established it will either use the
proxy
all the time or it will be a direct connection all the time. The Pool
keys streams on host-port-scheme so a stream is either proxy or direct.

The logic is the following:

1. connect to the proxy instead of the desired host
2. for http use the appropriate RequestUri (pre-existing class)
3. for https we need to issue a CONNECT first

Let me know what you think.

[GitCop]

Thanks for contributing! Unfortunately, I'm here to tell you there were the following style issues with your Pull Request:

• Commit: 5109c29
  • Commits must be in the following format: %{type}(%{scope}): %{description}
  • Invalid type. Valid types are feat, fix, docs, style, refactor, perf, test, chore, revert

Guidelines are available at https://github.com/hyperium/hyper/blob/master/CONTRIBUTING.md

---

This message was auto-generated by https://gitcop.com

[GitCop]

Thanks for contributing! Unfortunately, I'm here to tell you there were the following style issues with your Pull Request:

• Commit: 5109c29
  • Commits must be in the following format: %{type}(%{scope}): %{description}
  • Invalid type. Valid types are feat, fix, docs, style, refactor, perf, test, chore, revert
• Commit: 3ba3bcc
  • Commits must be in the following format: %{type}(%{scope}): %{description}

Guidelines are available at https://github.com/hyperium/hyper/blob/master/CONTRIBUTING.md

---

This message was auto-generated by https://gitcop.com

[GitCop]

Thanks for contributing! Unfortunately, I'm here to tell you there were the following style issues with your Pull Request:

• Commit: ca133ad
  • Commits must be in the following format: %{type}(%{scope}): %{description}

Guidelines are available at https://github.com/hyperium/hyper/blob/master/CONTRIBUTING.md

---

This message was auto-generated by https://gitcop.com

[GitCop]

Thanks for contributing! Unfortunately, I'm here to tell you there were the following style issues with your Pull Request:

• Commit: d0d3f29
  • Invalid type. Valid types are feat, fix, docs, style, refactor, perf, test, chore, revert

Guidelines are available at https://github.com/hyperium/hyper/blob/master/CONTRIBUTING.md

---

This message was auto-generated by https://gitcop.com

[winding-lines]

Good morning Hyper community,

I've rebased my client proxy changes on master and made all the tests pass. I need to write some tests but I would like to get some advice first on the architecture. I tried a couple of alternatives like removing the concept of the Route or not have it closely tied to the HttpStream. The challenge is that the proxying is a pretty low level concept so any change tends to propagate through the codebase.

Where would you like me to take next? We have a stable commit right now and we can compare with other approaches.

[shaleh]

At the moment there is an assumption that both HTTP and HTTPS will be handled by the same proxy. This could cause problems. While I do not need it I know others do require authentication support.

As for design concerns, is there a reason you made a new constructor instead of allowing the user to call something like add_proxy? Common use code will look something like:

let client = match read_proxy_env() {
    Some(proxy_url) => {
         Client::with_proxy_config(proxy::Config {
             proxy_host: proxy_url.some_struct_value,
             proxy_port: proxy_url.some_other_struct_value,
             proxy_version: "1.1".to_string()
         })
    }
    None => Client::new(),
}

Except as I noted above this only works when one proxy config works for both settings. If there is a separate HTTP and HTTPS proxy things get more complex. This complexity needs to be duplicated in each and every client project using hyper.

Not directly this patch's problem but reading env::var("http_proxy") is not trivial. I need to use something like into_url on the string, then find the bits in the Url struct. Similarly how should one handle no_proxy? It would be nice if there was convenience help provided for this set of common env variables.

[shaleh]

The code works as expected for HTTP requests with the caveat that all requests go to the proxy because there is no support for no_proxy or the equivalent.

Any attempt to access a SSL url generates a SIGPIPE for me.

[winding-lines]

@shaleh thanks for the feedback. I will add authentication and fix the https crash. I also need to add tests, I will do that after I refactor the internals based on other feedback.

I do not know how frequent it is for people to have multiple proxies. If we assume that most people have one proxy for all requests then the current API is the simplest. For your use case you can create 3 Hyper clients in the three configurations that you describe:

• without proxy
• http proxy
• https proxy

This keeps the library simple and allows flexibility in the consuming application, with the downside that there is more code in the application.

Let me know what you think.

[shaleh]

The point of a library is to make the application author's life easier AND to have one place to put redundant code.

Almost every user of the hyper crate will need to implement proxy support at some point. The crate should make it easy for the application author to get it right. Ideally this means stuffing as much proxy knowledge into the crate as possible.

I'd like to see a baked in way to create a hyper based Client that read its settings from the environment. A similar bit of code that took a list of proxies instead of reading the environment would be another useful interface for those that want the proxy to come from their own configs.

Sorry, I would have written code instead of asking but I have been busy on other projects.

[winding-lines]

Got it, I have been working on multiple proxies but I have only 6-8
hrs/week available myself. I will put out what ever code I have ready at
the end of this weekend.

On Fri, Oct 16, 2015 at 1:25 PM, Sean Perry [email protected]
wrote:

The point of a library is to make the application author's life easier
AND to have one place to put redundant code.

Almost every user of the hyper crate will need to implement proxy support
at some point. The crate should make it easy for the application author to
get it right. Ideally this means stuffing as much proxy knowledge into the
crate as possible.

I'd like to see a baked in way to create a hyper based Client that read
its settings from the environment. A similar bit of code that took a list
of proxies instead of reading the environment would be another useful
interface for those that want the proxy to come from their own configs.

Sorry, I would have written code instead of asking but I have been busy on
other projects.

—
Reply to this email directly or view it on GitHub
#639 (comment).

[seanmonstar]

I think the task of getting the proxy settings should be outside of hyper. The configuration could come from anywhere (environment variables, command line arguments, config files, hard coded, etc). Instead, hyper should accept the parameters in the Proxy constructor.

For instance:

client.set_proxy(Proxy::Http {
    host: env::var("HTTP_PROXY_HOST").unwrap(),
    port: env::var("HTTP_PROXY_PORT").unwrap(),
})

---

Also, it is possible to build a Proxy entirely outside of this crate, and use a hyper Client internally. That's why I've mentioned trying not to adjust too much of the internal http protocol modules. The Client (and Request) just need to be able to use all the parts of RFC7230, such as the CONNECT method and alternate RequestUri formats. I'd rather if the logic to use those was contained in the Proxy type.

[winding-lines]

Yeah, I have been trying to be less invasive. I think that some internal
refactoring is needed and it may be outside my current understanding of the
code. Let me try this weekend and see where I end up, you are certainly
welcome to take over as soon as you have the cycles :-)

On Sat, Oct 17, 2015 at 10:59 AM, Sean McArthur [email protected]
wrote:

I think the task of getting the proxy settings should be outside of
hyper. The configuration could come from anywhere (environment variables,
command line arguments, config files, hard coded, etc). Instead, hyper
should accept the parameters in the Proxy constructor.

For instance:

client.set_proxy(Proxy::Http {
host: env::var("HTTP_PROXY_HOST").unwrap(),
port: env::var("HTTP_PROXY_PORT").unwrap(),
})

---

Also, it is possible to build a Proxy entirely outside of this crate,
and use a hyper Client internally. That's why I've mentioned trying not to
adjust too much of the internal http protocol modules. The Client (and
Request) just need to be able to use all the parts of RFC7230, such as the
CONNECT method and alternate RequestUri formats. I'd rather if the logic to
use those was contained in the Proxy type.

—
Reply to this email directly or view it on GitHub
#639 (comment).

[GitCop]

Thanks for contributing! Unfortunately, I'm here to tell you there were the following style issues with your Pull Request:

• Commit: 65777ae
  • Commits must be in the following format: %{type}(%{scope}): %{description}

Guidelines are available at https://github.com/hyperium/hyper/blob/master/CONTRIBUTING.md

---

This message was auto-generated by https://gitcop.com

[winding-lines]

@seanmonstar I got rid of the Route and instead added a request_uri in Request (and RequestHead) as you had suggested on the initial PR. I've added a list of proxies each with their own proxy policies. All of this is only in place for the Http 1.1 code.

For the SSL NetworkConnector there is still one architecture issue because all the SSL types are defined only in the h2.rs file. So the NetworkConnector's connect still defers back to the Proxy, and this code is not ported to use the new list of proxies as per the desired APIs.

Where do you want me to take this code next?

[seanmonstar]

What's the difference between using a Connector with_proxy, and calling client.add_proxy?

[GitCop]

Thanks for contributing! Unfortunately, I'm here to tell you there were the following style issues with your Pull Request:

• Commit: 65777ae
  • Commits must be in the following format: %{type}(%{scope}): %{description}

Guidelines are available at https://github.com/hyperium/hyper/blob/master/CONTRIBUTING.md

---

This message was auto-generated by https://gitcop.com

[GitCop]

Thanks for contributing! Unfortunately, I'm here to tell you there were the following style issues with your Pull Request:

• Commit: c30780b
  • Commits must be in the following format: %{type}(%{scope}): %{description}

Guidelines are available at https://github.com/hyperium/hyper/blob/master/CONTRIBUTING.md

---

This message was auto-generated by https://gitcop.com

[winding-lines]

@seanmonstar, thanks for your feedback I've addressed some of it.

I love your suggestion to have an implementation of NetworkConnector for Proxy, not yet implemented. What needs to happen is the following:

1. the Proxy implementation will connect to the proxied port
2. the socket must be passed down to the Http11 or SSL NetworkConnectors

With the current API this is not possible. This is the reason why the proxy is passed down.

Note that the Pool NetworkConnector has a different use case, the lower level NetworkConnectors are allowed to open their own sockets.

Please feel free to take over this PR and play with some approaches :)

[mitsuhiko]

As mentioned in the ticket I would love to help drive this PR forward. Proxy support is very important to me.

[seanmonstar]

Ideally some of this work can be brought over to the async world, as I hope to merge async io soon. It may even be that since the async version of hyper is a little lower level, using a proxy may be easier.