Unintuitive IssuerUrl joins.

[gibbz00]

Might be related to: #277

#[test]
fn temp() {
    let base = "http://localhost:9025";
    let suffix = "/jwk";

    let expected_join = format!("{}{}", base, suffix);

    let url = Url::parse(base).unwrap();
    let actual_url_join = url.join(suffix).unwrap();
    assert_eq!(expected_join, actual_url_join.as_str());

    let issuer_url = IssuerUrl::new(base.to_string()).unwrap();
    let actual_issuer_url_join = issuer_url.join(suffix).unwrap();
    assert_eq!(expected_join, actual_issuer_url_join.as_str());
}

Last assertion fails:

assertion `left == right` failed
 left: "http://localhost:9025/jwk"
 right: "http://localhost:9025//jwk"

openidconnect-rs/src/types/mod.rs

Line 310 in 0252532

Url::parse(&(self.1.clone() + "/" + suffix))

I'm currently able to work around this by calling trim_start_matches() so it not that big of an issue really. Mostly interested if/where I've missed the underlying motivation for this behavior :)

const INVALID_JOIN: &str = "unable to join issuer url server endpoint path";
let auth_url = AuthUrl::from_url(issuer_url.join(Routes::AUTHORIZE.trim_start_matches('/')).expect(INVALID_JOIN));

[ramosbugs]

I can see how this would be confusing, and I agree that the sensible result should be http://localhost:9025/jwk.

As context, I intentionally didn't use Url::join because (iiuc) it always replaces the last component of the issuer URL unless it has a trailing slash. Since an issuer URL should always be a prefix of the joined URL, we always want to append rather than replace.

The simplest fix is probably to check whether the suffix begins with a /, and not to insert another / in the middle if so (similar to the existing check on the issuer URL ending in a /). If the issuer URL ends in a / and the suffix starts with a /, we probably want to skip the duplicate leading slash in the suffix. I don't think any further canonicalization is necessary though (e.g., if there are multiple consecutive slashes in the issuer URL or the suffix).

Thoughts?