diff --git a/bin/dist/catch b/bin/dist/catch index 6dfe9cc..c8b0adb 100755 Binary files a/bin/dist/catch and b/bin/dist/catch differ diff --git a/crates/catch/src/download.rs b/crates/catch/src/download.rs index 5f06cb0..0c4cb15 100644 --- a/crates/catch/src/download.rs +++ b/crates/catch/src/download.rs @@ -5,8 +5,8 @@ use std::time::{Duration, Instant}; use crate::segment; use crate::state::{ - dir_for, load_state, mark_error, resolve_download_dir, sanitize_filename, unique_destination, - update_state, worker_holds, Status, + dir_for, load_state, mark_error, query_param, resolve_download_dir, sanitize_filename, + unique_destination, update_state, worker_holds, Status, }; /// Give up on a connection with no body data this long, then reconnect. @@ -102,20 +102,62 @@ pub(crate) enum Verdict { Fatal(String), } -/// True when the URL path ends with a file extension that is not itself -/// a page (used to refuse HTML answers to file links). +/// The file extension of a name, when it is a file rather than a page. +fn file_extension(name: &str) -> Option { + let i = name.rfind('.')?; + let ext = name[i + 1..].to_lowercase(); + if ext.is_empty() + || ext.len() > 5 + || ["html", "htm", "xhtml", "php", "asp", "aspx", "jsp"].contains(&ext.as_str()) + { + None + } else { + Some(ext) + } +} + +/// True when the URL names a file, either in its path or in a `filename=` +/// query parameter. Download endpoints often hide the name in the query +/// (`/download?filename=report.docx`); treating those as "not a file" let a +/// sign-in page's 400 through as a generic status. fn looks_like_file_url(url: &str) -> bool { let path = url.split(['?', '#']).next().unwrap_or(""); let tail = path.rsplit('/').next().unwrap_or(""); - match tail.rfind('.') { - Some(i) => { - let ext = tail[i + 1..].to_lowercase(); - !(ext.is_empty() - || ext.len() > 5 - || ["html", "htm", "xhtml", "php", "asp", "aspx", "jsp"].contains(&ext.as_str())) - } - None => false, + if file_extension(tail).is_some() { + return true; } + query_param(url, "filename") + .map(|name| file_extension(&name).is_some()) + .unwrap_or(false) +} + +/// A redirect that ends at a sign-in host means the link is tied to the +/// browser's account session. Catch has no cookies, so no retry can work. +fn is_signin_url(url: &reqwest::Url) -> bool { + let host = url.host_str().unwrap_or("").to_ascii_lowercase(); + host == "accounts.google.com" || host.ends_with(".accounts.google.com") +} + +const SIGNIN_WALL: &str = "sign-in required — this link needs the browser's account session, which Catch cannot use; download it with the browser"; +const PAGE_NOT_FILE: &str = "server returned a page, not the file (login wall or expired link? refresh the address with `catch refresh`)"; + +/// What an HTTP answer says when it is not the file we asked for. A +/// sign-in redirect is named as such instead of being reported as a bare +/// `400 Bad Request`, and a `filename=` query is enough to know an HTML +/// answer is a page rather than the download. +fn response_wall( + status: reqwest::StatusCode, + content_type: &str, + final_url: &reqwest::Url, + original_url: &str, +) -> Option<&'static str> { + if !status.is_success() && is_signin_url(final_url) { + return Some(SIGNIN_WALL); + } + if content_type.starts_with("text/html") && looks_like_file_url(original_url) { + return Some(PAGE_NOT_FILE); + } + None } /// "bytes 200-1000/67589" -> (200, 1000, Some(67589)). A malformed or @@ -717,14 +759,17 @@ pub async fn download_entry( // 200-on-resume path restarts from zero, and the partial bytes must // survive until we know the server is not about to hand us a login // page instead of the file. Either way, nothing is deleted here. + // + // A redirect to a sign-in host is the same wall even when the URL + // does not end in a file extension; naming it as a sign-in wall keeps + // a bare `400 Bad Request` from looking like a retryable transfer. let content_type = resp .headers() .get(reqwest::header::CONTENT_TYPE) .and_then(|v| v.to_str().ok()) .unwrap_or("") .to_string(); - if content_type.starts_with("text/html") && looks_like_file_url(&url) { - let msg = "server returned a page, not the file (login wall or expired link? refresh the address with `catch refresh`)"; + if let Some(msg) = response_wall(status, &content_type, resp.url(), &url) { let _ = mark_error(id, msg); anyhow::bail!("{url}: {msg}"); } @@ -1856,6 +1901,14 @@ pub(crate) async fn probe_remote(url: &str) -> Result { })? .with_context(|| format!("GET {url}"))?; let status = resp.status(); + let content_type = resp + .headers() + .get(reqwest::header::CONTENT_TYPE) + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + if let Some(msg) = response_wall(status, content_type, resp.url(), url) { + anyhow::bail!("{url}: {msg}"); + } if !status.is_success() && status != reqwest::StatusCode::RANGE_NOT_SATISFIABLE { let msg = match status.as_u16() { 401 | 403 => format!("{status} — the link is expired or requires sign-in"), @@ -1890,14 +1943,6 @@ pub(crate) async fn probe_remote(url: &str) -> Result { .and_then(|v| v.to_str().ok()) .unwrap_or("") .to_string(); - let content_type = resp - .headers() - .get(reqwest::header::CONTENT_TYPE) - .and_then(|v| v.to_str().ok()) - .unwrap_or(""); - if content_type.starts_with("text/html") && looks_like_file_url(url) { - anyhow::bail!("{url}: that address serves a web page, not a file (login wall?)"); - } Ok(RemoteInfo { total, etag, @@ -1909,6 +1954,50 @@ pub(crate) async fn probe_remote(url: &str) -> Result { mod range_tests { use super::*; + #[test] + fn filename_query_counts_as_a_file_url() { + // The path is `/download`; the name is only in the query. A 400 + // HTML answer to this must be treated as a page, not a bare status. + assert!(looks_like_file_url( + "https://contribution.usercontent.google.com/download?filename=report.docx" + )); + assert!(looks_like_file_url( + "https://example.com/download?filename=archive.tar.gz" + )); + // A page in the query stays a page. + assert!(!looks_like_file_url( + "https://example.com/download?filename=page.html" + )); + assert!(!looks_like_file_url("https://example.com/app")); + assert!(looks_like_file_url("https://example.com/file.bin")); + } + + #[test] + fn signin_redirect_is_named_not_reported_as_400() { + let final_url = + reqwest::Url::parse("https://accounts.google.com/ServiceLogin?continue=x").unwrap(); + let msg = response_wall( + reqwest::StatusCode::BAD_REQUEST, + "text/html; charset=utf-8", + &final_url, + "https://contribution.usercontent.google.com/download?filename=report.docx", + ); + assert_eq!(msg, Some(SIGNIN_WALL)); + assert!(SIGNIN_WALL.contains("browser")); + + // A normal host with an HTML body on a file link is the page wall. + let local = reqwest::Url::parse("http://127.0.0.1:1/login").unwrap(); + assert_eq!( + response_wall( + reqwest::StatusCode::BAD_REQUEST, + "text/html", + &local, + "http://127.0.0.1:1/download?filename=report.docx", + ), + Some(PAGE_NOT_FILE) + ); + } + #[test] fn content_range_parses_well_formed_headers() { assert_eq!( diff --git a/crates/catch/src/main.rs b/crates/catch/src/main.rs index 8dd0564..99b735d 100644 --- a/crates/catch/src/main.rs +++ b/crates/catch/src/main.rs @@ -1460,7 +1460,9 @@ fn why_one(id: &str) -> Result { } Status::Error => { let e = d.error.to_lowercase(); - if e.contains("page, not the file") + if e.contains("sign-in") || e.contains("browser session") { + "the link needs the browser's account session — download it with the browser; Catch cannot use those cookies".to_string() + } else if e.contains("page, not the file") || e.contains("expired") || e.contains(" 401") || e.contains(" 403") diff --git a/crates/catch/src/state.rs b/crates/catch/src/state.rs index 5f7a3c8..279807d 100644 --- a/crates/catch/src/state.rs +++ b/crates/catch/src/state.rs @@ -556,6 +556,11 @@ pub fn sanitize_filename(raw: &str) -> String { } /// Derive a safe filename from a URL. Falls back to `download-`. +/// +/// Download endpoints often keep the real name in a query parameter while +/// the path is just `/download` (Gemini and Google Drive do this). Prefer +/// that name when the path itself has no extension, so the row does not +/// read `download` and the saved file keeps its `.docx`/`.zip` suffix. pub fn filename_for_url(url: &str, id: &str) -> String { let tail = url .split(['?', '#']) @@ -565,9 +570,17 @@ pub fn filename_for_url(url: &str, id: &str) -> String { .next() .unwrap_or("") .trim(); - // Percent-decode minimally: '+' and %XX for the common cases. + // Percent-decode minimally: %XX for the common cases. let decoded = percent_decode(tail); - let name = sanitize_filename(&decoded); + let mut name = sanitize_filename(&decoded); + if name.is_empty() || !name.contains('.') { + if let Some(hinted) = query_param(url, "filename") { + let hinted = sanitize_filename(&hinted); + if !hinted.is_empty() { + name = hinted; + } + } + } if name.is_empty() { format!("download-{}", id) } else { @@ -575,6 +588,19 @@ pub fn filename_for_url(url: &str, id: &str) -> String { } } +/// The first value of `name` in the URL's query string, percent-decoded. +/// `+` is a space in query values; path tails keep their literal `+`. +pub(crate) fn query_param(url: &str, name: &str) -> Option { + let query = url.split_once('?')?.1.split('#').next()?; + query.split('&').find_map(|pair| { + let (key, value) = pair.split_once('=').unwrap_or((pair, "")); + if key != name { + return None; + } + Some(percent_decode(&value.replace('+', " "))) + }) +} + fn percent_decode(s: &str) -> String { let bytes = s.as_bytes(); let mut out: Vec = Vec::with_capacity(bytes.len()); @@ -1675,6 +1701,31 @@ mod tests { } } + #[test] + fn filename_for_url_prefers_a_query_filename() { + let id = "id"; + // Gemini/Drive hide the real name in the query; the path is + // `/download`, so the row used to read just `download`. + assert_eq!( + filename_for_url( + "https://contribution.usercontent.google.com/download?filename=%D8%B1%D8%B2%D9%88%D9%85%D9%87-v2.docx", + id + ), + "رزومه-v2.docx" + ); + assert_eq!( + filename_for_url("https://example.com/download?filename=report+final.pdf", id), + "report final.pdf" + ); + // A real path name wins over the query hint. + assert_eq!( + filename_for_url("https://example.com/file.bin?filename=other.zip", id), + "file.bin" + ); + // No hint, no path name: the historical fallback. + assert_eq!(filename_for_url("https://example.com/", id), "download-id"); + } + /// A queued row carries no pid and has no worker behind it yet, so /// reaping must leave it exactly as it is. `enqueue` builds rows this /// way on purpose: a row that claimed itself would look like an active diff --git a/crates/catch/tests/reliability.rs b/crates/catch/tests/reliability.rs index a3426cc..62dafa0 100644 --- a/crates/catch/tests/reliability.rs +++ b/crates/catch/tests/reliability.rs @@ -75,6 +75,46 @@ fn header_connection_failure_then_retry_succeeds() { assert_eq!(server.hits(), 2, "one failed connection, one success"); } +#[test] +fn login_wall_on_a_query_named_download_is_named_not_400() { + let sb = Sandbox::new("login-wall"); + let server = Server::start(|req, _n| { + if req.target.starts_with("/download") { + Reply::new(302).header("Location", "/login") + } else { + Reply::new(400).header("Content-Type", "text/html; charset=utf-8") + } + }); + let url = server.url("/download?filename=test.docx"); + let out = add(&sb, &url); + assert!( + !out.status.success(), + "a login wall must fail, not save a page" + ); + + let row = sb.row_for_url(&url).expect("row"); + assert_eq!(row["status"], "error"); + assert_eq!( + row["filename"], "test.docx", + "the filename= query must name the row, not the /download path" + ); + let error = row["error"].as_str().unwrap_or(""); + assert!( + error.contains("page, not the file"), + "the 400 must be explained as a page/login wall, got {error:?}" + ); + + // `why` must not tell the user to resume a transfer that can never + // authenticate; it should point at the login wall instead. + let id = row["id"].as_str().unwrap(); + let why = run_args(&sb, &["why", id], Duration::from_secs(10)); + let text = String::from_utf8_lossy(&why.stdout); + assert!( + text.contains("login wall") || text.contains("browser session"), + "why did not explain the wall: {text}" + ); +} + #[test] fn stalled_body_times_out_and_resumes_with_range() { let sb = Sandbox::new("stall"); diff --git a/crates/catch/tests/support/mod.rs b/crates/catch/tests/support/mod.rs index ee767c5..447f0f9 100644 --- a/crates/catch/tests/support/mod.rs +++ b/crates/catch/tests/support/mod.rs @@ -482,6 +482,7 @@ fn reason(status: u16) -> &'static str { match status { 200 => "OK", 206 => "Partial Content", + 302 => "Found", 400 => "Bad Request", 404 => "Not Found", 416 => "Range Not Satisfiable", diff --git a/extensions/chromium-catch/background-3.js b/extensions/chromium-catch/background-3.js index 97b8cde..5fe3f41 100644 --- a/extensions/chromium-catch/background-3.js +++ b/extensions/chromium-catch/background-3.js @@ -292,6 +292,27 @@ function pageTitle(tab) { // Catch; if the native host is missing or refuses, leave the browser // download running so nothing is ever swallowed. // +// Account-bound Google downloads. The browser's request carries the session +// cookies that Catch (a native process) does not have, so taking one of these +// over cancels the only request that can succeed and leaves Catch with a +// sign-in redirect. Let the browser keep them. +const AUTH_GATED_HOSTS = [ + "contribution.usercontent.google.com", + "drive.usercontent.google.com", + "docs.google.com", + "drive.google.com", + "mail.google.com", +]; + +function authGated(url) { + try { + const host = new URL(url).hostname.toLowerCase(); + return AUTH_GATED_HOSTS.some((h) => host === h || host.endsWith("." + h)); + } catch (e) { + return false; + } +} + // Canceling a download a click started makes Chromium re-navigate to the // URL and fetch the file again, which is why a click can look like it is // "not working" and then take a while. Catch still has the row and the file @@ -301,6 +322,7 @@ function pageTitle(tab) { function intercept(item) { const url = httpUrl(item && item.url); if (!url) return; // blob:, data:, chrome: stay in the browser + if (authGated(url)) return; // the browser has the account session if (item.state && item.state !== "in_progress") return; // too late, browser won if (bypassed(url)) return; const msg = { url, title: "", filenameHint: item.filename || "" }; diff --git a/extensions/firefox-catch/background-3.js b/extensions/firefox-catch/background-3.js index 97b8cde..5fe3f41 100644 --- a/extensions/firefox-catch/background-3.js +++ b/extensions/firefox-catch/background-3.js @@ -292,6 +292,27 @@ function pageTitle(tab) { // Catch; if the native host is missing or refuses, leave the browser // download running so nothing is ever swallowed. // +// Account-bound Google downloads. The browser's request carries the session +// cookies that Catch (a native process) does not have, so taking one of these +// over cancels the only request that can succeed and leaves Catch with a +// sign-in redirect. Let the browser keep them. +const AUTH_GATED_HOSTS = [ + "contribution.usercontent.google.com", + "drive.usercontent.google.com", + "docs.google.com", + "drive.google.com", + "mail.google.com", +]; + +function authGated(url) { + try { + const host = new URL(url).hostname.toLowerCase(); + return AUTH_GATED_HOSTS.some((h) => host === h || host.endsWith("." + h)); + } catch (e) { + return false; + } +} + // Canceling a download a click started makes Chromium re-navigate to the // URL and fetch the file again, which is why a click can look like it is // "not working" and then take a while. Catch still has the row and the file @@ -301,6 +322,7 @@ function pageTitle(tab) { function intercept(item) { const url = httpUrl(item && item.url); if (!url) return; // blob:, data:, chrome: stay in the browser + if (authGated(url)) return; // the browser has the account session if (item.state && item.state !== "in_progress") return; // too late, browser won if (bypassed(url)) return; const msg = { url, title: "", filenameHint: item.filename || "" };