Skip to content

set_scheme should fail if url has host or port #366

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 40 additions & 10 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ impl Url {
/// ```rust
/// use url::Url;
/// # use url::ParseError;
///
///
/// # fn run() -> Result<(), ParseError> {
/// let base = Url::parse("https://example.net/a/b.html")?;
/// let url = base.join("c.png")?;
Expand Down Expand Up @@ -729,6 +729,28 @@ impl Url {
!matches!(self.host, HostInternal::None)
}

/// Equivalent to `url.port().is_some()`.
///
/// # Examples
///
/// ```
/// use url::Url;
/// # use url::ParseError;
///
/// # fn run() -> Result<(), ParseError> {
/// let url = Url::parse("http://example.com:8080")?;
/// assert!(url.has_port());
///
/// let url = Url::parse("file:///tmp/something")?;
/// assert!(!url.has_port());
/// # Ok(())
/// # }
/// # run().unwrap();
/// ```
pub fn has_port(&self) -> bool {
self.port.is_some()
}

/// Return the string representation of the host (domain or IP address) for this URL, if any.
///
/// Non-ASCII domains are punycode-encoded per IDNA.
Expand Down Expand Up @@ -1310,7 +1332,7 @@ impl Url {
/// ```
/// use url::Url;
/// # use url::ParseError;
///
///
/// # fn run() -> Result<(), ParseError> {
/// let mut url = Url::parse("foo://example.net")?;
/// let result = url.set_host(None);
Expand All @@ -1326,7 +1348,7 @@ impl Url {
/// ```
/// use url::Url;
/// # use url::ParseError;
///
///
/// # fn run() -> Result<(), ParseError> {
/// let mut url = Url::parse("https://example.net")?;
/// let result = url.set_host(None);
Expand All @@ -1342,7 +1364,7 @@ impl Url {
/// ```
/// use url::Url;
/// # use url::ParseError;
///
///
/// # fn run() -> Result<(), ParseError> {
/// let mut url = Url::parse("mailto:[email protected]")?;
///
Expand Down Expand Up @@ -1552,7 +1574,7 @@ impl Url {
/// ```
/// use url::Url;
/// # use url::ParseError;
///
///
/// # fn run() -> Result<(), ParseError> {
/// let mut url = Url::parse("https://example.net")?;
/// let result = url.set_scheme("foo");
Expand All @@ -1569,7 +1591,7 @@ impl Url {
/// ```
/// use url::Url;
/// # use url::ParseError;
///
///
/// # fn run() -> Result<(), ParseError> {
/// let mut url = Url::parse("https://example.net")?;
/// let result = url.set_scheme("foõ");
Expand All @@ -1585,7 +1607,7 @@ impl Url {
/// ```
/// use url::Url;
/// # use url::ParseError;
///
///
/// # fn run() -> Result<(), ParseError> {
/// let mut url = Url::parse("mailto:[email protected]")?;
/// let result = url.set_scheme("https");
Expand All @@ -1598,8 +1620,8 @@ impl Url {
pub fn set_scheme(&mut self, scheme: &str) -> Result<(), ()> {
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()) {
if !(remaining.is_empty() &&
self.can_use_scheme(&parser.serialization)) {
return Err(())
}
let old_scheme_end = self.scheme_end;
Expand All @@ -1622,6 +1644,14 @@ impl Url {
Ok(())
}

fn can_use_scheme(&self, scheme: &str) -> bool {
match SchemeType::from(scheme) {
SchemeType::File => !(self.has_host() || self.has_port()),
SchemeType::SpecialNotFile => self.has_host(),
SchemeType::NotSpecial => true
}
}

/// 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 All @@ -1634,7 +1664,7 @@ impl Url {
/// ```
/// # if cfg!(unix) {
/// use url::Url;
///
///
/// # fn run() -> Result<(), ()> {
/// let url = Url::from_file_path("/tmp/foo.txt")?;
/// assert_eq!(url.as_str(), "file:///tmp/foo.txt");
Expand Down
7 changes: 7 additions & 0 deletions tests/unit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,13 @@ fn issue_241() {
Url::parse("mailto:").unwrap().cannot_be_a_base();
}

#[test]
fn issue_278() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if this is the best place to put the unit test.

let mut url: Url = "http://localhost:6767/foo/bar".parse().unwrap();
assert_eq!(url.to_string(), "http://localhost:6767/foo/bar");
assert_eq!(url.set_scheme("file"), Err(()));
}

#[test]
/// https://github.com/servo/rust-url/issues/222
fn append_trailing_slash() {
Expand Down