fix(renderer): don't feed binary bodies to the HTML extractor

crates/crw-cli/src/commands/scrape.rs

@@ -328,6 +328,18 @@ pub async fn run(mut args: ScrapeArgs) -> Result<(), CmdError> {
                     data = Some(d);
                 }
             }
+            Err(crw_core::error::CrwError::UnsupportedContentType(msg)) => {
+                // Same rule as the server ladder: the body is not a web page at
+                // all, so spawning LightPanda and Chrome to look at it again
+                // only costs the user startup time and prints a "trying JS
+                // renderer" line that never had a chance of working.
+                eprintln!("error: Unsupported content type: {msg}");
+                // `CmdError`, never `process::exit`: `teardown` owns the single
+                // exit path so `kill_all_browsers()` runs on every one of them,
+                // and it keeps that guarantee structurally rather than by
+                // auditing which call sites happen to run before a spawn.
+                return Err(CmdError::code_only(1));
+            }
             Err(e) => {
                 // HTTP-only failure → fall through to JS escalation below.
                 eprintln!("info: HTTP fetch failed ({e}), trying JS renderer...");

crates/crw-core/src/error.rs

@@ -20,6 +20,14 @@ pub enum CrwError {
     #[error("Extraction error: {0}")]
     ExtractionError(String),
 
+    /// The origin returned a body that is neither HTML nor a document crw can
+    /// parse (a ZIP-container office file, an image, an archive). Distinct from
+    /// `HttpError` because it must NOT escalate to the JS renderer ladder: no
+    /// browser turns a .docx into a page, so climbing the ladder only burns a
+    /// Chromium/Camoufox session before failing anyway.
+    #[error("Unsupported content type: {0}")]
+    UnsupportedContentType(String),
+
     #[error("Crawl error: {0}")]
     CrawlError(String),
 
@@ -58,6 +66,7 @@ impl CrwError {
             CrwError::InvalidRequest(_) => "invalid_request",
             CrwError::RendererError(_) => "renderer_error",
             CrwError::ExtractionError(_) => "extraction_error",
+            CrwError::UnsupportedContentType(_) => "unsupported_content_type",
             CrwError::CrawlError(_) => "crawl_error",
             CrwError::Timeout(_) => "timeout",
             CrwError::ConfigError(_) => "config_error",

crates/crw-renderer/src/http_only.rs

@@ -913,7 +913,7 @@ impl PageFetcher for HttpFetcher {
             .get("content-type")
             .and_then(|v| v.to_str().ok())
             .map(|s| s.to_string());
-        let content_type = content_type_header
+        let mut content_type = content_type_header
             .as_deref()
             .map(|s| s.split(';').next().unwrap_or(s).trim().to_lowercase());
         // Charset from the Content-Type header (P1-1): pages served as Latin-1 /
@@ -925,8 +925,6 @@ impl PageFetcher for HttpFetcher {
 
         let challenge = challenge_header(resp.headers());
 
-        let is_pdf = content_type.as_deref() == Some("application/pdf");
-
         let final_url_str = resp.url().as_str().to_string();
 
         // Bound the body read by the caller's remaining budget. Without this the
@@ -970,8 +968,55 @@ impl PageFetcher for HttpFetcher {
             )));
         }
 
+        // Route on the actual bytes, not only on the declared type. Keying the
+        // PDF branch on `content_type == "application/pdf"` and treating
+        // EVERYTHING else as text means a body that is neither HTML nor a
+        // correctly-labelled PDF gets UTF-8-lossy'd and handed to the HTML
+        // extractor: a .docx/.xlsx/.pptx comes back as `markdown` beginning
+        // "PK\u{3}\u{4}...[Content_Types].xml" under `success: true`, which a
+        // caller cannot tell apart from a real scrape.
+        if bytes.starts_with(b"%PDF-") {
+            // A PDF served as octet-stream (or text/html). Relabel it so the
+            // downstream PDF branch in crw-crawl, which gates on the same
+            // content type, engages instead of extracting an empty body.
+            content_type = Some("application/pdf".to_string());
+        }
+        // Computed AFTER the relabel, so a sniffed PDF and a declared one are
+        // one case from here down rather than a disjunction repeated at every use.
+        let is_pdf = content_type.as_deref() == Some("application/pdf");
+        // The NUL test applies only when the origin did NOT declare an HTML-ish
+        // type. A page served as `text/html` with a stray NUL in it renders
+        // fine in a real browser (the HTML5 tokenizer maps NUL to U+FFFD), so
+        // rejecting one would cost a page we scrape today, and would hand any
+        // origin a one-byte way to shut the ladder down that costs it nothing
+        // with human visitors. An undeclared or empty type stays in scope: a
+        // body with no `Content-Type` at all is exactly what the sniff is for.
+        // `is_html_like_content_type` answers true for an empty type as well as
+        // for `None`, so the emptiness is checked here: an origin that sends a
+        // bare `Content-Type:` has declared nothing, and treating that as a
+        // declaration of HTML would let a .docx back through the hole this
+        // exists to close.
+        let declared_html = content_type
+            .as_deref()
+            .is_some_and(|ct| !ct.is_empty() && crate::is_html_like_content_type(Some(ct)));
         let (html, raw_bytes) = if is_pdf {
             (String::new(), Some(bytes.to_vec()))
+        } else if !declared_html && looks_binary(&bytes, header_charset.as_deref()) {
+            // Logged rather than silent: this is the one path that returns a
+            // hard error without climbing the ladder, so if a class of real
+            // pages ever lands here it has to be visible in production.
+            tracing::info!(
+                url,
+                content_type = content_type.as_deref().unwrap_or("none"),
+                bytes = bytes.len(),
+                "binary body, returning unsupported content type without escalating"
+            );
+            return Err(CrwError::UnsupportedContentType(format!(
+                "{} ({} bytes): the body is binary, not HTML and not a PDF, \
+                 so there is nothing to extract",
+                content_type.as_deref().unwrap_or("no content-type"),
+                bytes.len()
+            )));
         } else {
             (decode_html_bytes(&bytes, header_charset.as_deref()), None)
         };
@@ -1098,6 +1143,34 @@ fn sniff_meta_charset(bytes: &[u8]) -> Option<String> {
     (!label.is_empty()).then(|| label.to_string())
 }
 
+/// True when a response body is binary rather than text. A NUL byte in the
+/// first 1KB is the standard heuristic (it is the one git uses): no HTML, JSON,
+/// XML, CSV or plain-text document in a single-byte or UTF-8 encoding carries
+/// one, while ZIP containers (.docx/.xlsx/.pptx), images and archives hit one
+/// within a few bytes.
+///
+/// UTF-16 documents are legitimately NUL-rich, so a declared UTF-16 charset opts
+/// out. The verdict comes from `encoding_rs` rather than a substring test on the
+/// label, because `decode_html_bytes` resolves the HEADER label the same way and
+/// the two must not disagree about it: a hand-written list misses
+/// `charset=unicode` and `charset=csunicode`, both of which
+/// `Encoding::for_label` maps to UTF-16LE and classic IIS still emits.
+///
+/// The agreement stops at the header label. `decode_html_bytes` also falls back
+/// to a `<meta charset>` sniff, and lets `Encoding::decode` override the label
+/// from a BOM; neither is mirrored here. Both gaps need a wide encoding under a
+/// non-HTML content type to matter at all, since a declared HTML-ish type skips
+/// this function outright, and `sniff_meta_charset` cannot read UTF-16 anyway.
+fn looks_binary(bytes: &[u8], header_charset: Option<&str>) -> bool {
+    // `for_label` lowercases and trims the label itself, so no normalisation here.
+    if let Some(enc) = header_charset.and_then(|l| encoding_rs::Encoding::for_label(l.as_bytes()))
+        && (enc == encoding_rs::UTF_16LE || enc == encoding_rs::UTF_16BE)
+    {
+        return false;
+    }
+    bytes[..bytes.len().min(1024)].contains(&0)
+}
+
 /// Decode fetched HTML bytes to a `String` honoring the declared charset
 /// (P1-1): HTTP `Content-Type` charset first, then a `<meta charset>` sniff,
 /// then UTF-8. Without this, a Latin-1 / Windows-1252 page has every 0x80–0xFF
@@ -1120,6 +1193,79 @@ fn decode_html_bytes(bytes: &[u8], header_charset: Option<&str>) -> String {
 mod tests {
     use super::*;
 
+    // ── looks_binary ────────────────────────────────────────────────────
+    #[test]
+    fn looks_binary_flags_a_zip_container() {
+        // Opening bytes of any .docx/.xlsx/.pptx.
+        assert!(looks_binary(b"PK\x03\x04\x14\x00\x08\x00\x00\x00", None));
+    }
+
+    #[test]
+    fn looks_binary_passes_html_and_empty_bodies() {
+        assert!(!looks_binary(
+            b"<!doctype html><html><body>hi</body></html>",
+            None
+        ));
+        assert!(!looks_binary(b"", None));
+    }
+
+    #[test]
+    fn looks_binary_respects_a_declared_wide_charset() {
+        // UTF-16LE "hi": NUL-rich, but genuinely text.
+        assert!(!looks_binary(b"h\x00i\x00", Some("utf-16le")));
+        assert!(looks_binary(b"h\x00i\x00", None));
+    }
+
+    #[test]
+    fn looks_binary_accepts_every_utf16_label_decode_html_bytes_accepts() {
+        // The two functions must agree on what counts as UTF-16, or a page one
+        // of them decodes the other rejects as binary. `unicode` in particular
+        // is what classic IIS emits, and a substring test on the label misses
+        // it: the page came back 422 instead of its text.
+        for label in [
+            "utf-16",
+            "UTF-16LE",
+            "utf-16be",
+            "ucs-2",
+            "unicode",
+            "csunicode",
+            "unicodefeff",
+            "unicodefffe",
+            "iso-10646-ucs-2",
+        ] {
+            assert!(
+                encoding_rs::Encoding::for_label(label.as_bytes()).is_some(),
+                "{label} is no longer a charset label, drop it from this test"
+            );
+            assert!(
+                !looks_binary(b"h\x00i\x00", Some(label)),
+                "{label} decodes as UTF-16 but was called binary"
+            );
+        }
+    }
+
+    #[test]
+    fn looks_binary_ignores_a_charset_that_is_not_wide() {
+        // A single-byte or UTF-8 label buys no exemption: those encodings never
+        // carry a NUL, so one means the header is lying about the body.
+        // `utf-32` is in this list on purpose: WHATWG has no UTF-32, so
+        // `for_label` rejects the label and `decode_html_bytes` cannot decode
+        // such a body either. An honest refusal beats handing back mojibake.
+        for label in [
+            "utf-8",
+            "windows-1252",
+            "iso-8859-1",
+            "shift_jis",
+            "utf-32",
+            "not-a-charset",
+        ] {
+            assert!(
+                looks_binary(b"PK\x03\x04\x14\x00", Some(label)),
+                "{label} should not exempt a ZIP container"
+            );
+        }
+    }
+
     /// Guards every test below that mutates process-wide env vars
     /// (`CRW_HTTP_TLS_RELAXED_FALLBACK`, `CRW_HTTP_RATELIMIT_PROXY_URL`,
     /// `HTTP_PROXY`/etc, `CRW_ALLOW_LOOPBACK_FOR_TESTS`). `cargo test` runs

crates/crw-renderer/src/lib.rs

@@ -1589,7 +1589,16 @@ impl FallbackRenderer {
                     .await
                 {
                     Ok(r) => r,
-                    Err(e) if !self.js_renderers.is_empty() => {
+                    // `UnsupportedContentType` is excluded on purpose: it means
+                    // the body is not a web page at all (a .docx ZIP, an image),
+                    // which no renderer can fix. Escalating one costs a full
+                    // lightpanda -> chrome -> camoufox climb (measured ~23s and a
+                    // Camoufox session) and still fails, with the precise content
+                    // type lost behind the ladder's generic "no usable content".
+                    Err(e)
+                        if !self.js_renderers.is_empty()
+                            && !matches!(e, CrwError::UnsupportedContentType(_)) =>
+                    {
                         tracing::info!(
                             url,
                             error = %e,
@@ -1741,7 +1750,16 @@ impl FallbackRenderer {
                     .await
                 {
                     Ok(r) => r,
-                    Err(e) if !self.js_renderers.is_empty() => {
+                    // `UnsupportedContentType` is excluded on purpose: it means
+                    // the body is not a web page at all (a .docx ZIP, an image),
+                    // which no renderer can fix. Escalating one costs a full
+                    // lightpanda -> chrome -> camoufox climb (measured ~23s and a
+                    // Camoufox session) and still fails, with the precise content
+                    // type lost behind the ladder's generic "no usable content".
+                    Err(e)
+                        if !self.js_renderers.is_empty()
+                            && !matches!(e, CrwError::UnsupportedContentType(_)) =>
+                    {
                         tracing::info!(
                             url,
                             error = %e,
@@ -4112,6 +4130,111 @@ mod tests {
         );
     }
 
+    /// An HTTP tier whose origin answered with a body that is not a web page.
+    struct BinaryBody;
+    #[async_trait::async_trait]
+    impl PageFetcher for BinaryBody {
+        async fn fetch(
+            &self,
+            _url: &str,
+            _h: &HashMap<String, String>,
+            _w: Option<u64>,
+            _d: crw_core::Deadline,
+        ) -> CrwResult<FetchResult> {
+            Err(CrwError::UnsupportedContentType(
+                "application/zip (1200 bytes): the body is binary".to_string(),
+            ))
+        }
+        fn name(&self) -> &str {
+            "http"
+        }
+        fn supports_js(&self) -> bool {
+            false
+        }
+        async fn is_available(&self) -> bool {
+            true
+        }
+    }
+
+    /// A body that is not a web page must NOT climb the ladder. No browser turns
+    /// a .docx into a page, so escalating one only spends a Chromium/Camoufox
+    /// session before failing anyway, and the ladder's generic "no usable
+    /// content" replaces the content type the caller needs to see.
+    ///
+    /// The call COUNT is the assertion that matters: the error variant alone
+    /// would survive deleting the guard, since the JS tier's own failure loses
+    /// to nothing here.
+    #[tokio::test]
+    async fn unsupported_content_type_does_not_climb_the_ladder_in_auto() {
+        let calls = Arc::new(std::sync::atomic::AtomicUsize::new(0));
+        let js = Arc::new(CountingFetcher {
+            name: "chrome",
+            calls: calls.clone(),
+        });
+        let mut r = make_renderer_with_mocks(vec![js]);
+        r.http = Arc::new(BinaryBody);
+        r.render_js_default = None; // auto branch
+
+        let err = r
+            .fetch(
+                "https://example.com/spec.docx",
+                &HashMap::new(),
+                None, // render_js: auto
+                None,
+                None,
+                tdl(),
+            )
+            .await
+            .expect_err("a binary body has nothing to extract");
+
+        assert_eq!(
+            calls.load(std::sync::atomic::Ordering::SeqCst),
+            0,
+            "the JS renderer must never be invoked for a binary body"
+        );
+        assert!(
+            matches!(err, CrwError::UnsupportedContentType(_)),
+            "the content type must reach the caller, not the ladder's generic \
+             failure; got {err:?}"
+        );
+    }
+
+    /// Same rule on the forced-JS arm. `renderJs: true` (and every screenshot
+    /// request, which is routed down this arm) fetches over HTTP first for the
+    /// content-type check, so it hits the identical guard.
+    #[tokio::test]
+    async fn unsupported_content_type_does_not_climb_the_ladder_when_js_is_forced() {
+        let calls = Arc::new(std::sync::atomic::AtomicUsize::new(0));
+        let js = Arc::new(CountingFetcher {
+            name: "chrome",
+            calls: calls.clone(),
+        });
+        let mut r = make_renderer_with_mocks(vec![js]);
+        r.http = Arc::new(BinaryBody);
+
+        let err = r
+            .fetch(
+                "https://example.com/spec.docx",
+                &HashMap::new(),
+                Some(true), // render_js: on
+                None,
+                None,
+                tdl(),
+            )
+            .await
+            .expect_err("a binary body has nothing to extract");
+
+        assert_eq!(
+            calls.load(std::sync::atomic::Ordering::SeqCst),
+            0,
+            "an explicit renderJs:true must not spend a browser on a binary body"
+        );
+        assert!(
+            matches!(err, CrwError::UnsupportedContentType(_)),
+            "got {err:?}"
+        );
+    }
+
     /// Control: with a healthy budget the same tier IS invoked. Guards against the
     /// floor silently disabling the ladder.
     #[tokio::test]