From 6431ca96c1fea4edd3c42cce9bf272b3f94ffa8d Mon Sep 17 00:00:00 2001 From: Chris Beck Date: Sun, 2 Jun 2024 10:46:32 -0600 Subject: [PATCH 1/5] add an "append_path" function to url this function is an alternative to `Url::join` which addresses issues discussed in #333, mainly that `Url::join` is sensitive to trailing slashes is in the `Url`, and if the trailing slash is missing, may remove segments from the base url and replace them with segments from the joined `Url`. There are good reasons for `Url::join` to behave that way, because that is was is specified in the `Url` standard. However it's still inconvenient because it often leads to situations where, a service takes some base-url for some API as a config parameter, uses `Url::join` to append various routes to it and make requests, and if a trailing `/` is omitted in a config file, you don't figure it out until deploying and looking at logs and seeing nonsense requests failing. In many situations in web development these trailing `/` are not significant so this is easy to forget and can become just an annoying papercut. One suggestion in #333 was to add an alternative utility function that isn't sensitive to the trailing `/`'s in this way. This commit adds such a utility function with tests. --- url/src/lib.rs | 32 ++++++++++++++++++++++++++++++++ url/tests/unit.rs | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/url/src/lib.rs b/url/src/lib.rs index fa2803681..2afb7afc2 100644 --- a/url/src/lib.rs +++ b/url/src/lib.rs @@ -2756,6 +2756,38 @@ impl Url { Err(()) } + /// Append path segments to the path of a Url, escaping if necesary. + /// Fails if the Url is cannot-be-a-base. + /// + /// This differs from Url::join in that it is insensitive to trailing slashes + /// in the url and leading slashes in the passed string. See documentation of Url::join for discussion + /// of this subtlety. Also, this function cannot change any part of the Url other than the path. + /// + /// Examples: + /// + /// ``` + /// # use url::Url; + /// let mut my_url = Url::parse("http://www.example.com/api/v1").unwrap(); + /// my_url.append_path("system/status").unwrap(); + /// assert_eq!(my_url.as_str(), "http://www.example.com/api/v1/system/status"); + /// ``` + #[inline] + pub fn append_path(&mut self, path: impl AsRef) -> Result<(), ()> { + // This fails if self is cannot-be-a-base but succeeds otherwise. + let mut path_segments_mut = self.path_segments_mut()?; + + // Remove the last segment if it is empty, this makes our code tolerate trailing `/`'s + path_segments_mut.pop_if_empty(); + + // Remove any leading `/` from the path we are appending, this makes our code tolerate leading `/`'s + let path = path.as_ref(); + let path = path.strip_prefix('/').unwrap_or(path); + for segment in path.split('/') { + path_segments_mut.push(segment); + } + Ok(()) + } + // Private helper methods: #[inline] diff --git a/url/tests/unit.rs b/url/tests/unit.rs index 828f79756..1539ab9bc 100644 --- a/url/tests/unit.rs +++ b/url/tests/unit.rs @@ -1392,3 +1392,42 @@ fn test_parse_url_with_single_byte_control_host() { let url2 = Url::parse(url1.as_str()).unwrap(); assert_eq!(url2, url1); } + +#[test] +/// append_path is an alternative to Url::join addressing issues described in +/// https://github.com/servo/rust-url/issues/333 +fn test_append_path() { + // append_path behaves as expected when path is `/` regardless of trailing & leading slashes + let mut url = Url::parse("http://test.com").unwrap(); + url.append_path("/a/b/c").unwrap(); + assert_eq!(url.as_str(), "http://test.com/a/b/c"); + + let mut url = Url::parse("http://test.com").unwrap(); + url.append_path("a/b/c").unwrap(); + assert_eq!(url.as_str(), "http://test.com/a/b/c"); + + let mut url = Url::parse("http://test.com/").unwrap(); + url.append_path("/a/b/c").unwrap(); + assert_eq!(url.as_str(), "http://test.com/a/b/c"); + + let mut url = Url::parse("http://test.com/").unwrap(); + url.append_path("a/b/c").unwrap(); + assert_eq!(url.as_str(), "http://test.com/a/b/c"); + + // append_path behaves as expected when path is `/api/v1` regardless of trailing & leading slashes + let mut url = Url::parse("http://test.com/api/v1").unwrap(); + url.append_path("/a/b/c").unwrap(); + assert_eq!(url.as_str(), "http://test.com/api/v1/a/b/c"); + + let mut url = Url::parse("http://test.com/api/v1").unwrap(); + url.append_path("a/b/c").unwrap(); + assert_eq!(url.as_str(), "http://test.com/api/v1/a/b/c"); + + let mut url = Url::parse("http://test.com/api/v1/").unwrap(); + url.append_path("/a/b/c").unwrap(); + assert_eq!(url.as_str(), "http://test.com/api/v1/a/b/c"); + + let mut url = Url::parse("http://test.com/api/v1/").unwrap(); + url.append_path("a/b/c").unwrap(); + assert_eq!(url.as_str(), "http://test.com/api/v1/a/b/c"); +} From 4d258c4b2c9c0fdae15d9ed53cde5c5a29963c2e Mon Sep 17 00:00:00 2001 From: Chris Beck Date: Mon, 3 Jun 2024 10:52:01 -0600 Subject: [PATCH 2/5] fix clippy --- url/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/url/src/lib.rs b/url/src/lib.rs index 2afb7afc2..f677ca8f9 100644 --- a/url/src/lib.rs +++ b/url/src/lib.rs @@ -2757,7 +2757,6 @@ impl Url { } /// Append path segments to the path of a Url, escaping if necesary. - /// Fails if the Url is cannot-be-a-base. /// /// This differs from Url::join in that it is insensitive to trailing slashes /// in the url and leading slashes in the passed string. See documentation of Url::join for discussion @@ -2771,6 +2770,9 @@ impl Url { /// my_url.append_path("system/status").unwrap(); /// assert_eq!(my_url.as_str(), "http://www.example.com/api/v1/system/status"); /// ``` + /// + /// Fails if the Url is cannot-be-a-base. + #[allow(clippy::result_unit_err)] #[inline] pub fn append_path(&mut self, path: impl AsRef) -> Result<(), ()> { // This fails if self is cannot-be-a-base but succeeds otherwise. From 9872852e6a5f06404e4dff131f76cf9aed44e5b7 Mon Sep 17 00:00:00 2001 From: Alexander van Saase Date: Wed, 22 Apr 2026 13:58:08 +0200 Subject: [PATCH 3/5] Fix typo and error section in doc comment --- url/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/url/src/lib.rs b/url/src/lib.rs index f677ca8f9..0f3bc64f5 100644 --- a/url/src/lib.rs +++ b/url/src/lib.rs @@ -2756,7 +2756,7 @@ impl Url { Err(()) } - /// Append path segments to the path of a Url, escaping if necesary. + /// Append path segments to the path of a Url, escaping if necessary. /// /// This differs from Url::join in that it is insensitive to trailing slashes /// in the url and leading slashes in the passed string. See documentation of Url::join for discussion @@ -2771,6 +2771,8 @@ impl Url { /// assert_eq!(my_url.as_str(), "http://www.example.com/api/v1/system/status"); /// ``` /// + /// # Errors + /// /// Fails if the Url is cannot-be-a-base. #[allow(clippy::result_unit_err)] #[inline] From 3c417691686f9026bbebfdb6041a9e433aaa14eb Mon Sep 17 00:00:00 2001 From: Alexander van Saase Date: Wed, 22 Apr 2026 14:54:56 +0200 Subject: [PATCH 4/5] Use links in doc comments --- url/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/url/src/lib.rs b/url/src/lib.rs index 0f3bc64f5..ce8d07401 100644 --- a/url/src/lib.rs +++ b/url/src/lib.rs @@ -2758,8 +2758,8 @@ impl Url { /// Append path segments to the path of a Url, escaping if necessary. /// - /// This differs from Url::join in that it is insensitive to trailing slashes - /// in the url and leading slashes in the passed string. See documentation of Url::join for discussion + /// This differs from [`Url::join`] in that it is insensitive to trailing slashes + /// in the url and leading slashes in the passed string. See documentation of [`Url::join`] for discussion /// of this subtlety. Also, this function cannot change any part of the Url other than the path. /// /// Examples: From a40daf89dbc9438ba1fd2770795c8e0fbfab5b3c Mon Sep 17 00:00:00 2001 From: Alexander van Saase Date: Wed, 22 Apr 2026 14:47:33 +0200 Subject: [PATCH 5/5] Rename to append_path_mut and add append_path that returns owned Url --- url/src/lib.rs | 34 +++++++++++++++++++++++++++-- url/tests/unit.rs | 54 +++++++++++++++++++++++++++++++++-------------- 2 files changed, 70 insertions(+), 18 deletions(-) diff --git a/url/src/lib.rs b/url/src/lib.rs index ce8d07401..8b2b4804a 100644 --- a/url/src/lib.rs +++ b/url/src/lib.rs @@ -2756,18 +2756,48 @@ impl Url { Err(()) } + /// Append path segments to the path of a Url, escaping if necessary. + /// + /// This differs from [`Url::join`] in that it is insensitive to trailing slashes + /// in the url and leading slashes in the passed string. See documentation of [`Url::join`] for discussion + /// of this subtlety. Also, this function cannot change any part of the Url other than the path. Note that + /// this clones the URL, so if you want to mutate the URL in place, use [`Url::append_path_mut`] instead. + /// + /// Examples: + /// + /// ``` + /// # use url::Url; + /// let mut my_url = Url::parse("http://www.example.com/api/v1").unwrap(); + /// my_url.append_path_mut("system/status").unwrap(); + /// assert_eq!(my_url.as_str(), "http://www.example.com/api/v1/system/status"); + /// ``` + /// + /// # Errors + /// + /// Fails if the Url is cannot-be-a-base. + #[allow(clippy::result_unit_err)] + #[inline] + pub fn append_path(&self, path: impl AsRef) -> Result { + let mut url = self.clone(); + url.append_path_mut(path)?; + Ok(url) + } + /// Append path segments to the path of a Url, escaping if necessary. /// /// This differs from [`Url::join`] in that it is insensitive to trailing slashes /// in the url and leading slashes in the passed string. See documentation of [`Url::join`] for discussion /// of this subtlety. Also, this function cannot change any part of the Url other than the path. /// + /// This mutates the URL in place. For a non-mutating variant that returns + /// a new `Url`, use [`Url::append_path`]. + /// /// Examples: /// /// ``` /// # use url::Url; /// let mut my_url = Url::parse("http://www.example.com/api/v1").unwrap(); - /// my_url.append_path("system/status").unwrap(); + /// my_url.append_path_mut("system/status").unwrap(); /// assert_eq!(my_url.as_str(), "http://www.example.com/api/v1/system/status"); /// ``` /// @@ -2776,7 +2806,7 @@ impl Url { /// Fails if the Url is cannot-be-a-base. #[allow(clippy::result_unit_err)] #[inline] - pub fn append_path(&mut self, path: impl AsRef) -> Result<(), ()> { + pub fn append_path_mut(&mut self, path: impl AsRef) -> Result<(), ()> { // This fails if self is cannot-be-a-base but succeeds otherwise. let mut path_segments_mut = self.path_segments_mut()?; diff --git a/url/tests/unit.rs b/url/tests/unit.rs index 1539ab9bc..acc27fba9 100644 --- a/url/tests/unit.rs +++ b/url/tests/unit.rs @@ -541,6 +541,28 @@ fn append_empty_segment_then_mutate() { assert_eq!(url.to_string(), "http://localhost:6767/foo/bar?a=b"); } +#[test] +fn append_path_returns_new_url() { + let url = Url::parse("http://www.example.com/api/v1/").unwrap(); + + let appended = url.append_path("/system/status").unwrap(); + + assert_eq!(url.as_str(), "http://www.example.com/api/v1/"); + assert_eq!( + appended.as_str(), + "http://www.example.com/api/v1/system/status" + ); +} + +#[test] +fn append_path_and_append_path_mut_fail_for_cannot_be_a_base() { + let url = Url::parse("mailto:test@example.net").unwrap(); + assert!(url.append_path("x").is_err()); + + let mut url = Url::parse("mailto:test@example.net").unwrap(); + assert!(url.append_path_mut("x").is_err()); +} + #[test] /// https://github.com/servo/rust-url/issues/243 fn test_set_host() { @@ -1398,36 +1420,36 @@ fn test_parse_url_with_single_byte_control_host() { /// https://github.com/servo/rust-url/issues/333 fn test_append_path() { // append_path behaves as expected when path is `/` regardless of trailing & leading slashes - let mut url = Url::parse("http://test.com").unwrap(); - url.append_path("/a/b/c").unwrap(); + let url = Url::parse("http://test.com").unwrap(); + let url = url.append_path("/a/b/c").unwrap(); assert_eq!(url.as_str(), "http://test.com/a/b/c"); - let mut url = Url::parse("http://test.com").unwrap(); - url.append_path("a/b/c").unwrap(); + let url = Url::parse("http://test.com").unwrap(); + let url = url.append_path("a/b/c").unwrap(); assert_eq!(url.as_str(), "http://test.com/a/b/c"); - let mut url = Url::parse("http://test.com/").unwrap(); - url.append_path("/a/b/c").unwrap(); + let url = Url::parse("http://test.com/").unwrap(); + let url = url.append_path("/a/b/c").unwrap(); assert_eq!(url.as_str(), "http://test.com/a/b/c"); - let mut url = Url::parse("http://test.com/").unwrap(); - url.append_path("a/b/c").unwrap(); + let url = Url::parse("http://test.com/").unwrap(); + let url = url.append_path("a/b/c").unwrap(); assert_eq!(url.as_str(), "http://test.com/a/b/c"); // append_path behaves as expected when path is `/api/v1` regardless of trailing & leading slashes - let mut url = Url::parse("http://test.com/api/v1").unwrap(); - url.append_path("/a/b/c").unwrap(); + let url = Url::parse("http://test.com/api/v1").unwrap(); + let url = url.append_path("/a/b/c").unwrap(); assert_eq!(url.as_str(), "http://test.com/api/v1/a/b/c"); - let mut url = Url::parse("http://test.com/api/v1").unwrap(); - url.append_path("a/b/c").unwrap(); + let url = Url::parse("http://test.com/api/v1").unwrap(); + let url = url.append_path("a/b/c").unwrap(); assert_eq!(url.as_str(), "http://test.com/api/v1/a/b/c"); - let mut url = Url::parse("http://test.com/api/v1/").unwrap(); - url.append_path("/a/b/c").unwrap(); + let url = Url::parse("http://test.com/api/v1/").unwrap(); + let url = url.append_path("/a/b/c").unwrap(); assert_eq!(url.as_str(), "http://test.com/api/v1/a/b/c"); - let mut url = Url::parse("http://test.com/api/v1/").unwrap(); - url.append_path("a/b/c").unwrap(); + let url = Url::parse("http://test.com/api/v1/").unwrap(); + let url = url.append_path("a/b/c").unwrap(); assert_eq!(url.as_str(), "http://test.com/api/v1/a/b/c"); }