From f7ab901c79bcf66fab20973b925b5f0c30f97404 Mon Sep 17 00:00:00 2001 From: lazulit3 <128615656+lazulit3@users.noreply.github.com> Date: Thu, 19 Dec 2024 18:19:35 -0800 Subject: [PATCH 1/2] test: check for extra path segments from build_url Adds test cases testing `build_url()` with different combinations of leading and trailing slashes between the `base` and `path`. This is for identifying bugs where an empty path segment results in a double `//` like `https://192.0.2.2/base/path//foobar` --- src/http.rs | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/src/http.rs b/src/http.rs index 1d7c674..2adca18 100644 --- a/src/http.rs +++ b/src/http.rs @@ -71,3 +71,62 @@ pub fn build_url(base: &str, path: &str, query: Option) -> Result() .map_err(|e| ClientError::UrlBuildError { source: e }) } + +#[cfg(test)] +mod test { + use super::build_url; + + #[test] + fn build_url_path_with_trailing_slash() { + let url = build_url("https://192.0.2.2", "foobar/", None).unwrap(); + assert_eq!(url.to_string(), "https://192.0.2.2/foobar/"); + } + + #[test] + fn build_url_base_without_path_and_path_without_slash() { + let url = build_url("https://192.0.2.2", "foobar", None).unwrap(); + assert_eq!(url.to_string(), "https://192.0.2.2/foobar"); + } + + #[test] + fn build_url_base_without_path_and_path_with_slash() { + let url = build_url("https://192.0.2.2", "/foobar", None).unwrap(); + assert_eq!(url.to_string(), "https://192.0.2.2/foobar"); + } + + #[test] + fn build_url_base_with_trailing_slash_and_path_without_slash() { + let url = build_url("https://192.0.2.2/", "foobar", None).unwrap(); + assert_eq!(url.to_string(), "https://192.0.2.2/foobar"); + } + + #[test] + fn build_url_base_with_trailing_slash_and_path_with_slash() { + let url = build_url("https://192.0.2.2/", "/foobar", None).unwrap(); + assert_eq!(url.to_string(), "https://192.0.2.2/foobar"); + } + + #[test] + fn build_url_base_with_path_and_path_without_slash() { + let url = build_url("https://192.0.2.2/base/path", "foobar", None).unwrap(); + assert_eq!(url.to_string(), "https://192.0.2.2/base/path/foobar"); + } + + #[test] + fn build_url_base_with_path_and_path_with_slash() { + let url = build_url("https://192.0.2.2/base/path", "/foobar", None).unwrap(); + assert_eq!(url.to_string(), "https://192.0.2.2/base/path/foobar"); + } + + #[test] + fn build_url_base_with_path_and_trailing_slash_and_path_without_slash() { + let url = build_url("https://192.0.2.2/base/path/", "foobar", None).unwrap(); + assert_eq!(url.to_string(), "https://192.0.2.2/base/path/foobar"); + } + + #[test] + fn build_url_base_with_path_and_trailing_slash_and_path_with_slash() { + let url = build_url("https://192.0.2.2/base/path/", "/foobar", None).unwrap(); + assert_eq!(url.to_string(), "https://192.0.2.2/base/path/foobar"); + } +} From 452db5c482649bf23f003f2bc2ed08af809d99ec Mon Sep 17 00:00:00 2001 From: lazulit3 <128615656+lazulit3@users.noreply.github.com> Date: Thu, 19 Dec 2024 18:20:24 -0800 Subject: [PATCH 2/2] fix: empty path segments produced by `build_url()` Fixes approach to joining `base` and `path` in `http::build_url()` so that trailing or leading slashes (`/`) do not result in empty path segments (`//` in produced `Url`). --- src/http.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/http.rs b/src/http.rs index 2adca18..8eff4ce 100644 --- a/src/http.rs +++ b/src/http.rs @@ -62,7 +62,16 @@ pub fn build_request( #[instrument(skip(query), err)] pub fn build_url(base: &str, path: &str, query: Option) -> Result { let mut url = Url::parse(base).map_err(|e| ClientError::UrlParseError { source: e })?; - url.path_segments_mut().unwrap().extend(path.split('/')); + + // remove leading `/` from path to avoid double `//` when base has a path + let path = path.strip_prefix('/').unwrap_or(path); + + url.path_segments_mut() + .unwrap() + // avoids double `//` when base path has trailing slash + .pop_if_empty() + .extend(path.split('/')); + if let Some(q) = query { url.set_query(Some(q.as_str())); }