Repository: algesten/ureq
Pull request #936 (2025-01-06 21:22:11)

Fix host header for connect proxy


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0de1c57..f9ad64a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
   * `TlsConfig::unversioned_rustls_crypto_provider()` (#931)
   * Feature `rustls-no-provider` to compile without ring (#931)
+  * Fix CONNECT proxy Host header (#936)
   * Re-enable CONNECT proxy support (#932)
   * Body::content_length (#927)
   * Handle Authorization: Basic from URI (#923)
diff --git a/Cargo.lock b/Cargo.lock
index c9d4d56..eb881a2 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1435,9 +1435,9 @@ dependencies = [
 
 [[package]]
 name = "ureq-proto"
-version = "0.2.2"
+version = "0.2.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f76790855073cbbdf9a23a7a622741480cebaec1d9b9b06b6ed2c55611fed37c"
+checksum = "a27729fd2c15426f48992a911ce31bd1f29c1cd0898c2c86b410d24f51c99eb3"
 dependencies = [
  "base64",
  "http",
diff --git a/Cargo.toml b/Cargo.toml
index 2ecaebc..14a9e19 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -56,7 +56,7 @@ features = ["rustls", "platform-verifier", "native-tls", "socks-proxy", "cookies
 
 [dependencies]
 base64 = "0.22.1"
-ureq-proto = "0.2.2"
+ureq-proto = "0.2.3"
 # ureq-proto = { path = "../ureq-proto" }
 log = "0.4.22"
 once_cell = "1.19.0"
diff --git a/src/lib.rs b/src/lib.rs
index 58f06cb..c8460fc 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -913,6 +913,22 @@ pub(crate) mod test {
         let _ = agent.get("http://cookie.test/cookie-test").call().unwrap();
     }
 
+    #[test]
+    #[cfg(feature = "_test")]
+    fn http_connect_proxy() {
+        init_test_log();
+
+        let proxy = Proxy::new("http://my_proxy:1234/connect-proxy").unwrap();
+
+        let agent = Agent::config_builder()
+            .proxy(Some(proxy))
+            .build()
+            .new_agent();
+
+        let mut res = agent.get("http://httpbin.org/get").call().unwrap();
+        res.body_mut().read_to_string().unwrap();
+    }
+
     // This doesn't need to run, just compile.
     fn _ensure_send_sync() {
         fn is_send(_t: impl Send) {}
diff --git a/src/proxy.rs b/src/proxy.rs
index b614962..665a4b3 100644
--- a/src/proxy.rs
+++ b/src/proxy.rs
@@ -211,16 +211,17 @@ impl<In: Transport> Connector<In> for ConnectProxyConnector {
 
         let mut w = TransportAdapter::new(transport);
 
-        let uri = &details.uri;
-        uri.ensure_valid_url()?;
+        // unwrap is ok because run.rs will construct the ConnectionDetails
+        // such that CONNECT proxy _must_ have the `proxied` field set.
+        let proxied = details.proxied.unwrap();
+        proxied.ensure_valid_url()?;
 
-        // All these unwrap() are ok because ensure_valid_uri() above checks them.
-        let host = uri.host().unwrap();
-        let port = uri
+        let proxied_host = proxied.host().unwrap();
+        let proxied_port = proxied
             .port_u16()
-            .unwrap_or(uri.scheme().unwrap().default_port().unwrap());
+            .unwrap_or(proxied.scheme().unwrap().default_port().unwrap());
 
-        write!(w, "CONNECT {}:{} HTTP/1.1\r\n", host, port)?;
+        write!(w, "CONNECT {}:{} HTTP/1.1\r\n", proxied_host, proxied_port)?;
         write!(w, "Host: {}:{}\r\n", proxy.host(), proxy.port())?;
         if let Some(v) = details.config.user_agent().as_str(DEFAULT_USER_AGENT) {
             write!(w, "User-Agent: {}\r\n", v)?;
diff --git a/src/run.rs b/src/run.rs
index b561fd5..a3c77e9 100644
--- a/src/run.rs
+++ b/src/run.rs
@@ -315,23 +315,25 @@ fn add_headers(
 fn connect(
     agent: &Agent,
     config: &Config,
-    uri: &Uri,
+    wanted_uri: &Uri,
     timings: &mut CallTimings,
 ) -> Result<Connection, Error> {
     // If we're using a CONNECT proxy, we need to resolve that hostname.
     let maybe_connect_uri = config.connect_proxy_uri();
 
-    let effective_uri = maybe_connect_uri.unwrap_or(uri);
+    let (uri, proxied) = if let Some(connect_uri) = maybe_connect_uri {
+        (connect_uri, Some(wanted_uri))
+    } else {
+        (wanted_uri, None)
+    };
 
     // Before resolving the URI we need to ensure it is a full URI. We
     // cannot make requests with partial uri like "/path".
-    effective_uri.ensure_valid_url()?;
+    uri.ensure_valid_url()?;
 
-    let addrs = agent.resolver.resolve(
-        effective_uri,
-        config,
-        timings.next_timeout(Timeout::Resolve),
-    )?;
+    let addrs = agent
+        .resolver
+        .resolve(uri, config, timings.next_timeout(Timeout::Resolve))?;
 
     timings.record_time(Timeout::Resolve);
 
@@ -342,6 +344,7 @@ fn connect(
         config,
         now: timings.now(),
         timeout: timings.next_timeout(Timeout::Connect),
+        proxied,
     };
 
     let connection = agent.pool.connect(&details, config.max_idle_age().into())?;
diff --git a/src/unversioned/transport/mod.rs b/src/unversioned/transport/mod.rs
index 5c94dac..96930cc 100644
--- a/src/unversioned/transport/mod.rs
+++ b/src/unversioned/transport/mod.rs
@@ -173,6 +173,9 @@ where
 /// The parameters needed to create a [`Transport`].
 pub struct ConnectionDetails<'a> {
     /// Full uri that is being requested.
+    ///
+    /// In the case of CONNECT (HTTP) proxy, this is the URI of the
+    /// proxy, and the actual URI is in the `proxied` field.
     pub uri: &'a Uri,
 
     /// The resolved IP address + port for the uri being requested. See [`Resolver`].
@@ -196,6 +199,14 @@ pub struct ConnectionDetails<'a> {
     /// The next timeout for making the connection.
     // TODO(martin): Make mechanism to lower duration for each step in the connector chain.
     pub timeout: NextTimeout,
+
+    /// In case of CONNECT (HTTP) proxy, this is the actual requested
+    /// uri that will go through the proxy.
+    ///
+    /// This ends up in the connect line like `CONNECT host:port HTTP/1.1`.
+    ///
+    /// For socks proxy it is `None`.
+    pub proxied: Option<&'a Uri>,
 }
 
 impl<'a> ConnectionDetails<'a> {
@@ -349,6 +360,8 @@ impl Default for DefaultConnector {
             crate::tls::TlsProvider::NativeTls,
         ));
 
+        // If this is a CONNECT proxy, we must "prepare" the socket
+        // by sending the `CONNECT host:port` line.
         let inner = inner.chain(ConnectProxyConnector::default());
 
         DefaultConnector {


Test patch:
diff --git a/src/unversioned/transport/test.rs b/src/unversioned/transport/test.rs
index 9f6c444..b1be72e 100644
--- a/src/unversioned/transport/test.rs
+++ b/src/unversioned/transport/test.rs
@@ -2,12 +2,12 @@
 
 use std::cell::RefCell;
 use std::io::Write;
-use std::io::{BufRead, BufReader};
 use std::sync::mpsc::{self, Receiver, RecvTimeoutError};
 use std::sync::{Arc, Mutex};
 use std::{fmt, io, thread};
 
 use http::{Method, Request, Uri};
+use ureq_proto::parser::try_parse_request;
 
 use crate::http;
 use crate::Error;
@@ -37,6 +37,7 @@ impl<In: Transport> Connector<In> for TestConnector {
         let config = details.config;
 
         let uri = details.uri.clone();
+        debug!("Test uri: {}", uri);
 
         let buffers = LazyBuffers::new(config.input_buffer_size(), config.output_buffer_size());
 
@@ -110,14 +111,13 @@ fn test_run(
     tx: mpsc::SyncSender<Vec<u8>>,
     handlers: Vec<TestHandler>,
 ) {
-    let mut reader = BufReader::new(RxRead(rx));
+    let mut reader = SaneBufReader(Some(RxRead(rx)), vec![]);
     let mut writer = TxWrite(tx);
     let uri_s = uri.to_string();
 
     let req = loop {
         let input = reader.fill_buf().expect("test fill_buf");
-        let maybe =
-            ureq_proto::parser::try_parse_request::<100>(input).expect("test parse request");
+        let maybe = try_parse_request::<100>(input).expect("test parse request");
         if let Some((amount, req)) = maybe {
             reader.consume(amount);
             break req;
@@ -338,6 +338,25 @@ fn setup_default_handlers(handlers: &mut Vec<TestHandler>) {
         handlers,
     );
 
+    maybe_add(
+        TestHandler::new("/connect-proxy", |_uri, req, w| {
+            assert_eq!(req.uri(), "httpbin.org:80");
+            write!(
+                w,
+                "HTTP/1.1 200 OK\r\n\
+                \r\n\
+                HTTP/1.1 200 OK\r\n\
+                Content-Type: application/json\r\n\
+                Content-Length: {}\r\n\
+                \r\n",
+                HTTPBIN_GET.as_bytes().len()
+            )?;
+            w.write_all(HTTPBIN_GET.as_bytes())?;
+            Ok(())
+        }),
+        handlers,
+    );
+
     #[cfg(feature = "charset")]
     {
         let (cow, _, _) =
@@ -439,6 +458,46 @@ impl io::Write for TxWrite {
     }
 }
 
+struct SaneBufReader<R: io::Read>(Option<R>, Vec<u8>);
+
+impl<R: io::Read> io::Read for SaneBufReader<R> {
+    fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
+        if !self.1.is_empty() {
+            let max = buf.len().min(self.1.len());
+            buf[..max].copy_from_slice(&self.1[..max]);
+            self.1.drain(..max);
+            return Ok(max);
+        }
+
+        let Some(reader) = &mut self.0 else {
+            return Ok(0);
+        };
+        reader.read(buf)
+    }
+}
+
+impl<R: io::Read> SaneBufReader<R> {
+    pub fn fill_buf(&mut self) -> io::Result<&[u8]> {
+        let Some(reader) = &mut self.0 else {
+            return Ok(&self.1);
+        };
+
+        let l = self.1.len();
+        self.1.resize(l + 1024, 0);
+        let buf = &mut self.1[l..];
+        let n = reader.read(buf)?;
+        if n == 0 {
+            self.0 = None;
+        }
+        self.1.truncate(l + n);
+        Ok(&self.1)
+    }
+
+    pub fn consume(&mut self, n: usize) {
+        self.1.drain(..n);
+    }
+}
+
 pub(crate) struct TestTransport {
     buffers: LazyBuffers,
     tx: mpsc::SyncSender<Vec<u8>>,