Re-land "fix(cli): ipv6 parsing for --allow-net params" (#6472)

With some minor adjustments

cli/flags.rs

@@ -974,7 +974,8 @@ fn permission_args<'a, 'b>(app: App<'a, 'b>) -> App<'a, 'b> {
         .takes_value(true)
         .use_delimiter(true)
         .require_equals(true)
-        .help("Allow network access"),
+        .help("Allow network access")
+        .validator(crate::flags_allow_net::validator),
     )
     .arg(
       Arg::with_name("allow-env")
@@ -1324,7 +1325,8 @@ fn permission_args_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
     if raw_net_allowlist.is_empty() {
       flags.allow_net = true;
     } else {
-      flags.net_allowlist = resolve_hosts(raw_net_allowlist);
+      flags.net_allowlist =
+        crate::flags_allow_net::parse(raw_net_allowlist).unwrap();
       debug!("net allowlist: {:#?}", &flags.net_allowlist);
     }
   }
@@ -1375,41 +1377,6 @@ pub fn resolve_urls(urls: Vec<String>) -> Vec<String> {
   out
 }
 
-/// Expands "bare port" paths (eg. ":8080") into full paths with hosts. It
-/// expands to such paths into 3 paths with following hosts: `0.0.0.0:port`,
-/// `127.0.0.1:port` and `localhost:port`.
-fn resolve_hosts(paths: Vec<String>) -> Vec<String> {
-  let mut out: Vec<String> = vec![];
-  for host_and_port in paths.iter() {
-    let parts = host_and_port.split(':').collect::<Vec<&str>>();
-
-    match parts.len() {
-      // host only
-      1 => {
-        out.push(host_and_port.to_owned());
-      }
-      // host and port (NOTE: host might be empty string)
-      2 => {
-        let host = parts[0];
-        let port = parts[1];
-
-        if !host.is_empty() {
-          out.push(host_and_port.to_owned());
-          continue;
-        }
-
-        // we got bare port, let's add default hosts
-        for host in ["0.0.0.0", "127.0.0.1", "localhost"].iter() {
-          out.push(format!("{}:{}", host, port));
-        }
-      }
-      _ => panic!("Bad host:port pair: {}", host_and_port),
-    }
-  }
-
-  out
-}
-
 #[cfg(test)]
 mod tests {
   use super::*;
@@ -2481,6 +2448,37 @@ mod tests {
     );
   }
 
+  #[test]
+  fn allow_net_allowlist_with_ipv6_address() {
+    let r = flags_from_vec_safe(svec![
+      "deno",
+      "run",
+      "--allow-net=deno.land,deno.land:80,::,127.0.0.1,[::1],1.2.3.4:5678,:5678,[::1]:8080",
+      "script.ts"
+    ]);
+    assert_eq!(
+      r.unwrap(),
+      Flags {
+        subcommand: DenoSubcommand::Run {
+          script: "script.ts".to_string(),
+        },
+        net_allowlist: svec![
+          "deno.land",
+          "deno.land:80",
+          "::",
+          "127.0.0.1",
+          "[::1]",
+          "1.2.3.4:5678",
+          "0.0.0.0:5678",
+          "127.0.0.1:5678",
+          "localhost:5678",
+          "[::1]:8080"
+        ],
+        ..Flags::default()
+      }
+    );
+  }
+
   #[test]
   fn lock_write() {
     let r = flags_from_vec_safe(svec![

cli/flags_allow_net.rs

@@ -0,0 +1,199 @@
+use std::net::IpAddr;
+use std::str::FromStr;
+use url::Url;
+
+#[derive(Debug, PartialEq, Eq)]
+pub struct ParsePortError(String);
+
+#[derive(Debug, PartialEq, Eq)]
+pub struct BarePort(u16);
+
+impl FromStr for BarePort {
+  type Err = ParsePortError;
+  fn from_str(s: &str) -> Result<BarePort, ParsePortError> {
+    if s.starts_with(':') {
+      match s.split_at(1).1.parse::<u16>() {
+        Ok(port) => Ok(BarePort(port)),
+        Err(e) => Err(ParsePortError(e.to_string())),
+      }
+    } else {
+      Err(ParsePortError(
+        "Bare Port doesn't start with ':'".to_string(),
+      ))
+    }
+  }
+}
+
+pub fn validator(host_and_port: String) -> Result<(), String> {
+  if Url::parse(&format!("deno://{}", host_and_port)).is_ok()
+    || host_and_port.parse::<IpAddr>().is_ok()
+    || host_and_port.parse::<BarePort>().is_ok()
+  {
+    Ok(())
+  } else {
+    Err(format!("Bad host:port pair: {}", host_and_port))
+  }
+}
+
+/// Expands "bare port" paths (eg. ":8080") into full paths with hosts. It
+/// expands to such paths into 3 paths with following hosts: `0.0.0.0:port`,
+/// `127.0.0.1:port` and `localhost:port`.
+pub fn parse(paths: Vec<String>) -> clap::Result<Vec<String>> {
+  let mut out: Vec<String> = vec![];
+  for host_and_port in paths.iter() {
+    if Url::parse(&format!("deno://{}", host_and_port)).is_ok()
+      || host_and_port.parse::<IpAddr>().is_ok()
+    {
+      out.push(host_and_port.to_owned())
+    } else if let Ok(port) = host_and_port.parse::<BarePort>() {
+      // we got bare port, let's add default hosts
+      for host in ["0.0.0.0", "127.0.0.1", "localhost"].iter() {
+        out.push(format!("{}:{}", host, port.0));
+      }
+    } else {
+      return Err(clap::Error::with_description(
+        &format!("Bad host:port pair: {}", host_and_port),
+        clap::ErrorKind::InvalidValue,
+      ));
+    }
+  }
+  Ok(out)
+}
+
+#[cfg(test)]
+mod bare_port_tests {
+  use super::{BarePort, ParsePortError};
+
+  #[test]
+  fn bare_port_parsed() {
+    let expected = BarePort(8080);
+    let actual = ":8080".parse::<BarePort>();
+    assert_eq!(actual, Ok(expected));
+  }
+
+  #[test]
+  fn bare_port_parse_error1() {
+    let expected =
+      ParsePortError("Bare Port doesn't start with ':'".to_string());
+    let actual = "8080".parse::<BarePort>();
+    assert_eq!(actual, Err(expected));
+  }
+
+  #[test]
+  fn bare_port_parse_error2() {
+    let actual = ":65536".parse::<BarePort>();
+    assert!(actual.is_err());
+  }
+
+  #[test]
+  fn bare_port_parse_error3() {
+    let actual = ":14u16".parse::<BarePort>();
+    assert!(actual.is_err());
+  }
+
+  #[test]
+  fn bare_port_parse_error4() {
+    let actual = "Deno".parse::<BarePort>();
+    assert!(actual.is_err());
+  }
+
+  #[test]
+  fn bare_port_parse_error5() {
+    let actual = "deno.land:8080".parse::<BarePort>();
+    assert!(actual.is_err());
+  }
+}
+
+#[cfg(test)]
+mod tests {
+  use super::parse;
+
+  // Creates vector of strings, Vec<String>
+  macro_rules! svec {
+      ($($x:expr),*) => (vec![$($x.to_string()),*]);
+  }
+
+  #[test]
+  fn parse_net_args_() {
+    let entries = svec![
+      "deno.land",
+      "deno.land:80",
+      "::",
+      "::1",
+      "127.0.0.1",
+      "[::1]",
+      "1.2.3.4:5678",
+      "0.0.0.0:5678",
+      "127.0.0.1:5678",
+      "[::]:5678",
+      "[::1]:5678",
+      "localhost:5678",
+      "[::1]:8080",
+      "[::]:8000",
+      "[::1]:8000",
+      "localhost:8000",
+      "0.0.0.0:4545",
+      "127.0.0.1:4545",
+      "999.0.88.1:80"
+    ];
+    let expected = svec![
+      "deno.land",
+      "deno.land:80",
+      "::",
+      "::1",
+      "127.0.0.1",
+      "[::1]",
+      "1.2.3.4:5678",
+      "0.0.0.0:5678",
+      "127.0.0.1:5678",
+      "[::]:5678",
+      "[::1]:5678",
+      "localhost:5678",
+      "[::1]:8080",
+      "[::]:8000",
+      "[::1]:8000",
+      "localhost:8000",
+      "0.0.0.0:4545",
+      "127.0.0.1:4545",
+      "999.0.88.1:80"
+    ];
+    let actual = parse(entries).unwrap();
+    assert_eq!(actual, expected);
+  }
+
+  #[test]
+  fn parse_net_args_expansion() {
+    let entries = svec![":8080"];
+    let expected = svec!["0.0.0.0:8080", "127.0.0.1:8080", "localhost:8080"];
+    let actual = parse(entries).unwrap();
+    assert_eq!(actual, expected);
+  }
+
+  #[test]
+  fn parse_net_args_ipv6() {
+    let entries =
+      svec!["::", "::1", "[::1]", "[::]:5678", "[::1]:5678", "::cafe"];
+    let expected =
+      svec!["::", "::1", "[::1]", "[::]:5678", "[::1]:5678", "::cafe"];
+    let actual = parse(entries).unwrap();
+    assert_eq!(actual, expected);
+  }
+
+  #[test]
+  fn parse_net_args_ipv6_error1() {
+    let entries = svec![":::"];
+    assert!(parse(entries).is_err());
+  }
+
+  #[test]
+  fn parse_net_args_ipv6_error2() {
+    let entries = svec!["0123:4567:890a:bcde:fg::"];
+    assert!(parse(entries).is_err());
+  }
+
+  #[test]
+  fn parse_net_args_ipv6_error3() {
+    let entries = svec!["[::q]:8080"];
+    assert!(parse(entries).is_err());
+  }
+}

cli/main.rs

@@ -31,6 +31,7 @@ mod disk_cache;
 mod doc;
 mod file_fetcher;
 pub mod flags;
+mod flags_allow_net;
 mod fmt;
 pub mod fmt_errors;
 mod fs;

cli/tests/complex_permissions_test.ts

@@ -14,7 +14,11 @@ const test: { [key: string]: Function } = {
   },
   netListen(endpoints: string[]): void {
     endpoints.forEach((endpoint) => {
-      const [hostname, port] = endpoint.split(":");
+      const index = endpoint.lastIndexOf(":");
+      const [hostname, port] = [
+        endpoint.substr(0, index),
+        endpoint.substr(index + 1),
+      ];
       const listener = Deno.listen({
         transport: "tcp",
         hostname,
@@ -25,7 +29,11 @@ const test: { [key: string]: Function } = {
   },
   async netConnect(endpoints: string[]): Promise<void> {
     for (const endpoint of endpoints) {
-      const [hostname, port] = endpoint.split(":");
+      const index = endpoint.lastIndexOf(":");
+      const [hostname, port] = [
+        endpoint.substr(0, index),
+        endpoint.substr(index + 1),
+      ];
       const listener = await Deno.connect({
         transport: "tcp",
         hostname,