Repository: servo/rust-url
Pull request #788 (2022-08-29 16:40:21)

feat(url): Add `quirks::internal_components`
Supersedes #762 with added doc comments


codecov-commenter: # [Codecov](https://codecov.io/gh/servo/rust-url/pull/788?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Servo) Report
> Merging [#788](https://codecov.io/gh/servo/rust-url/pull/788?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Servo) (9006e62) into [master](https://codecov.io/gh/servo/rust-url/commit/1d307ae51a28fecc630ecec03380788bfb03a643?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Servo) (1d307ae) will **decrease** coverage by `0.19%`.
> The diff coverage is `0.00%`.

```diff
@@            Coverage Diff             @@
##           master     #788      +/-   ##
==========================================
- Coverage   85.23%   85.03%   -0.20%     
==========================================
  Files          22       22              
  Lines        3894     3903       +9     
==========================================
  Hits         3319     3319              
- Misses        575      584       +9     
```


| [Impacted Files](https://codecov.io/gh/servo/rust-url/pull/788?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Servo) | Coverage Δ | |
|---|---|---|
| [url/src/quirks.rs](https://codecov.io/gh/servo/rust-url/pull/788/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Servo#diff-dXJsL3NyYy9xdWlya3MucnM=) | `90.07% <0.00%> (-6.65%)` | :arrow_down: |

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)

valenting: I would prefer feature-gate this API instead. If that's not possible I can live with it. 🤷‍♂️

Patch:
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index a114a27..2bf71d3 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -49,7 +49,7 @@ jobs:
         uses: actions-rs/cargo@v1
         with:
           command: test
-          args: --features "url/serde"
+          args: --features "url/serde,url/expose_internals"
       # The #[debugger_visualizer] attribute is currently gated behind an unstable feature flag.
       # In order to test the visualizers for the url crate, they have to be tested on a nightly build.
       - name: Run debugger_visualizer tests
diff --git a/idna/src/uts46.rs b/idna/src/uts46.rs
index f3073e0..ec2fd0b 100644
--- a/idna/src/uts46.rs
+++ b/idna/src/uts46.rs
@@ -318,50 +318,48 @@ fn check_validity(label: &str, config: Config, errors: &mut Errors) {
     // V8: Bidi rules are checked inside `processing()`
 }
 
-/// http://www.unicode.org/reports/tr46/#Processing
-fn processing(
-    domain: &str,
-    config: Config,
-    normalized: &mut String,
-    output: &mut String,
-) -> Errors {
-    // Weed out the simple cases: only allow all lowercase ASCII characters and digits where none
-    // of the labels start with PUNYCODE_PREFIX and labels don't start or end with hyphen.
-    let (mut prev, mut simple, mut puny_prefix) = ('?', !domain.is_empty(), 0);
+// Detect simple cases: all lowercase ASCII characters and digits where none
+// of the labels start with PUNYCODE_PREFIX and labels don't start or end with hyphen.
+fn is_simple(domain: &str) -> bool {
+    if domain.is_empty() {
+        return false;
+    }
+    let (mut prev, mut puny_prefix) = ('?', 0);
     for c in domain.chars() {
         if c == '.' {
             if prev == '-' {
-                simple = false;
-                break;
+                return false;
             }
             puny_prefix = 0;
             continue;
         } else if puny_prefix == 0 && c == '-' {
-            simple = false;
-            break;
+            return false;
         } else if puny_prefix < 5 {
             if c == ['x', 'n', '-', '-'][puny_prefix] {
                 puny_prefix += 1;
                 if puny_prefix == 4 {
-                    simple = false;
-                    break;
+                    return false;
                 }
             } else {
                 puny_prefix = 5;
             }
         }
         if !c.is_ascii_lowercase() && !c.is_ascii_digit() {
-            simple = false;
-            break;
+            return false;
         }
         prev = c;
     }
 
-    if simple {
-        output.push_str(domain);
-        return Errors::default();
-    }
+    true
+}
 
+/// http://www.unicode.org/reports/tr46/#Processing
+fn processing(
+    domain: &str,
+    config: Config,
+    normalized: &mut String,
+    output: &mut String,
+) -> Errors {
     normalized.clear();
     let mut errors = Errors::default();
     let offset = output.len();
@@ -447,11 +445,13 @@ impl Idna {
         }
     }
 
-    /// http://www.unicode.org/reports/tr46/#ToASCII
-    #[allow(clippy::wrong_self_convention)]
-    pub fn to_ascii<'a>(&'a mut self, domain: &str, out: &mut String) -> Result<(), Errors> {
-        let mut errors = processing(domain, self.config, &mut self.normalized, &mut self.output);
-
+    pub fn to_ascii_inner(&mut self, domain: &str, out: &mut String) -> Errors {
+        if is_simple(domain) {
+            out.push_str(domain);
+            return Errors::default();
+        }
+        let mut errors = processing(domain, self.config, &mut self.normalized, out);
+        self.output = std::mem::replace(out, String::with_capacity(out.len()));
         let mut first = true;
         for label in self.output.split('.') {
             if !first {
@@ -470,6 +470,13 @@ impl Idna {
                 }
             }
         }
+        errors
+    }
+
+    /// http://www.unicode.org/reports/tr46/#ToASCII
+    #[allow(clippy::wrong_self_convention)]
+    pub fn to_ascii<'a>(&'a mut self, domain: &str, out: &mut String) -> Result<(), Errors> {
+        let mut errors = self.to_ascii_inner(domain, out);
 
         if self.config.verify_dns_length {
             let domain = if out.ends_with('.') {
@@ -491,6 +498,10 @@ impl Idna {
     /// http://www.unicode.org/reports/tr46/#ToUnicode
     #[allow(clippy::wrong_self_convention)]
     pub fn to_unicode<'a>(&'a mut self, domain: &str, out: &mut String) -> Result<(), Errors> {
+        if is_simple(domain) {
+            out.push_str(domain);
+            return Errors::default().into();
+        }
         processing(domain, self.config, &mut self.normalized, out).into()
     }
 }
diff --git a/url/Cargo.toml b/url/Cargo.toml
index 1747e4a..1eaf155 100644
--- a/url/Cargo.toml
+++ b/url/Cargo.toml
@@ -38,6 +38,8 @@ default = ["idna"]
 # UNSTABLE FEATURES (requires Rust nightly)
 # Enable to use the #[debugger_visualizer] attribute.
 debugger_visualizer = []
+# Expose internal offsets of the URL.
+expose_internals = []
 
 [[bench]]
 name = "parse_url"
diff --git a/url/src/lib.rs b/url/src/lib.rs
index 967282e..e359e74 100644
--- a/url/src/lib.rs
+++ b/url/src/lib.rs
@@ -1813,8 +1813,10 @@ impl Url {
             return Err(ParseError::SetHostOnCannotBeABaseUrl);
         }
 
+        let scheme_type = SchemeType::from(self.scheme());
+
         if let Some(host) = host {
-            if host.is_empty() && SchemeType::from(self.scheme()).is_special() {
+            if host.is_empty() && scheme_type.is_special() && !scheme_type.is_file() {
                 return Err(ParseError::EmptyHost);
             }
             let mut host_substr = host;
@@ -1838,15 +1840,20 @@ impl Url {
                 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 scheme_type.is_special() && !scheme_type.is_file() {
                 return Err(ParseError::EmptyHost);
             } else if self.serialization.len() == self.path_start as usize {
                 self.serialization.push('/');
             }
             debug_assert!(self.byte_at(self.scheme_end) == b':');
             debug_assert!(self.byte_at(self.path_start) == b'/');
-            let new_path_start = self.scheme_end + 1;
+
+            let new_path_start = if scheme_type.is_file() {
+                self.scheme_end + 3
+            } else {
+                self.scheme_end + 1
+            };
+
             self.serialization
                 .drain(new_path_start as usize..self.path_start as usize);
             let offset = self.path_start - new_path_start;
@@ -2730,6 +2737,7 @@ fn path_to_file_url_segments_windows(
     let host_start = serialization.len() + 1;
     let host_end;
     let host_internal;
+
     match components.next() {
         Some(Component::Prefix(ref p)) => match p.kind() {
             Prefix::Disk(letter) | Prefix::VerbatimDisk(letter) => {
@@ -2750,7 +2758,6 @@ fn path_to_file_url_segments_windows(
             }
             _ => return Err(()),
         },
-
         _ => return Err(()),
     }
 
@@ -2759,12 +2766,15 @@ fn path_to_file_url_segments_windows(
         if component == Component::RootDir {
             continue;
         }
+
         path_only_has_prefix = false;
         // FIXME: somehow work with non-unicode?
         let component = component.as_os_str().to_str().ok_or(())?;
+
         serialization.push('/');
         serialization.extend(percent_encode(component.as_bytes(), PATH_SEGMENT));
     }
+
     // A windows drive letter must end with a slash.
     if serialization.len() > host_start
         && parser::is_windows_drive_letter(&serialization[host_start..])
@@ -2772,6 +2782,7 @@ fn path_to_file_url_segments_windows(
     {
         serialization.push('/');
     }
+
     Ok((host_end, host_internal))
 }
 
@@ -2795,10 +2806,12 @@ fn file_url_segments_to_pathbuf(
     } else {
         Vec::new()
     };
+
     for segment in segments {
         bytes.push(b'/');
         bytes.extend(percent_decode(segment.as_bytes()));
     }
+
     // A windows drive letter must end with a slash.
     if bytes.len() > 2
         && matches!(bytes[bytes.len() - 2], b'a'..=b'z' | b'A'..=b'Z')
@@ -2806,12 +2819,15 @@ fn file_url_segments_to_pathbuf(
     {
         bytes.push(b'/');
     }
+
     let os_str = OsStr::from_bytes(&bytes);
     let path = PathBuf::from(os_str);
+
     debug_assert!(
         path.is_absolute(),
         "to_file_path() failed to produce an absolute Path"
     );
+
     Ok(path)
 }
 
diff --git a/url/src/quirks.rs b/url/src/quirks.rs
index 3ecb3c2..3a51064 100644
--- a/url/src/quirks.rs
+++ b/url/src/quirks.rs
@@ -14,6 +14,49 @@
 use crate::parser::{default_port, Context, Input, Parser, SchemeType};
 use crate::{Host, ParseError, Position, Url};
 
+/// Internal components / offsets of a URL.
+///
+/// https://user@pass:example.com:1234/foo/bar?baz#quux
+///      |      |    |          | ^^^^|       |   |
+///      |      |    |          | |   |       |   `----- fragment_start
+///      |      |    |          | |   |       `--------- query_start
+///      |      |    |          | |   `----------------- path_start
+///      |      |    |          | `--------------------- port
+///      |      |    |          `----------------------- host_end
+///      |      |    `---------------------------------- host_start
+///      |      `--------------------------------------- username_end
+///      `---------------------------------------------- scheme_end
+#[derive(Copy, Clone)]
+#[cfg(feature = "expose_internals")]
+pub struct InternalComponents {
+    pub scheme_end: u32,
+    pub username_end: u32,
+    pub host_start: u32,
+    pub host_end: u32,
+    pub port: Option<u16>,
+    pub path_start: u32,
+    pub query_start: Option<u32>,
+    pub fragment_start: Option<u32>,
+}
+
+/// Internal component / parsed offsets of the URL.
+///
+/// This can be useful for implementing efficient serialization
+/// for the URL.
+#[cfg(feature = "expose_internals")]
+pub fn internal_components(url: &Url) -> InternalComponents {
+    InternalComponents {
+        scheme_end: url.scheme_end,
+        username_end: url.username_end,
+        host_start: url.host_start,
+        host_end: url.host_end,
+        port: url.port,
+        path_start: url.path_start,
+        query_start: url.query_start,
+        fragment_start: url.fragment_start,
+    }
+}
+
 /// https://url.spec.whatwg.org/#dom-url-domaintoascii
 pub fn domain_to_ascii(domain: &str) -> String {
     match Host::parse(domain) {


Test patch:
diff --git a/url/tests/unit.rs b/url/tests/unit.rs
index 401eaf9..b0fd5a3 100644
--- a/url/tests/unit.rs
+++ b/url/tests/unit.rs
@@ -43,6 +43,14 @@ fn test_set_empty_host() {
     assert_eq!(base.as_str(), "moz:/baz");
     base.set_host(Some("servo")).unwrap();
     assert_eq!(base.as_str(), "moz://servo/baz");
+
+    let mut base: Url = "file://server/share/foo/bar".parse().unwrap();
+    base.set_host(None).unwrap();
+    assert_eq!(base.as_str(), "file:///share/foo/bar");
+
+    let mut base: Url = "file://server/share/foo/bar".parse().unwrap();
+    base.set_host(Some("foo")).unwrap();
+    assert_eq!(base.as_str(), "file://foo/share/foo/bar");
 }
 
 #[test]
@@ -723,6 +731,34 @@ fn test_domain_encoding_quirks() {
     }
 }
 
+#[cfg(feature = "expose_internals")]
+#[test]
+fn test_expose_internals() {
+    use url::quirks::internal_components;
+    use url::quirks::InternalComponents;
+
+    let url = Url::parse("https://example.com/path/file.ext?key=val&key2=val2#fragment").unwrap();
+    let InternalComponents {
+        scheme_end,
+        username_end,
+        host_start,
+        host_end,
+        port,
+        path_start,
+        query_start,
+        fragment_start,
+    } = internal_components(&url);
+
+    assert_eq!(scheme_end, 5);
+    assert_eq!(username_end, 8);
+    assert_eq!(host_start, 8);
+    assert_eq!(host_end, 19);
+    assert_eq!(port, None);
+    assert_eq!(path_start, 19);
+    assert_eq!(query_start, Some(33));
+    assert_eq!(fragment_start, Some(51));
+}
+
 #[test]
 fn test_windows_unc_path() {
     if !cfg!(windows) {