web browser (Servo)
git clone https://git.lucas.co/cce-browser.git
fix: a download URL passed as argv downloads instead of erroring
`cce-browser https://…/thing.tar.gz` rendered Servo's "Unknown content
type (application/octet-stream)" page and left nothing on disk.
The sniff that diverts downloadable URLs lives in the delegate's
`request_navigation`, which fires only for navigations the *content*
starts — a link, a `location.href`. A URL the embedder supplies never
reaches it. The first tab's URL goes straight into `WebViewBuilder::url`,
so Servo fetched the archive as a document and gave up on the content
type; the same blind spot covers the URL bar, whose `load` is equally
embedder-initiated.
So the check also runs in `ServoHost::take_as_download`, at the two points
where a URL enters from outside the page. Callers return without
navigating when it takes one, which is what keeps the two paths from
starting the same transfer twice.
Startup deliberately does not raise `download_started`: it opens its first
tab on cce://downloads already, and the flag would add a second one, since
`open_internal_page` dedupes on a tab URL the delegate has not reported
yet. That is what the first cut did — two identical Downloads tabs.
Shadow-verified end to end against a throttled local server: argv'd a
40 MB .bin, got one Downloads tab showing 9.4 MB / 40.0 MB (23%) climbing,
and the full file on disk. The URL-bar path shares the function and the
unit tests but was not driven by hand.
Co-Authored-By: Claude Opus 5 <[email protected]>
CLAUDE.md | 19 ++++++++++++++-----
src/downloads.rs | 34 ++++++++++++++++++++++++++++++++++
src/webview.rs | 44 +++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 91 insertions(+), 6 deletions(-)
diff --git a/CLAUDE.md b/CLAUDE.md
index 2e59746..746585e 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -176,11 +176,20 @@ user out of everything.
## Downloads
-Servo has no download pipeline at all. `request_navigation` sniffs the URL against
-`DOWNLOAD_EXTENSIONS`, `deny()`s the navigation, and hands the URL to a `reqwest`
-blocking worker that streams into the download dir. Note the sniff is **extension-only**
-— there is no `Content-Disposition` or content-type handling, so a download URL with no
-recognizable extension navigates instead.
+Servo has no download pipeline at all, so a URL that looks downloadable is diverted to
+a `reqwest` blocking worker that streams it into the download dir. The sniff is
+**extension-only** (`DOWNLOAD_EXTENSIONS`) — no `Content-Disposition` or content-type
+handling — so a download URL with no recognizable extension navigates instead.
+
+It has to happen in **two places**, and that is not redundancy.
+`WebViewDelegate::request_navigation` fires only for navigations the *content* starts
+(a link, `location.href`). A URL the **embedder** supplies never reaches it — neither
+the first tab's, which Servo loads straight from `WebViewBuilder::url`, nor one from
+the URL bar — so those are sniffed in `ServoHost::take_as_download` instead. Until that
+existed, `cce-browser https://…/thing.tar.gz` rendered Servo's "Unknown content type
+(application/octet-stream)" page rather than downloading. A caller that takes a URL as
+a download must return *without* navigating, which is what stops the two paths from
+starting the same transfer twice.
`Download::id` exists because `clear_finished` shifts Vec positions; worker updates
must never carry an index across a lock boundary.
diff --git a/src/downloads.rs b/src/downloads.rs
index 9bf942a..4ca4fb2 100644
--- a/src/downloads.rs
+++ b/src/downloads.rs
@@ -264,3 +264,37 @@ impl Downloads {
page("Downloads", &meta, &body, head)
}
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+
+ /// The argv case: a release-artifact URL must be recognized before it is
+ /// ever handed to Servo. `request_navigation` does not fire for a URL the
+ /// embedder supplies, so `ServoHost::take_as_download` is the only thing
+ /// standing between this and Servo's "Unknown content type" page.
+ #[test]
+ fn download_urls_are_recognized_by_extension() {
+ for u in [
+ "https://example.com/rel/app-1.2.3.tar.gz",
+ "http://127.0.0.1:8740/big.bin",
+ "https://example.com/Installer.EXE",
+ "https://example.com/x.zip?token=abc",
+ ] {
+ assert!(is_download_url(&Url::parse(u).unwrap()), "should download: {u}");
+ }
+ }
+
+ #[test]
+ fn ordinary_pages_and_non_http_schemes_are_not_downloads() {
+ for u in [
+ "https://www.cloudflare.com/",
+ "https://example.com/page.html",
+ "https://example.com/binary", // no extension: navigates
+ "cce://downloads",
+ "file:///home/me/x.zip", // only http(s) is fetched here
+ ] {
+ assert!(!is_download_url(&Url::parse(u).unwrap()), "should not download: {u}");
+ }
+ }
+}
diff --git a/src/webview.rs b/src/webview.rs
index 3d342d5..e258d9b 100644
--- a/src/webview.rs
+++ b/src/webview.rs
@@ -244,6 +244,10 @@ pub struct ServoHost {
reload_at: Vec<std::time::Instant>,
/// Raised by the cce://cookies/clear page; acted on here in `pump`.
clear_cookies: std::sync::Arc<std::sync::atomic::AtomicBool>,
+ /// Shared with the delegate and the `cce:` handler. The host needs it
+ /// directly because the delegate's sniff cannot see every navigation —
+ /// see [`ServoHost::take_as_download`].
+ downloads: std::sync::Arc<Downloads>,
}
impl ServoHost {
@@ -399,11 +403,45 @@ impl ServoHost {
force_dark,
reload_at: Vec::new(),
clear_cookies,
+ downloads,
+ };
+ // argv can name a download, and the first tab's URL is one of the
+ // navigations the delegate never sees, so it has to be sniffed here.
+ let start = if host.take_as_download(&url) {
+ Url::parse("cce://downloads").expect("downloads url")
+ } else {
+ url
};
- host.open_tab(url);
+ host.open_tab(start);
host
}
+ /// Take `url` as a download instead of a navigation, if it looks like
+ /// one. Returns whether it was taken.
+ ///
+ /// `WebViewDelegate::request_navigation` — where the sniff normally
+ /// happens — only fires for navigations the *content* starts. A URL the
+ /// embedder supplies never reaches it: not the first tab's (Servo loads
+ /// it straight from `WebViewBuilder::url`), and not one typed in the URL
+ /// bar. Passing an archive URL as argv therefore rendered Servo's
+ /// "Unknown content type (application/octet-stream)" page instead of
+ /// downloading it.
+ ///
+ /// Callers must return without navigating when this returns true, which
+ /// is also what keeps the delegate from starting the same download twice.
+ /// Raising `download_started` (so the app surfaces the downloads page) is
+ /// left to the caller: at startup the first tab opens on that page
+ /// already, and the flag would add a second one — `open_internal_page`
+ /// cannot dedupe against a tab whose URL the delegate has not reported
+ /// yet.
+ fn take_as_download(&self, url: &Url) -> bool {
+ if !is_download_url(url) {
+ return false;
+ }
+ self.downloads.start(url.clone());
+ true
+ }
+
fn build_webview(&self, url: Url) -> WebView {
let webview = WebViewBuilder::new(&self.servo, self.context.clone())
.url(url)
@@ -618,6 +656,10 @@ impl ServoHost {
}
pub fn load(&self, url: Url) {
+ if self.take_as_download(&url) {
+ self.shared.download_started.set(true);
+ return;
+ }
self.active_tab().webview.load(url);
}