Repository: hatoo/oha (rust)

Issue:
IPv6 address support for `--connect-to`
I wrote the code for this so I'm to blame 🙈  but it just splits across `:` and expects 4 tokens, which obviously doesn't work for IPv6 addresses.

Since curl supports it, and oha's feature is modelled after curl's, I think it should support IPv6 syntax with brackets:

```shell
$ curl -I https://example.org --connect-to 'example.org:443:[2606:2800:220:1:248:1893:25c8:1946]:443'
HTTP/2 200
content-encoding: gzip
accept-ranges: bytes
age: 295951
cache-control: max-age=604800
content-type: text/html; charset=UTF-8
date: Fri, 27 May 2022 09:52:13 GMT
etag: "3147526947+gzip"
expires: Fri, 03 Jun 2022 09:52:13 GMT
last-modified: Thu, 17 Oct 2019 07:18:26 GMT
server: ECS (bsa/EB23)
x-cache: HIT
content-length: 648
```

Pull request:
Add support for bracketed IPv6 syntax in connect-to
Closes #166

This also takes care of a clippy warning (hold mutex guard across await)
by switching tests to tokio's `Mutex` rather than libstd's.

Patch:
diff --git a/src/main.rs b/src/main.rs
index a93c85b..bceb889 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -125,22 +125,34 @@ pub struct ConnectToEntry {
 }
 
 impl FromStr for ConnectToEntry {
-    type Err = &'static str;
+    type Err = String;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let tokens: Vec<&str> = s.split(':').collect();
-        if tokens.len() != 4 {
-            return Err("must have 4 items separated by colons");
-        }
+        let expected_syntax: String =
+            "syntax for --connect-to is host:port:target_host:target_port".into();
+
+        let (requested_host, s) = s.split_once(':').ok_or_else(|| expected_syntax.clone())?;
+        let (requested_port, s) = s.split_once(':').ok_or_else(|| expected_syntax.clone())?;
+
+        let (target_host, target_port) = if s.starts_with('[') {
+            // parse bracketed IPv6 address
+            let end = s.find("]:").ok_or(expected_syntax)?;
+            let target_host = &s[1..end];
+            let target_port = &s[end + 2..]; // shouldn't panic because we looked for `]:`, not just `]`
+            (target_host, target_port)
+        } else {
+            s.split_once(':').ok_or_else(|| expected_syntax.clone())?
+        };
+
         Ok(ConnectToEntry {
-            requested_host: tokens[0].into(),
-            requested_port: tokens[1]
-                .parse()
-                .map_err(|_| "requested port must be an u16")?,
-            target_host: tokens[2].into(),
-            target_port: tokens[3]
-                .parse()
-                .map_err(|_| "target port must be an u16")?,
+            requested_host: requested_host.into(),
+            requested_port: requested_port.parse().map_err(|err| {
+                format!("requested port must be an u16, but got {requested_port}: {err}")
+            })?,
+            target_host: target_host.into(),
+            target_port: target_port.parse().map_err(|err| {
+                format!("target port must be an u16, but got {target_port}: {err}")
+            })?,
         })
     }
 }


Test patch:
diff --git a/tests/tests.rs b/tests/tests.rs
index 649507f..58c6a6d 100644
--- a/tests/tests.rs
+++ b/tests/tests.rs
@@ -1,6 +1,8 @@
+use std::net::Ipv6Addr;
+
 use assert_cmd::Command;
 use get_port::Ops;
-use std::sync::Mutex;
+use tokio::sync::Mutex;
 use warp::{http::HeaderMap, Filter};
 
 lazy_static::lazy_static! {
@@ -17,7 +19,7 @@ async fn get_header_body(args: &[&str]) -> (HeaderMap, bytes::Bytes) {
             "Hello World"
         });
 
-    let _guard = PORT_LOCK.lock().unwrap();
+    let _guard = PORT_LOCK.lock().await;
     let port = get_port::tcp::TcpPort::any("127.0.0.1").unwrap();
     tokio::spawn(warp::serve(report_headers).run(([127, 0, 0, 1], port)));
     // It's not guaranteed that the port is used here.
@@ -48,7 +50,7 @@ async fn get_method(args: &[&str]) -> http::method::Method {
         },
     );
 
-    let _guard = PORT_LOCK.lock().unwrap();
+    let _guard = PORT_LOCK.lock().await;
     let port = get_port::tcp::TcpPort::any("127.0.0.1").unwrap();
     tokio::spawn(warp::serve(report_headers).run(([127, 0, 0, 1], port)));
     // It's not guaranteed that the port is used here.
@@ -79,7 +81,7 @@ async fn get_query(p: &'static str) -> String {
         },
     );
 
-    let _guard = PORT_LOCK.lock().unwrap();
+    let _guard = PORT_LOCK.lock().await;
     let port = get_port::tcp::TcpPort::any("127.0.0.1").unwrap();
     tokio::spawn(warp::serve(report_headers).run(([127, 0, 0, 1], port)));
     // It's not guaranteed that the port is used here.
@@ -101,7 +103,7 @@ async fn get_query(p: &'static str) -> String {
 
 async fn redirect(n: usize, is_relative: bool, limit: usize) -> bool {
     let (tx, rx) = flume::unbounded();
-    let _guard = PORT_LOCK.lock().unwrap();
+    let _guard = PORT_LOCK.lock().await;
     let port = get_port::tcp::TcpPort::any("127.0.0.1").unwrap();
 
     let route = warp::path!(usize).map(move |x| {
@@ -152,7 +154,7 @@ async fn get_host_with_connect_to(host: &'static str) -> String {
                 "Hello World"
             });
 
-    let _guard = PORT_LOCK.lock().unwrap();
+    let _guard = PORT_LOCK.lock().await;
     let port = get_port::tcp::TcpPort::any("127.0.0.1").unwrap();
     tokio::spawn(warp::serve(report_host).run(([127, 0, 0, 1], port)));
     // It's not guaranteed that the port is used here.
@@ -174,6 +176,41 @@ async fn get_host_with_connect_to(host: &'static str) -> String {
     rx.try_recv().unwrap()
 }
 
+async fn get_host_with_connect_to_ipv6(host: &'static str) -> String {
+    let (tx, rx) = flume::unbounded();
+    let report_host =
+        warp::get()
+            .and(warp::filters::header::header("host"))
+            .map(move |host: String| {
+                tx.send(host).unwrap();
+                "Hello World"
+            });
+
+    let _guard = PORT_LOCK.lock().await;
+    // sic. the `get_port` crate doesn't support IpV6 addresses, so we check
+    // with 127.0.0.1 even though we bind on ::1 later.
+    let port = get_port::tcp::TcpPort::any("127.0.0.1").unwrap();
+    let addr = Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1);
+    tokio::spawn(warp::serve(report_host).run((addr, port)));
+    // It's not guaranteed that the port is used here.
+    // So we can't drop guard here.
+
+    tokio::task::spawn_blocking(move || {
+        Command::cargo_bin("oha")
+            .unwrap()
+            .args(&["-n", "1", "--no-tui"])
+            .arg(format!("http://{}/", host))
+            .arg("--connect-to")
+            .arg(format!("{host}:80:[{addr}]:{port}"))
+            .assert()
+            .success();
+    })
+    .await
+    .unwrap();
+
+    rx.try_recv().unwrap()
+}
+
 async fn get_host_with_connect_to_redirect(host: &'static str) -> String {
     let (tx, rx) = flume::unbounded();
     let redirect = warp::get().and(warp::path!("source")).map(move || {
@@ -189,7 +226,7 @@ async fn get_host_with_connect_to_redirect(host: &'static str) -> String {
         });
     let routes = redirect.or(report_host);
 
-    let _guard = PORT_LOCK.lock().unwrap();
+    let _guard = PORT_LOCK.lock().await;
     let port = get_port::tcp::TcpPort::any("127.0.0.1").unwrap();
     tokio::spawn(warp::serve(routes).run(([127, 0, 0, 1], port)));
     // It's not guaranteed that the port is used here.
@@ -322,6 +359,14 @@ async fn test_connect_to() {
     )
 }
 
+#[tokio::test]
+async fn test_connect_to_ipv6() {
+    assert_eq!(
+        get_host_with_connect_to_ipv6("invalid.example.org").await,
+        "invalid.example.org"
+    )
+}
+
 #[tokio::test]
 async fn test_connect_to_redirect() {
     assert_eq!(