diff --git a/crates/engine/src/automation/doc_urls.rs b/crates/engine/src/automation/doc_urls.rs index 3b8417cd2..8b7d83199 100644 --- a/crates/engine/src/automation/doc_urls.rs +++ b/crates/engine/src/automation/doc_urls.rs @@ -28,6 +28,11 @@ pub fn openable(url: &str) -> bool { if url.is_empty() || url.chars().any(char::is_control) { return false; } + // Never valid in a URL unencoded, and read as syntax by the shells some openers go through + // (webbrowser on WSL hands the address to `cmd.exe /c start`, escaping only `&` and `^`). + if url.chars().any(|c| c.is_whitespace() || matches!(c, '|' | '<' | '>' | '"' | '`' | '\\' | '^')) { + return false; + } let Some((scheme, rest)) = url.split_once(':') else { return false }; // An RFC 3986 scheme, so a stray colon in text cannot look like one. let mut chars = scheme.chars(); @@ -44,5 +49,11 @@ pub fn openable(url: &str) -> bool { // `http` and `https` name a host, so the authority is required: `http:x` is not a web link, // and `http:///p` has no host. let Some(after) = rest.strip_prefix("//") else { return false }; - !after.is_empty() && !after.starts_with('/') + if after.is_empty() || after.starts_with('/') { + return false; + } + // The authority is made of host characters only (an opener that can't parse the address may + // fall back to treating it as a file path). + let authority = after.split(['/', '?', '#']).next().unwrap_or_default(); + authority.chars().all(|c| !c.is_ascii() || c.is_ascii_alphanumeric() || "-._~:@[]%!$&'()*+,;=".contains(c)) } diff --git a/crates/engine/src/doc_urls_tests.rs b/crates/engine/src/doc_urls_tests.rs index d42029a0f..7158eae06 100644 --- a/crates/engine/src/doc_urls_tests.rs +++ b/crates/engine/src/doc_urls_tests.rs @@ -85,3 +85,25 @@ fn a_scheme_is_required_and_control_characters_are_refused() { assert!(!openable(url), "{url:?} must be refused"); } } + +#[test] +fn shell_syntax_and_malformed_authorities_are_refused() { + for url in [ + // Never valid unencoded, and command syntax to the `cmd.exe /c start` an opener may use. + "https://example.com/?a=b|calc", + "https://example.com/", + "https://example.com/\"x\"", + "https://example.com/a b", + "https://example.com/`x`", + "https://example.com/x^y", + "https://example.com\\x", + // An authority an opener can't parse may be reopened as a file path. + "http://exam ple.com/", + "http://a{b}/", + ] { + assert!(!openable(url), "{url:?} must be refused"); + } + for url in ["https://example.com/?a=b&c=d%20e", "https://[::1]:8080/x", "https://bücher.example/", "https://a.b-c_d.example:443/"] { + assert!(openable(url), "{url:?} should open"); + } +}