servo / rust-url

URL parser for Rust
https://docs.rs/url/
Apache License 2.0
1.27k stars 317 forks source link

JOIN functionality not working #896

Closed Norlock closed 5 months ago

Norlock commented 5 months ago

Describe the bug The join functionality doesn't work correctly.

fn main() {
    let url = url::Url::parse("http://192.0.0.0:3030/unicore/").unwrap();

    let b = url.join("/auth/token/");

    println!("{:?}", b);
}

It removed the path /unicore/ instead of joining it.

Version: latest (2.5).

valenting commented 5 months ago

The description of https://docs.rs/url/latest/url/struct.Url.html#method.join says:

Parse a string as an URL, with this URL as the base URL.

That means it doesn't actually "join the path" but parses the string with the given URL as a base.

What you want is:

fn main() {
    let url = url::Url::parse("http://192.0.0.0:3030/unicore/").unwrap();

    let b = url.join("auth/token/"); // Note no leading /

    println!("{:?}", b);
}
Norlock commented 5 months ago

Can't you add maybe two functions to prevent developers from shooting themselves in the foot? Its very easy to create bugs especially if you join from functions or from environment variables from users, maybe something like:

pub trait JoinFunctions {
    fn add_path(&mut self, path: &str) -> url::Url;
    fn add_file(&mut self, file: &str) -> url::Url;
}

impl JoinFunctions for url::Url {
    fn add_path(&mut self, path: &str) -> url::Url {
        let mut path = path.to_string();

        if path.starts_with("/") {
            path.remove(0);
        }

        if !path.ends_with("/") {
            path.push('/')
        }

        self.join(&path);
    }

    fn add_file(&mut self, file: &str) -> url::Url {
        let mut path = file.to_string();

        if path.starts_with("/") {
            path.remove(0);
        }

        self.join(&path);
    }
}
valenting commented 5 months ago

If you need full control of the path changing I suggest using url.path_segments_mut() instead. https://docs.rs/url/latest/url/struct.PathSegmentsMut.html

valenting commented 5 months ago

I think in the next major revision of the URL crate we might remove join entirely, and add a new Url::parse_with_base(input, base) that works exactly like its DOM equivalent.