fix: name sign-in walls and keep account-bound downloads in the browser

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<String> {
+    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<RemoteInfo> {
         })?
         .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<RemoteInfo> {
         .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<RemoteInfo> {
 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!(

crates/catch/src/main.rs

@@ -1460,7 +1460,9 @@ fn why_one(id: &str) -> Result<String> {
         }
         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")

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-<id>`.
+///
+/// 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,16 +570,37 @@ 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 {
         name
     }
 }
 
+/// 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<String> {
+    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<u8> = 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

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");

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",

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 || "" };