diff --git a/cli/args/flags_net.rs b/cli/args/flags_net.rs index 57cf8d527c..88ffcf0e46 100644 --- a/cli/args/flags_net.rs +++ b/cli/args/flags_net.rs @@ -50,7 +50,7 @@ pub fn parse(paths: Vec) -> clap::error::Result> { out.push(format!("{}:{}", host, port.0)); } } else { - host_and_port.parse::().map_err(|e| { + NetDescriptor::parse(&host_and_port).map_err(|e| { clap::Error::raw(clap::error::ErrorKind::InvalidValue, format!("{e:?}")) })?; out.push(host_and_port) diff --git a/runtime/ops/permissions.rs b/runtime/ops/permissions.rs index 9ac9205e91..e6974efadf 100644 --- a/runtime/ops/permissions.rs +++ b/runtime/ops/permissions.rs @@ -1,6 +1,7 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. use ::deno_permissions::parse_sys_kind; +use ::deno_permissions::NetDescriptor; use ::deno_permissions::PermissionState; use ::deno_permissions::PermissionsContainer; use deno_core::error::custom_error; @@ -63,7 +64,7 @@ pub fn op_query_permission( "net" => permissions.net.query( match args.host.as_deref() { None => None, - Some(h) => Some(h.parse()?), + Some(h) => Some(NetDescriptor::parse(h)?), } .as_ref(), ), @@ -97,7 +98,7 @@ pub fn op_revoke_permission( "net" => permissions.net.revoke( match args.host.as_deref() { None => None, - Some(h) => Some(h.parse()?), + Some(h) => Some(NetDescriptor::parse(h)?), } .as_ref(), ), @@ -131,7 +132,7 @@ pub fn op_request_permission( "net" => permissions.net.request( match args.host.as_deref() { None => None, - Some(h) => Some(h.parse()?), + Some(h) => Some(NetDescriptor::parse(h)?), } .as_ref(), ), diff --git a/runtime/permissions/clippy.toml b/runtime/permissions/clippy.toml new file mode 100644 index 0000000000..b5ed912cce --- /dev/null +++ b/runtime/permissions/clippy.toml @@ -0,0 +1,3 @@ +disallowed-methods = [ + { path = "std::str::FromStr", reason = "Don't want to have stuff like `'0.0.0.0'.parse().unwrap()`. Instead implement ConcreteType::parse methods." }, +] diff --git a/runtime/permissions/lib.rs b/runtime/permissions/lib.rs index da9e493d3f..c5cfbff703 100644 --- a/runtime/permissions/lib.rs +++ b/runtime/permissions/lib.rs @@ -29,7 +29,6 @@ use std::net::IpAddr; use std::net::Ipv6Addr; use std::path::Path; use std::path::PathBuf; -use std::str::FromStr; use std::string::ToString; use std::sync::Arc; @@ -683,10 +682,9 @@ pub enum Host { Ip(IpAddr), } -impl FromStr for Host { - type Err = AnyError; - - fn from_str(s: &str) -> Result { +impl Host { + // TODO(bartlomieju): rewrite to not use `AnyError` but a specific error implementations + fn parse(s: &str) -> Result { if s.starts_with('[') && s.ends_with(']') { let ip = s[1..s.len() - 1] .parse::() @@ -708,14 +706,23 @@ impl FromStr for Host { } else { Cow::Owned(s.to_ascii_lowercase()) }; - let fqdn = FQDN::from_str(&lower) - .with_context(|| format!("invalid host: '{s}'"))?; + let fqdn = { + use std::str::FromStr; + FQDN::from_str(&lower) + .with_context(|| format!("invalid host: '{s}'"))? + }; if fqdn.is_root() { return Err(uri_error(format!("invalid empty host: '{s}'"))); } Ok(Host::Fqdn(fqdn)) } } + + #[cfg(test)] + #[track_caller] + fn must_parse(s: &str) -> Self { + Self::parse(s).unwrap() + } } #[derive(Clone, Eq, PartialEq, Hash, Debug)] @@ -750,10 +757,9 @@ impl Descriptor for NetDescriptor { } } -impl FromStr for NetDescriptor { - type Err = AnyError; - - fn from_str(hostname: &str) -> Result { +// TODO(bartlomieju): rewrite to not use `AnyError` but a specific error implementations +impl NetDescriptor { + pub fn parse(hostname: &str) -> Result { // If this is a IPv6 address enclosed in square brackets, parse it as such. if hostname.starts_with('[') { if let Some((ip, after)) = hostname.split_once(']') { @@ -784,7 +790,7 @@ impl FromStr for NetDescriptor { Some((host, port)) => (host, port), None => (hostname, ""), }; - let host = host.parse::()?; + let host = Host::parse(host)?; let port = if port.is_empty() { None @@ -1222,7 +1228,7 @@ impl UnaryPermission { let host = url .host_str() .ok_or_else(|| type_error(format!("Missing host in url: '{}'", url)))?; - let host = host.parse::()?; + let host = Host::parse(host)?; let port = url.port_or_known_default(); let descriptor = NetDescriptor(host, port); self.check_desc(Some(&descriptor), false, api_name, || { @@ -1865,7 +1871,7 @@ impl PermissionsContainer { host: &(T, Option), api_name: &str, ) -> Result<(), AnyError> { - let hostname = host.0.as_ref().parse::()?; + let hostname = Host::parse(host.0.as_ref())?; let descriptor = NetDescriptor(hostname, host.1); self.0.lock().net.check(&descriptor, Some(api_name)) } @@ -1914,7 +1920,7 @@ fn parse_net_list( ) -> Result, AnyError> { if let Some(v) = list { v.iter() - .map(|x| NetDescriptor::from_str(x)) + .map(|x| NetDescriptor::parse(x)) .collect::, AnyError>>() } else { Ok(HashSet::new()) @@ -2480,7 +2486,7 @@ mod tests { ]; for (host, port, is_ok) in domain_tests { - let host = host.parse().unwrap(); + let host = Host::parse(host).unwrap(); let descriptor = NetDescriptor(host, Some(port)); assert_eq!( is_ok, @@ -2522,7 +2528,7 @@ mod tests { ]; for (host_str, port) in domain_tests { - let host = host_str.parse().unwrap(); + let host = Host::parse(host_str).unwrap(); let descriptor = NetDescriptor(host, Some(port)); assert!( perms.net.check(&descriptor, None).is_ok(), @@ -2563,7 +2569,7 @@ mod tests { ]; for (host_str, port) in domain_tests { - let host = host_str.parse().unwrap(); + let host = Host::parse(host_str).unwrap(); let descriptor = NetDescriptor(host, Some(port)); assert!( perms.net.check(&descriptor, None).is_err(), @@ -2835,14 +2841,14 @@ mod tests { assert_eq!(perms4.ffi.query(Some(Path::new("/foo/bar"))), PermissionState::Denied); assert_eq!(perms4.ffi.query(Some(Path::new("/bar"))), PermissionState::Granted); assert_eq!(perms1.net.query(None), PermissionState::Granted); - assert_eq!(perms1.net.query(Some(&NetDescriptor("127.0.0.1".parse().unwrap(), None))), PermissionState::Granted); + assert_eq!(perms1.net.query(Some(&NetDescriptor(Host::must_parse("127.0.0.1"), None))), PermissionState::Granted); assert_eq!(perms2.net.query(None), PermissionState::Prompt); - assert_eq!(perms2.net.query(Some(&NetDescriptor("127.0.0.1".parse().unwrap(), Some(8000)))), PermissionState::Granted); + assert_eq!(perms2.net.query(Some(&NetDescriptor(Host::must_parse("127.0.0.1"), Some(8000)))), PermissionState::Granted); assert_eq!(perms3.net.query(None), PermissionState::Prompt); - assert_eq!(perms3.net.query(Some(&NetDescriptor("127.0.0.1".parse().unwrap(), Some(8000)))), PermissionState::Denied); + assert_eq!(perms3.net.query(Some(&NetDescriptor(Host::must_parse("127.0.0.1"), Some(8000)))), PermissionState::Denied); assert_eq!(perms4.net.query(None), PermissionState::GrantedPartial); - assert_eq!(perms4.net.query(Some(&NetDescriptor("127.0.0.1".parse().unwrap(), Some(8000)))), PermissionState::Denied); - assert_eq!(perms4.net.query(Some(&NetDescriptor("192.168.0.1".parse().unwrap(), Some(8000)))), PermissionState::Granted); + assert_eq!(perms4.net.query(Some(&NetDescriptor(Host::must_parse("127.0.0.1"), Some(8000)))), PermissionState::Denied); + assert_eq!(perms4.net.query(Some(&NetDescriptor(Host::must_parse("192.168.0.1"), Some(8000)))), PermissionState::Granted); assert_eq!(perms1.env.query(None), PermissionState::Granted); assert_eq!(perms1.env.query(Some("HOME")), PermissionState::Granted); assert_eq!(perms2.env.query(None), PermissionState::Prompt); @@ -2896,9 +2902,9 @@ mod tests { prompt_value.set(true); assert_eq!(perms.ffi.request(None), PermissionState::Denied); prompt_value.set(true); - assert_eq!(perms.net.request(Some(&NetDescriptor("127.0.0.1".parse().unwrap(), None))), PermissionState::Granted); + assert_eq!(perms.net.request(Some(&NetDescriptor(Host::must_parse("127.0.0.1"), None))), PermissionState::Granted); prompt_value.set(false); - assert_eq!(perms.net.request(Some(&NetDescriptor("127.0.0.1".parse().unwrap(), Some(8000)))), PermissionState::Granted); + assert_eq!(perms.net.request(Some(&NetDescriptor(Host::must_parse("127.0.0.1"), Some(8000)))), PermissionState::Granted); prompt_value.set(true); assert_eq!(perms.env.request(Some("HOME")), PermissionState::Granted); assert_eq!(perms.env.query(None), PermissionState::Prompt); @@ -2967,9 +2973,9 @@ mod tests { assert_eq!(perms.ffi.revoke(Some(Path::new("/foo/bar"))), PermissionState::Prompt); assert_eq!(perms.ffi.query(Some(Path::new("/foo"))), PermissionState::Prompt); assert_eq!(perms.ffi.query(Some(Path::new("/foo/baz"))), PermissionState::Granted); - assert_eq!(perms.net.revoke(Some(&NetDescriptor("127.0.0.1".parse().unwrap(), Some(9000)))), PermissionState::Prompt); - assert_eq!(perms.net.query(Some(&NetDescriptor("127.0.0.1".parse().unwrap(), None))), PermissionState::Prompt); - assert_eq!(perms.net.query(Some(&NetDescriptor("127.0.0.1".parse().unwrap(), Some(8000)))), PermissionState::Granted); + assert_eq!(perms.net.revoke(Some(&NetDescriptor(Host::must_parse("127.0.0.1"), Some(9000)))), PermissionState::Prompt); + assert_eq!(perms.net.query(Some(&NetDescriptor(Host::must_parse("127.0.0.1"), None))), PermissionState::Prompt); + assert_eq!(perms.net.query(Some(&NetDescriptor(Host::must_parse("127.0.0.1"), Some(8000)))), PermissionState::Granted); assert_eq!(perms.env.revoke(Some("HOME")), PermissionState::Prompt); assert_eq!(perms.env.revoke(Some("hostname")), PermissionState::Prompt); assert_eq!(perms.run.revoke(Some("deno")), PermissionState::Prompt); @@ -3004,7 +3010,7 @@ mod tests { assert!(perms .net .check( - &NetDescriptor("127.0.0.1".parse().unwrap(), Some(8000)), + &NetDescriptor(Host::must_parse("127.0.0.1"), Some(8000)), None ) .is_ok()); @@ -3012,31 +3018,31 @@ mod tests { assert!(perms .net .check( - &NetDescriptor("127.0.0.1".parse().unwrap(), Some(8000)), + &NetDescriptor(Host::must_parse("127.0.0.1"), Some(8000)), None ) .is_ok()); assert!(perms .net .check( - &NetDescriptor("127.0.0.1".parse().unwrap(), Some(8001)), + &NetDescriptor(Host::must_parse("127.0.0.1"), Some(8001)), None ) .is_err()); assert!(perms .net - .check(&NetDescriptor("127.0.0.1".parse().unwrap(), None), None) + .check(&NetDescriptor(Host::must_parse("127.0.0.1"), None), None) .is_err()); assert!(perms .net .check( - &NetDescriptor("deno.land".parse().unwrap(), Some(8000)), + &NetDescriptor(Host::must_parse("deno.land"), Some(8000)), None ) .is_err()); assert!(perms .net - .check(&NetDescriptor("deno.land".parse().unwrap(), None), None) + .check(&NetDescriptor(Host::must_parse("deno.land"), None), None) .is_err()); #[allow(clippy::disallowed_methods)] @@ -3121,7 +3127,7 @@ mod tests { assert!(perms .net .check( - &NetDescriptor("127.0.0.1".parse().unwrap(), Some(8000)), + &NetDescriptor(Host::must_parse("127.0.0.1"), Some(8000)), None ) .is_err()); @@ -3129,21 +3135,21 @@ mod tests { assert!(perms .net .check( - &NetDescriptor("127.0.0.1".parse().unwrap(), Some(8000)), + &NetDescriptor(Host::must_parse("127.0.0.1"), Some(8000)), None ) .is_err()); assert!(perms .net .check( - &NetDescriptor("127.0.0.1".parse().unwrap(), Some(8001)), + &NetDescriptor(Host::must_parse("127.0.0.1"), Some(8001)), None ) .is_ok()); assert!(perms .net .check( - &NetDescriptor("deno.land".parse().unwrap(), Some(8000)), + &NetDescriptor(Host::must_parse("deno.land"), Some(8000)), None ) .is_ok()); @@ -3151,14 +3157,14 @@ mod tests { assert!(perms .net .check( - &NetDescriptor("127.0.0.1".parse().unwrap(), Some(8001)), + &NetDescriptor(Host::must_parse("127.0.0.1"), Some(8001)), None ) .is_ok()); assert!(perms .net .check( - &NetDescriptor("deno.land".parse().unwrap(), Some(8000)), + &NetDescriptor(Host::must_parse("deno.land"), Some(8000)), None ) .is_ok()); @@ -3286,24 +3292,24 @@ mod tests { perms .net .check( - &NetDescriptor("allowed.domain.".parse().unwrap(), None), + &NetDescriptor(Host::must_parse("allowed.domain."), None), None, ) .unwrap(); perms .net - .check(&NetDescriptor("1.1.1.1".parse().unwrap(), None), None) + .check(&NetDescriptor(Host::must_parse("1.1.1.1"), None), None) .unwrap(); assert!(perms .net .check( - &NetDescriptor("denied.domain.".parse().unwrap(), None), + &NetDescriptor(Host::must_parse("denied.domain."), None), None ) .is_err()); assert!(perms .net - .check(&NetDescriptor("2.2.2.2".parse().unwrap(), None), None) + .check(&NetDescriptor(Host::must_parse("2.2.2.2"), None), None) .is_err()); } @@ -3599,7 +3605,7 @@ mod tests { ]; for (host_str, expected) in hosts { - assert_eq!(host_str.parse::().ok(), *expected, "{host_str}"); + assert_eq!(Host::parse(host_str).ok(), *expected, "{host_str}"); } } @@ -3665,7 +3671,7 @@ mod tests { ]; for (input, expected) in cases { - assert_eq!(input.parse::().ok(), *expected, "'{input}'"); + assert_eq!(NetDescriptor::parse(input).ok(), *expected, "'{input}'"); } } }