HTTP proxy support
diff mbox

Message ID 1462766571-7834-1-git-send-email-ruscur@russell.cc
State Changes Requested
Delegated to: Andrew Donnellan
Headers show

Commit Message

Russell Currey May 9, 2016, 4:02 a.m. UTC
With the new HTTP proxy support in hyper 0.9.2, we can finally include it
in snowpatch.

Enable snowpatch to use HTTP proxies through the http_proxy environment
variable, as seems to be standard.

Signed-off-by: Russell Currey <ruscur@russell.cc>
---
 Cargo.toml  |  2 +-
 src/main.rs | 18 +++++++++++++++++-
 2 files changed, 18 insertions(+), 2 deletions(-)

Comments

Russell Currey May 9, 2016, 6:27 a.m. UTC | #1
On Mon, 2016-05-09 at 14:02 +1000, Russell Currey wrote:
> With the new HTTP proxy support in hyper 0.9.2, we can finally include it
> in snowpatch.
> 
> Enable snowpatch to use HTTP proxies through the http_proxy environment
> variable, as seems to be standard.
> 
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> ---

Another idea would be to have individual proxy settings per server, which might
come up since Jenkins and Patchwork could reside in different networks.  This
would be too painful at the moment where proxy support in hyper is defined when
the Client is created (which for us is only once).  I think they'll expand it
further at some point though, and when they do we can look at implementing that.
Andrew Donnellan May 16, 2016, 12:52 a.m. UTC | #2
On 09/05/16 14:02, Russell Currey wrote:
> With the new HTTP proxy support in hyper 0.9.2, we can finally include it
> in snowpatch.

Excellent!

>
> Enable snowpatch to use HTTP proxies through the http_proxy environment
> variable, as seems to be standard.

What about https?

> +    let client = Arc::new(match env::var("http_proxy") {
> +        Ok(mut proxy) => {
> +            if proxy.starts_with("http://") {
> +                proxy.drain(..7);
> +            }
> +            let mut port = 80;
> +            if let Some(colon) = proxy.rfind(":") {
> +                port = proxy[colon + 1..].parse().unwrap_or_else(|e| {
> +                    panic!("http_proxy is malformed: {:?}, error: {}", proxy, e);
> +                });
> +                proxy.truncate(colon);
> +            }
> +            Client::with_http_proxy(proxy, port)
> +        },
> +        _ => Client::new()
> +    });

Can we do the URL parsing with the url module instead to avoid doing 
string awfulness?

Something along the lines of: (completely untested)

let client = Arc::new(match env::var("http_proxy") {
	Ok(proxy_url) => {
		let proxy = Url::parse(proxy_url).unwrap_or_else(|e| {
			panic!("Malformed URL: {:?}, error: {}", proxy_url, e);
		assert!(proxy.scheme() == "http");
		assert!(proxy.path() == "");
		Client::with_http_proxy(proxy.host_str(), if proxy.port() { 
proxy.port() } else { 80 })
	},
	_ => Client::new()
});
Russell Currey May 16, 2016, 12:56 a.m. UTC | #3
On Mon, 2016-05-16 at 10:52 +1000, Andrew Donnellan wrote:
> On 09/05/16 14:02, Russell Currey wrote:
> > With the new HTTP proxy support in hyper 0.9.2, we can finally include it
> > in snowpatch.
> 
> Excellent!
> 
> > 
> > Enable snowpatch to use HTTP proxies through the http_proxy environment
> > variable, as seems to be standard.
> 
> What about https?

I don't know.  The ::with_http_proxy thing is all that's in hyper from what I
can tell, whether that'll actually happily use a https proxy I don't know.  It
does make successful https connections through a http proxy from what I can
tell..I'll try to confirm.

> 
> > +    let client = Arc::new(match env::var("http_proxy") {
> > +        Ok(mut proxy) => {
> > +            if proxy.starts_with("http://") {
> > +                proxy.drain(..7);
> > +            }
> > +            let mut port = 80;
> > +            if let Some(colon) = proxy.rfind(":") {
> > +                port = proxy[colon + 1..].parse().unwrap_or_else(|e| {
> > +                    panic!("http_proxy is malformed: {:?}, error: {}",
> > proxy, e);
> > +                });
> > +                proxy.truncate(colon);
> > +            }
> > +            Client::with_http_proxy(proxy, port)
> > +        },
> > +        _ => Client::new()
> > +    });
> 
> Can we do the URL parsing with the url module instead to avoid doing 
> string awfulness?
> 
> Something along the lines of: (completely untested)
> 
> let client = Arc::new(match env::var("http_proxy") {
> 	Ok(proxy_url) => {
> 		let proxy = Url::parse(proxy_url).unwrap_or_else(|e| {
> 			panic!("Malformed URL: {:?}, error: {}", proxy_url, e);
> 		assert!(proxy.scheme() == "http");
> 		assert!(proxy.path() == "");
> 		Client::with_http_proxy(proxy.host_str(), if proxy.port() { 
> proxy.port() } else { 80 })
> 	},
> 	_ => Client::new()
> });

This is a very good idea that I'm upset I didn't think of.

Patch
diff mbox

diff --git a/Cargo.toml b/Cargo.toml
index 614abbb..2055e8a 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -26,7 +26,7 @@  license = "GPL-2.0+"
 name = "snowpatch"
 
 [dependencies]
-hyper = "0.9"
+hyper = ">0.9.2" # 0.9.2 or later required for proxy support
 git2 = "0.3"
 rustc-serialize = "0.3"
 url = "0.5"
diff --git a/src/main.rs b/src/main.rs
index e9c2035..25899a4 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -38,6 +38,7 @@  use std::sync::Arc;
 use std::thread;
 use std::time::Duration;
 use std::path::Path;
+use std::env;
 
 mod patchwork;
 use patchwork::{PatchworkServer, TestState, TestResult};
@@ -226,7 +227,22 @@  fn main() {
     let settings = settings::parse(args.arg_config_file);
 
     // The HTTP client we'll use to access the APIs
-    let client = Arc::new(Client::new());
+    let client = Arc::new(match env::var("http_proxy") {
+        Ok(mut proxy) => {
+            if proxy.starts_with("http://") {
+                proxy.drain(..7);
+            }
+            let mut port = 80;
+            if let Some(colon) = proxy.rfind(":") {
+                port = proxy[colon + 1..].parse().unwrap_or_else(|e| {
+                    panic!("http_proxy is malformed: {:?}, error: {}", proxy, e);
+                });
+                proxy.truncate(colon);
+            }
+            Client::with_http_proxy(proxy, port)
+        },
+        _ => Client::new()
+    });
 
     let mut patchwork = PatchworkServer::new(&settings.patchwork.url, &client);
     if settings.patchwork.user.is_some() {