diff --git a/src/lib.rs b/src/lib.rs index 92777e592..c2840863b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1427,10 +1427,10 @@ impl Url { /// # } /// # run().unwrap(); /// ``` - pub fn set_port(&mut self, mut port: Option) -> Result<(), ()> { + pub fn set_port(&mut self, mut port: Option) -> Result<(), ParseError> { // has_host implies !cannot_be_a_base if !self.has_host() || self.host() == Some(Host::Domain("")) || self.scheme() == "file" { - return Err(()); + return Err(ParseError::EmptyHost); } if port.is_some() && port == parser::default_port(self.scheme()) { port = None @@ -1679,9 +1679,9 @@ impl Url { /// # run().unwrap(); /// ``` /// - pub fn set_ip_host(&mut self, address: IpAddr) -> Result<(), ()> { + pub fn set_ip_host(&mut self, address: IpAddr) -> Result<(), ParseError> { if self.cannot_be_a_base() { - return Err(()); + return Err(ParseError::SetHostOnCannotBeABaseUrl); } let address = match address { @@ -1718,10 +1718,10 @@ impl Url { /// # } /// # run().unwrap(); /// ``` - pub fn set_password(&mut self, password: Option<&str>) -> Result<(), ()> { + pub fn set_password(&mut self, password: Option<&str>) -> Result<(), ParseError> { // has_host implies !cannot_be_a_base if !self.has_host() || self.host() == Some(Host::Domain("")) || self.scheme() == "file" { - return Err(()); + return Err(ParseError::EmptyHost); } if let Some(password) = password { let host_and_after = self.slice(self.host_start..).to_owned(); @@ -1810,10 +1810,10 @@ impl Url { /// # } /// # run().unwrap(); /// ``` - pub fn set_username(&mut self, username: &str) -> Result<(), ()> { + pub fn set_username(&mut self, username: &str) -> Result<(), ParseError> { // has_host implies !cannot_be_a_base if !self.has_host() || self.host() == Some(Host::Domain("")) || self.scheme() == "file" { - return Err(()); + return Err(ParseError::EmptyHost); } let username_start = self.scheme_end + 3; debug_assert!(self.slice(self.scheme_end..username_start) == "://"); @@ -1919,13 +1919,13 @@ impl Url { /// # } /// # run().unwrap(); /// ``` - pub fn set_scheme(&mut self, scheme: &str) -> Result<(), ()> { + pub fn set_scheme(&mut self, scheme: &str) -> Result<(), ParseError> { let mut parser = Parser::for_setter(String::new()); let remaining = parser.parse_scheme(parser::Input::new(scheme))?; if !remaining.is_empty() || (!self.has_host() && SchemeType::from(&parser.serialization).is_special()) { - return Err(()); + return Err(ParseError::InvalidScheme); } let old_scheme_end = self.scheme_end; let new_scheme_end = to_u32(parser.serialization.len()).unwrap(); @@ -1964,7 +1964,7 @@ impl Url { /// # if cfg!(unix) { /// use url::Url; /// - /// # fn run() -> Result<(), ()> { + /// # fn run() -> Result<(), url::ParseError> { /// let url = Url::from_file_path("/tmp/foo.txt")?; /// assert_eq!(url.as_str(), "file:///tmp/foo.txt"); /// @@ -1979,7 +1979,7 @@ impl Url { /// # } /// ``` #[cfg(any(unix, windows, target_os = "redox"))] - pub fn from_file_path>(path: P) -> Result { + pub fn from_file_path>(path: P) -> Result { let mut serialization = "file://".to_owned(); let host_start = serialization.len() as u32; let (host_end, host) = path_to_file_url_segments(path.as_ref(), &mut serialization)?; @@ -2015,7 +2015,7 @@ impl Url { /// Note that `std::path` does not consider trailing slashes significant /// and usually does not include them (e.g. in `Path::parent()`). #[cfg(any(unix, windows, target_os = "redox"))] - pub fn from_directory_path>(path: P) -> Result { + pub fn from_directory_path>(path: P) -> Result { let mut url = Url::from_file_path(path)?; if !url.serialization.ends_with('/') { url.serialization.push('/') @@ -2124,26 +2124,26 @@ impl Url { /// let path = url.to_file_path(); /// ``` /// - /// Returns `Err` if the host is neither empty nor `"localhost"` (except on Windows, where + /// Returns `Err(ParseError::InvalidLocalPath)` if the host is neither empty nor `"localhost"` (except on Windows, where /// `file:` URLs may have a non-local host), /// or if `Path::new_opt()` returns `None`. /// (That is, if the percent-decoded path contains a NUL byte or, /// for a Windows path, is not UTF-8.) #[inline] #[cfg(any(unix, windows, target_os = "redox"))] - pub fn to_file_path(&self) -> Result { + pub fn to_file_path(&self) -> Result { if let Some(segments) = self.path_segments() { let host = match self.host() { None | Some(Host::Domain("localhost")) => None, Some(_) if cfg!(windows) && self.scheme() == "file" => { Some(&self.serialization[self.host_start as usize..self.host_end as usize]) } - _ => return Err(()), + _ => return Err(ParseError::InvalidLocalPath), }; return file_url_segments_to_pathbuf(host, segments); } - Err(()) + Err(ParseError::InvalidLocalPath) } // Private helper methods: @@ -2293,10 +2293,10 @@ impl<'de> serde::Deserialize<'de> for Url { fn path_to_file_url_segments( path: &Path, serialization: &mut String, -) -> Result<(u32, HostInternal), ()> { +) -> Result<(u32, HostInternal), ParseError> { use std::os::unix::prelude::OsStrExt; if !path.is_absolute() { - return Err(()); + return Err(ParseError::PathNotAbsolute); } let host_end = to_u32(serialization.len()).unwrap(); let mut empty = true; @@ -2320,7 +2320,7 @@ fn path_to_file_url_segments( fn path_to_file_url_segments( path: &Path, serialization: &mut String, -) -> Result<(u32, HostInternal), ()> { +) -> Result<(u32, HostInternal), ParseError> { path_to_file_url_segments_windows(path, serialization) } @@ -2329,10 +2329,10 @@ fn path_to_file_url_segments( fn path_to_file_url_segments_windows( path: &Path, serialization: &mut String, -) -> Result<(u32, HostInternal), ()> { +) -> Result<(u32, HostInternal), ParseError> { use std::path::{Component, Prefix}; if !path.is_absolute() { - return Err(()); + return Err(ParseError::PathNotAbsolute); } let mut components = path.components(); @@ -2348,18 +2348,18 @@ fn path_to_file_url_segments_windows( serialization.push(':'); } Prefix::UNC(server, share) | Prefix::VerbatimUNC(server, share) => { - let host = Host::parse(server.to_str().ok_or(())?).map_err(|_| ())?; + let host = Host::parse(server.to_str().ok_or(ParseError::InvalidLocalPath)?)?; write!(serialization, "{}", host).unwrap(); host_end = to_u32(serialization.len()).unwrap(); host_internal = host.into(); serialization.push('/'); - let share = share.to_str().ok_or(())?; + let share = share.to_str().ok_or(ParseError::InvalidLocalPath)?; serialization.extend(percent_encode(share.as_bytes(), PATH_SEGMENT)); } - _ => return Err(()), + _ => return Err(ParseError::InvalidLocalPath), }, - _ => return Err(()), + _ => return Err(ParseError::InvalidLocalPath), } for component in components { @@ -2367,7 +2367,7 @@ fn path_to_file_url_segments_windows( continue; } // FIXME: somehow work with non-unicode? - let component = component.as_os_str().to_str().ok_or(())?; + let component = component.as_os_str().to_str().ok_or(ParseError::InvalidLocalPath)?; serialization.push('/'); serialization.extend(percent_encode(component.as_bytes(), PATH_SEGMENT)); } @@ -2378,12 +2378,12 @@ fn path_to_file_url_segments_windows( fn file_url_segments_to_pathbuf( host: Option<&str>, segments: str::Split, -) -> Result { +) -> Result { use std::ffi::OsStr; use std::os::unix::prelude::OsStrExt; if host.is_some() { - return Err(()); + return Err(ParseError::InvalidLocalPath); } let mut bytes = if cfg!(target_os = "redox") { @@ -2408,7 +2408,7 @@ fn file_url_segments_to_pathbuf( fn file_url_segments_to_pathbuf( host: Option<&str>, segments: str::Split, -) -> Result { +) -> Result { file_url_segments_to_pathbuf_windows(host, segments) } @@ -2417,16 +2417,16 @@ fn file_url_segments_to_pathbuf( fn file_url_segments_to_pathbuf_windows( host: Option<&str>, mut segments: str::Split, -) -> Result { +) -> Result { let mut string = if let Some(host) = host { r"\\".to_owned() + host } else { - let first = segments.next().ok_or(())?; + let first = segments.next().ok_or(ParseError::InvalidLocalPath)?; match first.len() { 2 => { if !first.starts_with(parser::ascii_alpha) || first.as_bytes()[1] != b':' { - return Err(()); + return Err(ParseError::InvalidLocalPath); } first.to_owned() @@ -2434,17 +2434,17 @@ fn file_url_segments_to_pathbuf_windows( 4 => { if !first.starts_with(parser::ascii_alpha) { - return Err(()); + return Err(ParseError::InvalidLocalPath); } let bytes = first.as_bytes(); if bytes[1] != b'%' || bytes[2] != b'3' || (bytes[3] != b'a' && bytes[3] != b'A') { - return Err(()); + return Err(ParseError::InvalidLocalPath); } first[0..1].to_owned() + ":" } - _ => return Err(()), + _ => return Err(ParseError::InvalidLocalPath), } }; @@ -2454,7 +2454,7 @@ fn file_url_segments_to_pathbuf_windows( // Currently non-unicode windows paths cannot be represented match String::from_utf8(percent_decode(segment.as_bytes()).collect()) { Ok(s) => string.push_str(&s), - Err(..) => return Err(()), + Err(..) => return Err(ParseError::InvalidLocalPath), } } let path = PathBuf::from(string); diff --git a/src/parser.rs b/src/parser.rs index 96906f94a..66aa002e5 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -72,6 +72,7 @@ simple_enum_error! { EmptyHost => "empty host", IdnaError => "invalid international domain name", InvalidPort => "invalid port number", + InvalidScheme => "invalid scheme", InvalidIpv4Address => "invalid IPv4 address", InvalidIpv6Address => "invalid IPv6 address", InvalidDomainCharacter => "invalid domain character", @@ -79,6 +80,8 @@ simple_enum_error! { RelativeUrlWithCannotBeABaseBase => "relative URL with a cannot-be-a-base base", SetHostOnCannotBeABaseUrl => "a cannot-be-a-base URL doesn’t have a host to set", Overflow => "URLs more than 4 GB are not supported", + InvalidLocalPath => "the url is not a valid local path", + PathNotAbsolute => "path component of url is not absolute", } impl fmt::Display for ParseError { @@ -357,9 +360,9 @@ impl<'a> Parser<'a> { } } - pub fn parse_scheme<'i>(&mut self, mut input: Input<'i>) -> Result, ()> { + pub fn parse_scheme<'i>(&mut self, mut input: Input<'i>) -> Result, ParseError> { if input.is_empty() || !input.starts_with(ascii_alpha) { - return Err(()); + return Err(ParseError::InvalidScheme); } debug_assert!(self.serialization.is_empty()); while let Some(c) = input.next() { @@ -370,7 +373,7 @@ impl<'a> Parser<'a> { ':' => return Ok(input), _ => { self.serialization.clear(); - return Err(()); + return Err(ParseError::InvalidScheme); } } } @@ -379,7 +382,7 @@ impl<'a> Parser<'a> { Ok(input) } else { self.serialization.clear(); - Err(()) + Err(ParseError::InvalidScheme) } } diff --git a/src/quirks.rs b/src/quirks.rs index 285ee21b6..fb3e6637d 100644 --- a/src/quirks.rs +++ b/src/quirks.rs @@ -56,7 +56,7 @@ pub fn protocol(url: &Url) -> &str { } /// Setter for https://url.spec.whatwg.org/#dom-url-protocol -pub fn set_protocol(url: &mut Url, mut new_protocol: &str) -> Result<(), ()> { +pub fn set_protocol(url: &mut Url, mut new_protocol: &str) -> Result<(), ParseError> { // The scheme state in the spec ignores everything after the first `:`, // but `set_scheme` errors if there is more. if let Some(position) = new_protocol.find(':') { @@ -72,7 +72,7 @@ pub fn username(url: &Url) -> &str { } /// Setter for https://url.spec.whatwg.org/#dom-url-username -pub fn set_username(url: &mut Url, new_username: &str) -> Result<(), ()> { +pub fn set_username(url: &mut Url, new_username: &str) -> Result<(), ParseError> { url.set_username(new_username) } @@ -83,7 +83,7 @@ pub fn password(url: &Url) -> &str { } /// Setter for https://url.spec.whatwg.org/#dom-url-password -pub fn set_password(url: &mut Url, new_password: &str) -> Result<(), ()> { +pub fn set_password(url: &mut Url, new_password: &str) -> Result<(), ParseError> { url.set_password(if new_password.is_empty() { None } else { diff --git a/tests/unit.rs b/tests/unit.rs index d5e81986a..70a8de4c3 100644 --- a/tests/unit.rs +++ b/tests/unit.rs @@ -15,7 +15,7 @@ use std::borrow::Cow; use std::cell::{Cell, RefCell}; use std::net::{Ipv4Addr, Ipv6Addr}; use std::path::{Path, PathBuf}; -use url::{form_urlencoded, Host, Url}; +use url::{form_urlencoded, Host, ParseError, Url}; #[test] fn size() { @@ -38,14 +38,32 @@ macro_rules! assert_from_file_path { #[test] fn new_file_paths() { if cfg!(unix) { - assert_eq!(Url::from_file_path(Path::new("relative")), Err(())); - assert_eq!(Url::from_file_path(Path::new("../relative")), Err(())); + assert_eq!( + Url::from_file_path(Path::new("relative")), + Err(ParseError::PathNotAbsolute) + ); + assert_eq!( + Url::from_file_path(Path::new("../relative")), + Err(ParseError::PathNotAbsolute) + ); } if cfg!(windows) { - assert_eq!(Url::from_file_path(Path::new("relative")), Err(())); - assert_eq!(Url::from_file_path(Path::new(r"..\relative")), Err(())); - assert_eq!(Url::from_file_path(Path::new(r"\drive-relative")), Err(())); - assert_eq!(Url::from_file_path(Path::new(r"\\ucn\")), Err(())); + assert_eq!( + Url::from_file_path(Path::new("relative")), + Err(ParseError::PathNotAbsolute) + ); + assert_eq!( + Url::from_file_path(Path::new(r"..\relative")), + Err(ParseError::PathNotAbsolute) + ); + assert_eq!( + Url::from_file_path(Path::new(r"\drive-relative")), + Err(ParseError::PathNotAbsolute) + ); + assert_eq!( + Url::from_file_path(Path::new(r"\\ucn\")), + Err(ParseError::PathNotAbsolute) + ); } if cfg!(unix) { @@ -90,22 +108,38 @@ fn new_path_windows_fun() { #[test] fn new_directory_paths() { + use url::ParseError; if cfg!(unix) { - assert_eq!(Url::from_directory_path(Path::new("relative")), Err(())); - assert_eq!(Url::from_directory_path(Path::new("../relative")), Err(())); + assert_eq!( + Url::from_directory_path(Path::new("relative")), + Err(ParseError::PathNotAbsolute) + ); + assert_eq!( + Url::from_directory_path(Path::new("../relative")), + Err(ParseError::PathNotAbsolute) + ); let url = Url::from_directory_path(Path::new("/foo/bar")).unwrap(); assert_eq!(url.host(), None); assert_eq!(url.path(), "/foo/bar/"); } if cfg!(windows) { - assert_eq!(Url::from_directory_path(Path::new("relative")), Err(())); - assert_eq!(Url::from_directory_path(Path::new(r"..\relative")), Err(())); + assert_eq!( + Url::from_directory_path(Path::new("relative")), + Err(ParseError::PathNotAbsolute) + ); + assert_eq!( + Url::from_directory_path(Path::new(r"..\relative")), + Err(ParseError::PathNotAbsolute) + ); assert_eq!( Url::from_directory_path(Path::new(r"\drive-relative")), - Err(()) + Err(ParseError::PathNotAbsolute) + ); + assert_eq!( + Url::from_directory_path(Path::new(r"\\ucn\")), + Err(ParseError::PathNotAbsolute) ); - assert_eq!(Url::from_directory_path(Path::new(r"\\ucn\")), Err(())); let url = Url::from_directory_path(Path::new(r"C:\foo\bar")).unwrap(); assert_eq!(url.host(), None);