Repository: servo/rust-url
Pull request #825 (2023-03-14 16:43:27)

No colon when setting empty password
This addresses https://github.com/servo/rust-url/issues/796 by removing the colon (`:`) when setting the password to `Some("")`. According to the [URL Standard](https://url.spec.whatwg.org/#url-serializing):

> If url’s [password](https://url.spec.whatwg.org/#concept-url-password) is not the empty string, then append U+003A (:), followed by url’s [password](https://url.spec.whatwg.org/#concept-url-password), to output.

This cherry-picks https://github.com/servo/rust-url/pull/806 to add some tests for this.

codecov[bot]: ## [Codecov](https://codecov.io/gh/servo/rust-url/pull/825?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=servo) Report
Patch coverage: **`100.00`**% and project coverage change: **`+0.02`** :tada:
> Comparison is base [(`1092960`)](https://codecov.io/gh/servo/rust-url/commit/10929606359afa54e363ef7ebf311f106247f5d9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=servo) 82.73% compared to head [(`f2842a5`)](https://codecov.io/gh/servo/rust-url/pull/825?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=servo) 82.76%.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master     #825      +/-   ##
==========================================
+ Coverage   82.73%   82.76%   +0.02%     
==========================================
  Files          20       20              
  Lines        3337     3337              
==========================================
+ Hits         2761     2762       +1     
+ Misses        576      575       -1     
```


| [Impacted Files](https://codecov.io/gh/servo/rust-url/pull/825?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=servo) | Coverage Δ | |
|---|---|---|
| [url/src/host.rs](https://codecov.io/gh/servo/rust-url/pull/825?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=servo#diff-dXJsL3NyYy9ob3N0LnJz) | `96.95% <100.00%> (-0.02%)` | :arrow_down: |
| [url/src/lib.rs](https://codecov.io/gh/servo/rust-url/pull/825?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=servo#diff-dXJsL3NyYy9saWIucnM=) | `76.51% <100.00%> (+0.03%)` | :arrow_up: |

... and [1 file with indirect coverage changes](https://codecov.io/gh/servo/rust-url/pull/825/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=servo)

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=servo). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=servo)

</details>

[:umbrella: View full report in Codecov by Sentry](https://codecov.io/gh/servo/rust-url/pull/825?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=servo).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=servo).


Patch:
diff --git a/url/src/host.rs b/url/src/host.rs
index f1921c6..9931c2f 100644
--- a/url/src/host.rs
+++ b/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 {
diff --git a/url/src/lib.rs b/url/src/lib.rs
index 7624b6f..65e965e 100644
--- a/url/src/lib.rs
+++ b/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(':');
diff --git a/url/src/parser.rs b/url/src/parser.rs
index ca0865c..0553caa 100644
--- a/url/src/parser.rs
+++ b/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)
 // " # % < > [ \ ] ^ ` { | }


Test patch:
diff --git a/url/tests/unit.rs b/url/tests/unit.rs
index 5affa16..8957aaf 100644
--- a/url/tests/unit.rs
+++ b/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)