Skip to content

Commit 41c6be6

Browse files
committed
add an "append_path" function to url
this function is an alternative to `Url::join` which addresses issues discussed in servo#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 servo#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.
1 parent de947ab commit 41c6be6

File tree

2 files changed

+71
-0
lines changed

2 files changed

+71
-0
lines changed

url/src/lib.rs

+32
Original file line numberDiff line numberDiff line change
@@ -2637,6 +2637,38 @@ impl Url {
26372637
Err(())
26382638
}
26392639

2640+
/// Append path segments to the path of a Url, escaping if necesary.
2641+
/// Fails if the Url is cannot-be-a-base.
2642+
///
2643+
/// This differs from Url::join in that it is insensitive to trailing slashes
2644+
/// in the url and leading slashes in the passed string. See documentation of Url::join for discussion
2645+
/// of this subtlety. Also, this function cannot change any part of the Url other than the path.
2646+
///
2647+
/// Examples:
2648+
///
2649+
/// ```
2650+
/// # use url::Url;
2651+
/// let mut my_url = Url::parse("http://www.example.com/api/v1").unwrap();
2652+
/// my_url.append_path("system/status").unwrap();
2653+
/// assert_eq!(my_url.as_str(), "http://www.example.com/api/v1/system/status");
2654+
/// ```
2655+
#[inline]
2656+
pub fn append_path(&mut self, path: impl AsRef<str>) -> Result<(), ()> {
2657+
// This fails if self is cannot-be-a-base but succeeds otherwise.
2658+
let mut path_segments_mut = self.path_segments_mut()?;
2659+
2660+
// Remove the last segment if it is empty, this makes our code tolerate trailing `/`'s
2661+
path_segments_mut.pop_if_empty();
2662+
2663+
// Remove any leading `/` from the path we are appending, this makes our code tolerate leading `/`'s
2664+
let path = path.as_ref();
2665+
let path = path.strip_prefix('/').unwrap_or(path);
2666+
for segment in path.split('/') {
2667+
path_segments_mut.push(segment);
2668+
}
2669+
Ok(())
2670+
}
2671+
26402672
// Private helper methods:
26412673

26422674
#[inline]

url/tests/unit.rs

+39
Original file line numberDiff line numberDiff line change
@@ -1316,3 +1316,42 @@ fn issue_864() {
13161316
url.set_path("x");
13171317
dbg!(&url);
13181318
}
1319+
1320+
#[test]
1321+
/// append_path is an alternative to Url::join addressing issues described in
1322+
/// https://github.com/servo/rust-url/issues/333
1323+
fn test_append_path() {
1324+
// append_path behaves as expected when path is `/` regardless of trailing & leading slashes
1325+
let mut url = Url::parse("http://test.com").unwrap();
1326+
url.append_path("/a/b/c").unwrap();
1327+
assert_eq!(url.as_str(), "http://test.com/a/b/c");
1328+
1329+
let mut url = Url::parse("http://test.com").unwrap();
1330+
url.append_path("a/b/c").unwrap();
1331+
assert_eq!(url.as_str(), "http://test.com/a/b/c");
1332+
1333+
let mut url = Url::parse("http://test.com/").unwrap();
1334+
url.append_path("/a/b/c").unwrap();
1335+
assert_eq!(url.as_str(), "http://test.com/a/b/c");
1336+
1337+
let mut url = Url::parse("http://test.com/").unwrap();
1338+
url.append_path("a/b/c").unwrap();
1339+
assert_eq!(url.as_str(), "http://test.com/a/b/c");
1340+
1341+
// append_path behaves as expected when path is `/api/v1` regardless of trailing & leading slashes
1342+
let mut url = Url::parse("http://test.com/api/v1").unwrap();
1343+
url.append_path("/a/b/c").unwrap();
1344+
assert_eq!(url.as_str(), "http://test.com/api/v1/a/b/c");
1345+
1346+
let mut url = Url::parse("http://test.com/api/v1").unwrap();
1347+
url.append_path("a/b/c").unwrap();
1348+
assert_eq!(url.as_str(), "http://test.com/api/v1/a/b/c");
1349+
1350+
let mut url = Url::parse("http://test.com/api/v1/").unwrap();
1351+
url.append_path("/a/b/c").unwrap();
1352+
assert_eq!(url.as_str(), "http://test.com/api/v1/a/b/c");
1353+
1354+
let mut url = Url::parse("http://test.com/api/v1/").unwrap();
1355+
url.append_path("a/b/c").unwrap();
1356+
assert_eq!(url.as_str(), "http://test.com/api/v1/a/b/c");
1357+
}

0 commit comments

Comments
 (0)