No colon when setting empty password (#825)

* set empty password tests

* Fix empty password should not keep colon

* Add test for setting empty query

* Address clippy::manual_is_ascii_check

* No point in decoding UTF-8 to check ASCII

* Remove obsolete is_ascii_hex_digit

---------

Co-authored-by: Ricardo Monteiro <ricardo.monteiro@getmanta.com>

url/src/host.rs

@@ -269,7 +269,7 @@ fn ends_in_a_number(input: &str) -> bool {
     } else {
         last
     };
-    if !last.is_empty() && last.chars().all(|c| ('0'..='9').contains(&c)) {
+    if !last.is_empty() && last.as_bytes().iter().all(|c| c.is_ascii_digit()) {
         return true;
     }
 
@@ -297,11 +297,9 @@ fn parse_ipv4number(mut input: &str) -> Result<Option<u32>, ()> {
     }
 
     let valid_number = match r {
-        8 => input.chars().all(|c| ('0'..='7').contains(&c)),
-        10 => input.chars().all(|c| ('0'..='9').contains(&c)),
-        16 => input.chars().all(|c| {
-            ('0'..='9').contains(&c) || ('a'..='f').contains(&c) || ('A'..='F').contains(&c)
-        }),
+        8 => input.as_bytes().iter().all(|c| (b'0'..=b'7').contains(c)),
+        10 => input.as_bytes().iter().all(|c| c.is_ascii_digit()),
+        16 => input.as_bytes().iter().all(|c| c.is_ascii_hexdigit()),
         _ => false,
     };
     if !valid_number {

url/src/lib.rs

@@ -2069,7 +2069,8 @@ impl Url {
         if !self.has_host() || self.host() == Some(Host::Domain("")) || self.scheme() == "file" {
             return Err(());
         }
-        if let Some(password) = password {
+        let password = password.unwrap_or_default();
+        if !password.is_empty() {
             let host_and_after = self.slice(self.host_start..).to_owned();
             self.serialization.truncate(self.username_end as usize);
             self.serialization.push(':');

url/src/parser.rs

@@ -1517,7 +1517,7 @@ impl<'a> Parser<'a> {
             if c == '%' {
                 let mut input = input.clone();
                 if !matches!((input.next(), input.next()), (Some(a), Some(b))
-                             if is_ascii_hex_digit(a) && is_ascii_hex_digit(b))
+                             if a.is_ascii_hexdigit() && b.is_ascii_hexdigit())
                 {
                     vfn(SyntaxViolation::PercentDecode)
                 }
@@ -1528,11 +1528,6 @@ impl<'a> Parser<'a> {
     }
 }
 
-#[inline]
-fn is_ascii_hex_digit(c: char) -> bool {
-    matches!(c, 'a'..='f' | 'A'..='F' | '0'..='9')
-}
-
 // Non URL code points:
 // U+0000 to U+0020 (space)
 // " # % < > [ \ ] ^ ` { | }

url/tests/unit.rs

@@ -64,6 +64,30 @@ fn test_set_empty_host() {
     assert_eq!(base.as_str(), "file://foo/share/foo/bar");
 }
 
+#[test]
+fn test_set_empty_username_and_password() {
+    let mut base: Url = "moz://foo:bar@servo/baz".parse().unwrap();
+    base.set_username("").unwrap();
+    assert_eq!(base.as_str(), "moz://:bar@servo/baz");
+
+    base.set_password(Some("")).unwrap();
+    assert_eq!(base.as_str(), "moz://servo/baz");
+
+    base.set_password(None).unwrap();
+    assert_eq!(base.as_str(), "moz://servo/baz");
+}
+
+#[test]
+fn test_set_empty_password() {
+    let mut base: Url = "moz://foo:bar@servo/baz".parse().unwrap();
+
+    base.set_password(Some("")).unwrap();
+    assert_eq!(base.as_str(), "moz://foo@servo/baz");
+
+    base.set_password(None).unwrap();
+    assert_eq!(base.as_str(), "moz://foo@servo/baz");
+}
+
 #[test]
 fn test_set_empty_hostname() {
     use url::quirks;
@@ -82,6 +106,17 @@ fn test_set_empty_hostname() {
     assert_eq!(base.as_str(), "moz:///baz");
 }
 
+#[test]
+fn test_set_empty_query() {
+    let mut base: Url = "moz://example.com/path?query".parse().unwrap();
+
+    base.set_query(Some(""));
+    assert_eq!(base.as_str(), "moz://example.com/path?");
+
+    base.set_query(None);
+    assert_eq!(base.as_str(), "moz://example.com/path");
+}
+
 macro_rules! assert_from_file_path {
     ($path: expr) => {
         assert_from_file_path!($path, $path)