Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add url.includes_credentials() and url.is_special(). #520

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
86 changes: 75 additions & 11 deletions src/lib.rs
Expand Up @@ -477,7 +477,7 @@ impl Url {
assert_eq!(host_str, h.to_string())
}
HostInternal::Domain => {
if SchemeType::from(self.scheme()).is_special() {
if self.is_special() {
assert!(!host_str.is_empty())
}
}
Expand Down Expand Up @@ -1497,8 +1497,7 @@ impl Url {
/// # run().unwrap();
/// ```
pub fn set_port(&mut self, mut port: Option<u16>) -> Result<(), ()> {
// has_host implies !cannot_be_a_base
if !self.has_host() || self.host() == Some(Host::Domain("")) || self.scheme() == "file" {
if self.cannot_have_username_password_port() {
return Err(());
}
if port.is_some() && port == parser::default_port(self.scheme()) {
Expand Down Expand Up @@ -1635,7 +1634,7 @@ impl Url {
}

if let Some(host) = host {
if host == "" && SchemeType::from(self.scheme()).is_special() {
if host == "" && self.is_special() {
return Err(ParseError::EmptyHost);
}
let mut host_substr = host;
Expand All @@ -1653,14 +1652,13 @@ impl Url {
None => {}
}
}
if SchemeType::from(self.scheme()).is_special() {
if self.is_special() {
self.set_host_internal(Host::parse(host_substr)?, None);
} else {
self.set_host_internal(Host::parse_opaque(host_substr)?, None);
}
} else if self.has_host() {
let scheme_type = SchemeType::from(self.scheme());
if scheme_type.is_special() {
if self.is_special() {
return Err(ParseError::EmptyHost);
} else {
if self.serialization.len() == self.path_start as usize {
Expand Down Expand Up @@ -1808,8 +1806,7 @@ impl Url {
/// # run().unwrap();
/// ```
pub fn set_password(&mut self, password: Option<&str>) -> Result<(), ()> {
// has_host implies !cannot_be_a_base
if !self.has_host() || self.host() == Some(Host::Domain("")) || self.scheme() == "file" {
if self.cannot_have_username_password_port() {
return Err(());
}
if let Some(password) = password {
Expand Down Expand Up @@ -1900,8 +1897,7 @@ impl Url {
/// # run().unwrap();
/// ```
pub fn set_username(&mut self, username: &str) -> Result<(), ()> {
// has_host implies !cannot_be_a_base
if !self.has_host() || self.host() == Some(Host::Domain("")) || self.scheme() == "file" {
if self.cannot_have_username_password_port() {
return Err(());
}
let username_start = self.scheme_end + 3;
Expand Down Expand Up @@ -2103,6 +2099,68 @@ impl Url {
Ok(())
}

/// Return whether the URL is special.
///
/// A URL is special if its scheme is one of the following:
///
/// "http" | "https" | "ws" | "wss" | "ftp" | "gopher" | "file"
///
/// # Examples
///
/// ```
/// use url::Url;
/// # use url::ParseError;
///
/// # fn run() -> Result<(), ParseError> {
/// let url = Url::parse("ftp://rms@example.com")?;
/// assert!(url.is_special());
///
/// let url = Url::parse("file://foo.bar")?;
/// assert!(url.is_special());
///
/// let url = Url::parse("unix:/run/foo.socket")?;
/// assert!(!url.is_special());
///
/// let url = Url::parse("data:text/plain,Stuff")?;
/// assert!(!url.is_special());
/// # Ok(())
/// # }
/// # run().unwrap();
/// ```
pub fn is_special(&self) -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand it, "special" is really about the scheme (which matches the docs), whereas a Url::is_special() method seems to be about the whole URL. Should this be renamed to is_special_scheme() or is_scheme_special() or has_special_scheme()?

SchemeType::from(self.scheme()).is_special()
}

/// Return whether the URL includes credentials.
///
/// A URL includes credentials if its username or password is not the empty string.
///
/// # Examples
///
/// ```
/// use url::Url;
/// # use url::ParseError;
///
/// # fn run() -> Result<(), ParseError> {
/// let url = Url::parse("https://username:password@www.my_site.com")?;
/// assert!(url.includes_credentials());
///
/// let url = Url::parse("https://username@www.my_site.com")?;
/// assert!(url.includes_credentials());
///
/// let url = Url::parse("https://www.my_site.com")?;
/// assert!(!url.includes_credentials());
///
/// let url = Url::parse("https://@www.my_site.com")?;
/// assert!(!url.includes_credentials());
/// # Ok(())
/// # }
/// # run().unwrap();
/// ```
pub fn includes_credentials(&self) -> bool {
self.username() != "" || self.password().unwrap_or(&"") != ""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused about this code, is it even possible to have a password without username?

}

/// Convert a file name as `std::path::Path` into an URL in the `file` scheme.
///
/// This returns `Err` if the given path is not absolute or,
Expand Down Expand Up @@ -2300,6 +2358,12 @@ impl Url {

// Private helper methods:

fn cannot_have_username_password_port(&self) -> bool {
self.host().unwrap_or(Host::Domain("")) == Host::Domain("")
Copy link
Contributor

@djc djc Aug 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to link https://url.spec.whatwg.org/#url-miscellaneous in a comment here.

|| self.cannot_be_a_base()
|| self.scheme() == "file"
}

#[inline]
fn slice<R>(&self, range: R) -> &str
where
Expand Down