openidconnect-rs
openidconnect-rs copied to clipboard
Unintuitive `IssuerUrl` joins.
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"
https://github.com/ramosbugs/openidconnect-rs/blob/02525323bfdcef1c0da17835fa0d9738db354765/src/types/mod.rs#L310
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));
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?