Skip to content

Link handling: external image fetch follows redirects (even https to http) and the opener gets the raw string - #233

Merged
Sythos merged 1 commit into
mainfrom
enhancement-issue-207
Oct 3, 2026
Merged

Sythos merged 1 commit into
mainfrom
enhancement-issue-207

Conversation

@Sythos

@Sythos Sythos commented Oct 3, 2026

Copy link
Copy Markdown
Owner

Closes #207.

Two gaps in link handling:

  • The opener got the raw string. open_in_browser validated the link with Url::parse but passed the original href to xdg-open, open or rundll32. A new media::validated_url(href) does the validation once and returns the parsed, normalised URL (lowercase scheme and host, punycode for IDN, default port dropped, \ as /), and that string is what the opener gets, from every call site (chat links, the media viewer button, the credits links, the release page, OpenLink). It refuses whitespace or control characters anywhere, more than 2,048 bytes, a parse failure, any scheme other than http, https and ftp (what the old is_openable allowed) and embedded user names or passwords. A refused chat click opens the media viewer in its existing "failed" state with the reason; a refusal at the opener is only logged, without the link.
  • The image fetch followed redirects blindly. fetch_public now installs a custom redirect policy (media::redirect_decision): at most five hops, only http and https (never ftp), no credentials in the next URL, no https to http downgrade even on the same host, and a public host may not redirect to a local one (localhost, *.localhost, or an IP literal that is loopback, private, link-local, CGNAT, unspecified or broadcast, IPv4-mapped IPv6 included; a link that is already local may stay local). The code had no private-host restriction before, so this one applies to redirects only and doesn't resolve DNS: a public name pointing at a private address isn't caught. Errors from the fetch never include the URL.
  • Tests: normalisation (uppercase scheme, default port, \@, %20, IDN), the refusals (whitespace, controls, userinfo, javascript:, file:, data:, mailto:, custom schemes, garbage, the 2,048 boundary), ftp accepted and ftp://user:pw@host refused, every redirect decision (including 127.1, decimal IPs, mapped IPv6 and CGNAT edges) and mock-server tests for a relative redirect, a loop and a redirect to ftp. No new dependency (reqwest::Url is the url crate), no new string. README and the feature matrix describe the rules.
  • Not verified: nothing is compiled before CI. The mock-server test that expects the reason "too many redirects" depends on how reqwest 0.13 exposes the policy error through Error::source() (if it wraps it differently the text would read "error following redirect" and that test would fail); without_url and the Policy::custom signatures also follow my reading of 0.13.5. The https-to-http and public-to-local refusals are covered by the pure tests only, since the mock server is plain http on loopback. On the viewer's failed screen for a refused link the "Open in browser" button does nothing, because there is no URL to open.

Branch: enhancement-issue-207

The system browser got the raw href of a chat link, not the parsed one,
and the external image fetch followed redirects blindly, even from
https to http. Add one validation (http, https and ftp, no whitespace or
control characters, no embedded credentials, at most 2,048 bytes) whose
normalised URL is what the opener gets, with a refused link landing on
the viewer's existing failed screen, and a redirect policy for the image
fetch: at most five hops, only http and https, no credentials, no
https to http downgrade, and no hop from a public host to a local one.
Fixes #207.
@Sythos Sythos added the enhancement New feature or request label Oct 3, 2026
@Sythos Sythos self-assigned this Oct 3, 2026
@Sythos
Sythos merged commit 87031d4 into main Oct 3, 2026
8 checks passed
@Sythos
Sythos deleted the enhancement-issue-207 branch October 3, 2026 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Link handling: external image fetch follows redirects (even https to http) and the opener gets the raw string

1 participant