proxy: enable HTTP CONNECT request support (#1200)

When the proxy receives a `CONNECT` request, the HTTP Upgrade pieces
are used since a CONNECT is very similar to an Upgrade. If the CONNECT
response back from the proxied client request is successful, the
connection is converted into a TCP proxy, just like with Upgrades.

Cargo.lock

@@ -132,9 +132,9 @@ dependencies = [
  "futures-mpsc-lossy 0.3.0",
  "futures-watch 0.1.0 (git+https://github.com/carllerche/better-future)",
  "h2 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)",
- "http 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
+ "http 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "httparse 1.2.4 (registry+https://github.com/rust-lang/crates.io-index)",
- "hyper 0.12.2 (registry+https://github.com/rust-lang/crates.io-index)",
+ "hyper 0.12.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "indexmap 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "inotify 0.5.2-dev (git+https://github.com/inotify-rs/inotify)",
  "ipnet 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -179,7 +179,7 @@ dependencies = [
  "bytes 0.4.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "futures 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)",
  "h2 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)",
- "http 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
+ "http 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "prost 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "prost-derive 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "prost-types 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -378,7 +378,7 @@ dependencies = [
  "bytes 0.4.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "fnv 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "futures 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)",
- "http 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
+ "http 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "indexmap 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "slab 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -405,11 +405,12 @@ dependencies = [
 
 [[package]]
 name = "http"
-version = "0.1.5"
+version = "0.1.7"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "bytes 0.4.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "fnv 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)",
+ "itoa 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -419,16 +420,17 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
 name = "hyper"
-version = "0.12.2"
+version = "0.12.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "bytes 0.4.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "futures 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)",
  "futures-cpupool 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "h2 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)",
- "http 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
+ "http 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "httparse 1.2.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "iovec 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
+ "itoa 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "net2 0.2.32 (registry+https://github.com/rust-lang/crates.io-index)",
  "time 0.1.39 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -509,6 +511,11 @@ dependencies = [
  "either 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
+[[package]]
+name = "itoa"
+version = "0.4.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+
 [[package]]
 name = "kernel32-sys"
 version = "0.2.2"
@@ -1244,7 +1251,7 @@ dependencies = [
  "bytes 0.4.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "futures 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)",
  "h2 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)",
- "http 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
+ "http 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "prost 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "tower-h2 0.1.0 (git+https://github.com/tower-rs/tower-h2)",
@@ -1269,7 +1276,7 @@ dependencies = [
  "bytes 0.4.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "futures 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)",
  "h2 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)",
- "http 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
+ "http 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "tokio-connect 0.1.0 (git+https://github.com/carllerche/tokio-connect)",
  "tokio-io 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1283,7 +1290,7 @@ source = "git+https://github.com/tower-rs/tower-h2#760f9fc1c83c3a96edb6fbddb6b0c
 dependencies = [
  "futures 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)",
  "h2 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)",
- "http 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
+ "http 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "tower-balance 0.1.0 (git+https://github.com/tower-rs/tower)",
  "tower-h2 0.1.0 (git+https://github.com/tower-rs/tower-h2)",
  "tower-service 0.1.0 (git+https://github.com/tower-rs/tower)",
@@ -1572,9 +1579,9 @@ dependencies = [
 "checksum h2 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)" = "6229ac66d3392dd83288fe04defd4b353354b15bbe07820d53dda063a736afcc"
 "checksum heck 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ea04fa3ead4e05e51a7c806fc07271fdbde4e246a6c6d1efd52e72230b771b82"
 "checksum hostname 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "58fab6e177434b0bb4cd344a4dabaa5bd6d7a8d792b1885aebcae7af1091d1cb"
-"checksum http 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)" = "75df369fd52c60635208a4d3e694777c099569b3dcf4844df8f652dc004644ab"
+"checksum http 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)" = "4fbced8864b04c030eebcb7d0dc3a81ba5231ac559f5116a29a8ba83ecee22cd"
 "checksum httparse 1.2.4 (registry+https://github.com/rust-lang/crates.io-index)" = "c2f407128745b78abc95c0ffbe4e5d37427fdc0d45470710cfef8c44522a2e37"
-"checksum hyper 0.12.2 (registry+https://github.com/rust-lang/crates.io-index)" = "ad39a4f15051ccd4ea6adf44df851e00fd9062c71734391d806246b94e69dc1f"
+"checksum hyper 0.12.3 (registry+https://github.com/rust-lang/crates.io-index)" = "f31c3ce511142fac936539abd3315bdc303a6e501fac5b77e0824310d542d081"
 "checksum idna 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "014b298351066f1512874135335d62a789ffe78a9974f94b43ed5621951eaf7d"
 "checksum indexmap 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "7b9378f1f3923647a9aea6af4c6b5de68cc8a71415459ad25ef191191c48f5b7"
 "checksum inotify 0.5.2-dev (git+https://github.com/inotify-rs/inotify)" = "<none>"
@@ -1583,6 +1590,7 @@ dependencies = [
 "checksum ipconfig 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)" = "9ec4e18c0a0d4340870c14284293632d8421f419008371422dd327892b88877c"
 "checksum ipnet 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "51268c3a27ad46afd1cca0bbf423a5be2e9fd3e6a7534736c195f0f834b763ef"
 "checksum itertools 0.7.6 (registry+https://github.com/rust-lang/crates.io-index)" = "b07332223953b5051bceb67e8c4700aa65291535568e1f12408c43c4a42c0394"
+"checksum itoa 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)" = "c069bbec61e1ca5a596166e55dfe4773ff745c3d16b700013bcaff9a6df2c682"
 "checksum kernel32-sys 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "7507624b29483431c0ba2d82aece8ca6cdba9382bff4ddd0f7490560c056098d"
 "checksum lazy_static 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "c8f31047daa365f19be14b47c29df4f7c3b581832407daabe6ae77397619237d"
 "checksum lazycell 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "a6f08839bc70ef4a3fe1d566d5350f519c5912ea86be0df1740a7d247c7fc0ef"

proxy/Cargo.toml

@@ -22,7 +22,7 @@ futures-watch = { git = "https://github.com/carllerche/better-future" }
 h2 = "0.1.10"
 http = "0.1"
 httparse = "1.2"
-hyper = "0.12.2"
+hyper = "0.12.3"
 ipnet = "1.0"
 log = "0.4.1"
 indexmap = "1.0.0"

proxy/src/bind.rs

@@ -539,15 +539,16 @@ impl Protocol {
             return Protocol::Http2
         }
 
-        let authority_part = req.uri().authority_part();
+        let was_absolute_form = h1::is_absolute_form(req.uri());
-        let was_absolute_form = authority_part.is_some();
         trace!(
             "Protocol::detect(); req.uri='{:?}'; was_absolute_form={:?};",
             req.uri(), was_absolute_form
         );
         // If the request has an authority part, use that as the host part of
         // the key for an HTTP/1.x request.
-        let host = authority_part
+        let host = req
+            .uri()
+            .authority_part()
             .cloned()
             .or_else(|| h1::authority_from_host(req))
             .map(Host::Authority)

proxy/src/transparency/client.rs

@@ -12,7 +12,7 @@ use bind;
 use task::BoxExecutor;
 use telemetry::sensor::http::RequestBody;
 use super::glue::{BodyPayload, HttpBody, HyperConnect};
-use super::upgrade::Http11Upgrade;
+use super::upgrade::{HttpConnect, Http11Upgrade};
 
 type HyperClient<C, B> =
     hyper::Client<HyperConnect<C>, BodyPayload<RequestBody<B>>>;
@@ -216,9 +216,15 @@ where
             ClientServiceInner::Http1(ref h1) => {
                 let mut req = req.map(BodyPayload::new);
                 let upgrade = req.extensions_mut().remove::<Http11Upgrade>();
+                let is_http_connect = if upgrade.is_some() {
+                    req.method() == &http::Method::CONNECT
+                } else {
+                    false
+                };
                 ClientServiceFuture::Http1 {
                     future: h1.request(req),
                     upgrade,
+                    is_http_connect,
                 }
             },
             ClientServiceInner::Http2(ref mut h2) => {
@@ -232,6 +238,7 @@ pub enum ClientServiceFuture {
     Http1 {
         future: hyper::client::ResponseFuture,
         upgrade: Option<Http11Upgrade>,
+        is_http_connect: bool,
     },
     Http2(tower_h2::client::ResponseFuture),
 }
@@ -242,13 +249,16 @@ impl Future for ClientServiceFuture {
 
     fn poll(&mut self) -> Poll<Self::Item, Self::Error> {
         match self {
-            ClientServiceFuture::Http1 { future, upgrade } => {
+            ClientServiceFuture::Http1 { future, upgrade, is_http_connect } => {
                 match future.poll() {
                     Ok(Async::Ready(res)) => {
-                        let res = res.map(move |b| HttpBody::Http1 {
+                        let mut res = res.map(move |b| HttpBody::Http1 {
                             body: Some(b),
                             upgrade: upgrade.take(),
                         });
+                        if *is_http_connect {
+                            res.extensions_mut().insert(HttpConnect);
+                        }
                         Ok(Async::Ready(res))
                     },
                     Ok(Async::NotReady) => Ok(Async::NotReady),

proxy/src/transparency/glue.rs

@@ -32,7 +32,7 @@ pub enum HttpBody {
 }
 
 /// Glue for `tower_h2::Body`s to be used in hyper.
-#[derive(Debug)]
+#[derive(Debug, Default)]
 pub(super) struct BodyPayload<B> {
     body: B,
 }
@@ -255,14 +255,19 @@ where
     >;
 
     fn call(&mut self, mut req: http::Request<Self::ReqBody>) -> Self::Future {
-        if let &http::Method::CONNECT = req.method() {
+        // Should this rejection happen later in the Service stack?
-            debug!("HTTP/1.1 CONNECT not supported");
+        //
-            let res = http::Response::builder()
+        // Rejecting here means telemetry doesn't record anything about it...
-                .status(http::StatusCode::BAD_GATEWAY)
+        //
-                .body(BodyPayload::new(Default::default()))
+        // At the same time, this stuff is specifically HTTP1, so it feels
-                .expect("building response with empty body should not error!");
+        // proper to not have the HTTP2 requests going through it...
+        if h1::is_bad_request(&req) {
+            let mut res = http::Response::default();
+            *res.status_mut() = http::StatusCode::BAD_REQUEST;
             return Either::B(future::ok(res));
         }
+
+
         req.extensions_mut().insert(self.srv_ctx.clone());
 
         let upgrade = if h1::wants_upgrade(&req) {

proxy/src/transparency/h1.rs

@@ -8,10 +8,17 @@ use http::header::{HOST, UPGRADE};
 use http::uri::{Authority, Parts, Scheme, Uri};
 
 use ctx::transport::{Server as ServerCtx};
+use super::upgrade::HttpConnect;
 
 /// Tries to make sure the `Uri` of the request is in a form needed by
 /// hyper's Client.
 pub fn normalize_our_view_of_uri<B>(req: &mut http::Request<B>) {
+    debug_assert!(
+        req.uri().scheme_part().is_none(),
+        "normalize_uri shouldn't be called with absolute URIs: {:?}",
+        req.uri()
+    );
+
     // try to parse the Host header
     if let Some(auth) = authority_from_host(&req) {
         set_authority(req.uri_mut(), auth);
@@ -50,9 +57,19 @@ pub fn authority_from_host<B>(req: &http::Request<B>) -> Option<Authority> {
 
 fn set_authority(uri: &mut http::Uri, auth: Authority) {
     let mut parts = Parts::from(mem::replace(uri, Uri::default()));
-    parts.scheme = Some(Scheme::HTTP);
+
     parts.authority = Some(auth);
 
+    // If this was an origin-form target (path only),
+    // then we can't *only* set the authority, as that's
+    // an illegal target (such as `conduit.io/docs`).
+    //
+    // But don't set a scheme if this was authority-form (CONNECT),
+    // since that would change it's meaning (like `https://conduit.io`).
+    if parts.path_and_query.is_some() {
+        parts.scheme = Some(Scheme::HTTP);
+    }
+
     let new = Uri::from_parts(parts)
         .expect("absolute uri");
 
@@ -99,18 +116,92 @@ pub fn wants_upgrade<B>(req: &http::Request<B>) -> bool {
         // the proxy strips h2 upgrade headers.
         //
         // Eventually, the proxy will support h2 upgrades directly.
-        upgrade != "h2c"
+        return upgrade != "h2c";
-    } else {
-        // No Upgrade header means no upgrade wanted!
-        false
     }
 
-
+    // HTTP/1.1 CONNECT requests are just like upgrades!
+    req.method() == &http::Method::CONNECT
 }
 
 /// Checks responses to determine if they are successful HTTP upgrades.
 pub fn is_upgrade<B>(res: &http::Response<B>) -> bool {
+    // Upgrades were introduced in HTTP/1.1
+    if res.version() != http::Version::HTTP_11 {
+        return false;
+    }
+
     // 101 Switching Protocols
-    res.status() == http::StatusCode::SWITCHING_PROTOCOLS
+    if res.status() == http::StatusCode::SWITCHING_PROTOCOLS {
-        && res.version() == http::Version::HTTP_11
+        return true;
+    }
+
+    // CONNECT requests are complete if status code is 2xx.
+    if res.extensions().get::<HttpConnect>().is_some()
+        && res.status().is_success() {
+        return true;
+    }
+
+    // Just a regular HTTP response...
+    false
+}
+
+/// Returns if the request target is in `absolute-form`.
+///
+/// This is `absolute-form`: `https://conduit.io/docs`
+///
+/// This is not:
+///
+/// - `/docs`
+/// - `conduit.io`
+pub fn is_absolute_form(uri: &Uri) -> bool {
+    // It's sufficient just to check for a scheme, since in HTTP1,
+    // it's required in absolute-form, and `http::Uri` doesn't
+    // allow URIs with the other parts missing when the scheme is set.
+    debug_assert!(
+        uri.scheme_part().is_none() ||
+        (
+            uri.authority_part().is_some() &&
+            uri.path_and_query().is_some()
+        ),
+        "is_absolute_form http::Uri invariants: {:?}",
+        uri
+    );
+
+    uri.scheme_part().is_some()
+}
+
+/// Returns if the request target is in `origin-form`.
+///
+/// This is `origin-form`: `conduit.io`
+fn is_origin_form(uri: &Uri) -> bool {
+    uri.scheme_part().is_none() &&
+        uri.path_and_query().is_none()
+}
+
+/// Returns if the received request is definitely bad.
+///
+/// Just because a request parses doesn't mean it's correct. For examples:
+///
+/// - `GET conduit.io`
+/// - `CONNECT /just-a-path
+pub fn is_bad_request<B>(req: &http::Request<B>) -> bool {
+    if req.method() == &http::Method::CONNECT {
+        // CONNECT is only valid over HTTP/1.1
+        if req.version() != http::Version::HTTP_11 {
+            debug!("CONNECT request not valid for HTTP/1.0: {:?}", req.uri());
+            return true;
+        }
+
+        // CONNECT requests are only valid in authority-form.
+        if !is_origin_form(req.uri()) {
+            debug!("CONNECT request with illegal URI: {:?}", req.uri());
+            return true;
+        }
+    // If not CONNECT, refuse any origin-form URIs
+    } else if is_origin_form(req.uri()) {
+        debug!("{} request with illegal URI: {:?}", req.method(), req.uri());
+        return true;
+    }
+
+    false
 }

proxy/src/transparency/upgrade.rs

@@ -35,6 +35,11 @@ pub struct Http11UpgradeHalves {
     _inner: (),
 }
 
+/// A marker type inserted into Extensions to signal it was an HTTP CONNECT
+/// request.
+#[derive(Debug)]
+pub struct HttpConnect;
+
 struct Inner {
     server: TryLock<Option<OnUpgrade>>,
     client: TryLock<Option<OnUpgrade>>,
@@ -157,6 +162,9 @@ impl Drop for Inner {
             if let Err(_) = self.upgrade_executor.execute(fut) {
                 trace!("error spawning HTTP upgrade task");
             }
+        } else {
+            trace!("HTTP/1.1 upgrade half missing");
         }
     }
 }
+

proxy/tests/transparency.rs

@@ -30,25 +30,6 @@ fn inbound_http1() {
     assert_eq!(client.get("/"), "hello h1");
 }
 
-#[test]
-fn http1_connect_not_supported() {
-    let _ = env_logger::try_init();
-
-    let srv = server::tcp()
-        .run();
-    let proxy = proxy::new()
-        .inbound(srv)
-        .run();
-
-    let client = client::tcp(proxy.inbound);
-
-    let tcp_client = client.connect();
-    tcp_client.write("CONNECT foo.bar:443 HTTP/1.1\r\nHost: foo.bar:443\r\n\r\n");
-
-    let expected = "HTTP/1.1 502 Bad Gateway\r\n";
-    assert_eq!(s(&tcp_client.read()[..expected.len()]), expected);
-}
-
 #[test]
 fn http1_removes_connection_headers() {
     let _ = env_logger::try_init();
@@ -439,6 +420,147 @@ fn http11_upgrade_h2_stripped() {
     assert_eq!(res.status(), http::StatusCode::OK);
 }
 
+#[test]
+fn http11_connect() {
+    let _ = env_logger::try_init();
+
+    // To simplify things for this test, we just use the test TCP
+    // client and server to do an HTTP CONNECT.
+    //
+    // We don't *actually* perfom a new connect to requested host,
+    // but client doesn't need to know that for our tests.
+
+    let connect_req = "\
+        CONNECT foo.bar HTTP/1.1\r\n\
+        Host: foo.bar\r\n\
+        \r\n\
+        ";
+    let connect_res = "\
+        HTTP/1.1 200 OK\r\n\
+        \r\n\
+        ";
+
+    let tunneled_req = "{send}: hi all\n";
+    let tunneled_res = "{recv}: welcome!\n";
+
+    let srv = server::tcp()
+        .accept_fut(move |sock| {
+            // Read connect_req...
+            tokio_io::io::read(sock, vec![0; 512])
+                .and_then(move |(sock, vec, n)| {
+                    let head = s(&vec[..n]);
+                    assert_contains!(head, "CONNECT foo.bar HTTP/1.1\r\n");
+
+                    // Write connect_res back...
+                    tokio_io::io::write_all(sock, connect_res)
+                })
+                .and_then(move |(sock, _)| {
+                    // Read the message after tunneling...
+                    tokio_io::io::read(sock, vec![0; 512])
+                })
+                .and_then(move |(sock, vec, n)| {
+                    assert_eq!(s(&vec[..n]), tunneled_req);
+
+                    // Some processing... and then write back tunneled res...
+                    tokio_io::io::write_all(sock, tunneled_res)
+                })
+                .map(|_| ())
+                .map_err(|e| panic!("tcp server error: {}", e))
+        })
+        .run();
+    let proxy = proxy::new()
+        .inbound(srv)
+        .run();
+
+    let client = client::tcp(proxy.inbound);
+
+    let tcp_client = client.connect();
+
+    tcp_client.write(connect_req);
+
+    let resp = tcp_client.read();
+    let resp_str = s(&resp);
+    assert!(
+        resp_str.starts_with("HTTP/1.1 200 OK\r\n"),
+        "response not an upgrade: {:?}",
+        resp_str
+    );
+
+    // We've CONNECTed from HTTP to foo.bar! Say hi!
+    tcp_client.write(tunneled_req);
+    // Did anyone respond?
+    let resp2 = tcp_client.read();
+    assert_eq!(s(&resp2), tunneled_res);
+}
+
+#[test]
+fn http11_connect_bad_requests() {
+    let _ = env_logger::try_init();
+
+    let srv = server::tcp()
+        .accept(move |_sock| -> Vec<u8> {
+            unreachable!("shouldn't get through the proxy");
+        })
+        .run();
+    let proxy = proxy::new()
+        .inbound(srv)
+        .run();
+
+    // A TCP client is used since the HTTP client would stop these requests
+    // from ever touching the network.
+    let client = client::tcp(proxy.inbound);
+
+    let bad_uris = vec![
+        "/origin-form",
+        "/",
+        "http://test/bar",
+        "http://test",
+        "*",
+    ];
+
+    for bad_uri in bad_uris {
+        let tcp_client = client.connect();
+
+        let req = format!("CONNECT {} HTTP/1.1\r\nHost: test\r\n\r\n", bad_uri);
+        tcp_client.write(req);
+
+        let resp = tcp_client.read();
+        let resp_str = s(&resp);
+        assert!(
+            resp_str.starts_with("HTTP/1.1 400 Bad Request\r\n"),
+            "bad URI ({:?}) should get 400 response: {:?}",
+            bad_uri,
+            resp_str
+        );
+    }
+
+    // origin-form URIs must be CONNECT
+    let tcp_client = client.connect();
+
+    tcp_client.write("GET test HTTP/1.1\r\nHost: test\r\n\r\n");
+
+    let resp = tcp_client.read();
+    let resp_str = s(&resp);
+    assert!(
+        resp_str.starts_with("HTTP/1.1 400 Bad Request\r\n"),
+        "origin-form without CONNECT should get 400 response: {:?}",
+        resp_str
+    );
+
+    // check that HTTP/1.0 is not allowed for CONNECT
+    let tcp_client = client.connect();
+
+    tcp_client.write("CONNECT test HTTP/1.0\r\nHost: test\r\n\r\n");
+
+    let resp = tcp_client.read();
+    let resp_str = s(&resp);
+    assert!(
+        resp_str.starts_with("HTTP/1.0 400 Bad Request\r\n"),
+        "HTTP/1.0 CONNECT should get 400 response: {:?}",
+        resp_str
+    );
+}
+
 #[test]
 fn http1_requests_without_body_doesnt_add_transfer_encoding() {
     let _ = env_logger::try_init();