From 287a53e992199431972c7618b70cc1d4f1bc5828 Mon Sep 17 00:00:00 2001 From: rqi14 <26152437+rqi14@users.noreply.github.com> Date: Mon, 31 Aug 2026 07:45:20 +0100 Subject: [PATCH 1/2] fix(renderer): don't feed binary bodies to the HTML extractor `http_only.rs` decides how to handle a response body from the declared content type alone: let is_pdf = content_type.as_deref() == Some("application/pdf"); let (html, raw_bytes) = if is_pdf { ... } else { decode_html_bytes(...) }; so *anything* that isn't literally `application/pdf` is assumed to be text. Two consequences, both silent: 1. A `.docx`/`.xlsx`/`.pptx` (or any ZIP container) is UTF-8-lossy'd and run through the HTML extractor. The caller gets `success: true` with `markdown` beginning `PK\u{3}\u{4}...[Content_Types].xml`, 1 MB of binary noise that is indistinguishable from a real scrape. An LLM consumer will happily summarise it. 2. A real PDF served as `application/octet-stream` (common for S3 and for `Content-Disposition: attachment` endpoints) misses the PDF branch entirely and comes back as ~2 MB of `%PDF-1.5 ... /FlateDecode ... stream` source text instead of the parsed document. This adds two cheap checks in the same place: * `%PDF-` magic sniff. If the bytes are a PDF but the header disagrees, relabel `content_type` so the existing PDF path, and the `content_type == "application/pdf"` gate in `crw-crawl/src/single.rs`, engage as they already do for correctly-labelled PDFs. * NUL byte in the first 1 KB (git's binary heuristic) => the body is not text. Skipped when the header declares a wide charset (utf-16/32, ucs-*), whose HTML legitimately contains NULs. Binary bodies now raise a new `CrwError::UnsupportedContentType`, mapped to HTTP 422. It needs to be its own variant rather than `HttpError` because `classify_renderer_error` maps `HttpError` to `FailoverErrorKind::NetworkError`, which escalates: measured on a 1.3 MB `.docx`, that climbs lightpanda -> chrome -> camoufox and burns 23.06s and a Camoufox session before failing with the generic "Near-empty content (61 bytes)". No browser turns a `.docx` into a page, so both escalation gates in `lib.rs` now exclude the variant. Same document after: 0.005s, HTTP 422, and an error naming the actual content type. Measured against a local origin serving a real `.docx` and arXiv 1706.03762 mislabelled as `application/octet-stream`: | target | before | after | |------------------------|-------------------------------|--------------------------| | PDF as octet-stream | success, 1,969,949 ch of raw | success, 40,701 ch of | | | `%PDF-1.5 ... /FlateDecode` | article text, 0.286s | | .docx | success, 1,019,506 ch of ZIP | 422 in 0.005s, no ladder | Tests: 3 for `looks_binary` (ZIP flagged, HTML/empty passed, declared wide charset respected) and one asserting the server maps the new variant to 422 rather than falling through to the `_ => 500` arm. --- crates/crw-core/src/error.rs | 9 ++++ crates/crw-renderer/src/http_only.rs | 66 +++++++++++++++++++++++++++- crates/crw-renderer/src/lib.rs | 22 +++++++++- crates/crw-server/src/error.rs | 11 +++++ 4 files changed, 104 insertions(+), 4 deletions(-) diff --git a/crates/crw-core/src/error.rs b/crates/crw-core/src/error.rs index a9d1bc8b..7fceddc8 100644 --- a/crates/crw-core/src/error.rs +++ b/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", diff --git a/crates/crw-renderer/src/http_only.rs b/crates/crw-renderer/src/http_only.rs index 205390c8..4ccb2feb 100644 --- a/crates/crw-renderer/src/http_only.rs +++ b/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 / @@ -970,8 +970,29 @@ impl PageFetcher for HttpFetcher { ))); } - let (html, raw_bytes) = if is_pdf { + // 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. + let sniffed_pdf = bytes.starts_with(b"%PDF-"); + if sniffed_pdf && !is_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()); + } + let (html, raw_bytes) = if is_pdf || sniffed_pdf { (String::new(), Some(bytes.to_vec())) + } else if looks_binary(&bytes, header_charset.as_deref()) { + return Err(CrwError::UnsupportedContentType(format!( + "{} ({} bytes): 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 +1119,24 @@ fn sniff_meta_charset(bytes: &[u8]) -> Option { (!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/32 documents are legitimately NUL-rich, so a declared wide charset +/// opts out — `decode_html_bytes` handles those correctly. +fn looks_binary(bytes: &[u8], header_charset: Option<&str>) -> bool { + if let Some(label) = header_charset { + let label = label.to_ascii_lowercase(); + if label.contains("utf-16") || label.contains("utf-32") || label.contains("ucs-") { + 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 `` sniff, /// then UTF-8. Without this, a Latin-1 / Windows-1252 page has every 0x80–0xFF @@ -1120,6 +1159,29 @@ 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"hi", + 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)); + } + /// 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 diff --git a/crates/crw-renderer/src/lib.rs b/crates/crw-renderer/src/lib.rs index 8bfbc578..3941ff8d 100644 --- a/crates/crw-renderer/src/lib.rs +++ b/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, diff --git a/crates/crw-server/src/error.rs b/crates/crw-server/src/error.rs index 25b579f4..28c8878e 100644 --- a/crates/crw-server/src/error.rs +++ b/crates/crw-server/src/error.rs @@ -45,6 +45,7 @@ impl IntoResponse for AppError { CrwError::HttpError(_) => StatusCode::BAD_GATEWAY, CrwError::TargetUnreachable(_) => StatusCode::UNPROCESSABLE_ENTITY, CrwError::ExtractionError(_) => StatusCode::UNPROCESSABLE_ENTITY, + CrwError::UnsupportedContentType(_) => StatusCode::UNPROCESSABLE_ENTITY, CrwError::RateLimited => StatusCode::TOO_MANY_REQUESTS, CrwError::SearchDisabled(_) => StatusCode::SERVICE_UNAVAILABLE, CrwError::SearchDegraded(_) => StatusCode::SERVICE_UNAVAILABLE, @@ -76,6 +77,16 @@ mod tests { ); } + #[test] + fn app_error_unsupported_content_type_422_not_500() { + // Must be an explicit arm: the `_` fallback would report a .docx as a + // 500, blaming the server for the origin's media type. + assert_eq!( + status_for(CrwError::UnsupportedContentType("application/zip".into())), + StatusCode::UNPROCESSABLE_ENTITY + ); + } + #[test] fn app_error_not_found_404() { assert_eq!( From 39c37cecf164f6df196c9c20b7ee3b7f052db81f Mon Sep 17 00:00:00 2001 From: us Date: Mon, 31 Aug 2026 20:00:40 +0300 Subject: [PATCH 2/2] fix(renderer): keep the binary-body check off real pages The charset opt-out in `looks_binary` was a substring test on the header label while `decode_html_bytes` resolves it with `encoding_rs::Encoding::for_label`. The two sets differ: `unicode`, `csunicode`, `unicodefeff` and `unicodefffe` all map to UTF-16 and none of them matched. Measured on the same UTF-16 bytes, only the label differing: `charset=utf-16le` returned 200 with the page text, `charset=unicode` returned 422. Classic IIS emits `unicode`. The opt-out now asks `for_label` directly so the two cannot drift. `utf-32` leaves the list because WHATWG has no such encoding and `for_label` rejects it. The NUL test also ran on bodies the origin correctly declared as HTML. A stray NUL in HTML is not a reason to refuse a page, since the HTML5 tokenizer maps it to U+FFFD and browsers render it, and refusing would hand any origin a one-byte way to stop the renderer ladder at no cost to its human visitors. It is now skipped for a declared HTML-ish type, via the existing `is_html_like_content_type`. An absent or empty `Content-Type` is not a declaration and stays in scope, which is the case the byte sniff exists for. Also: * `is_pdf` is computed after the `%PDF-` relabel, so a sniffed PDF and a declared one are one branch rather than a disjunction repeated at each use, and `rendered_with` no longer contradicts the relabelled type. * the refusal is logged. It is the only path that returns a hard error without climbing the ladder, so a class of pages landing there by mistake has to be visible in production. * `crw scrape` no longer escalates on it. The CLI runs its own phase-2 escalation outside `FallbackRenderer`, so it still spawned LightPanda and Chrome for a .docx and printed "trying JS renderer" first. It returns `CmdError` rather than exiting directly, so teardown keeps owning the single exit path. Tests: two that count JS-renderer invocations to prove the ladder is not climbed, and a wiremock file driving `FallbackRenderer::fetch` against a real origin for the octet-stream relabel, the refusal, a body with no declared type, and a UTF-16 page. Deleting the call site, the relabel, either escalation guard, the charset opt-out or the HTML carve-out each fails at least one of them; the previous unit tests passed through all of it. Docs: the new code joins the two error tables, and the PDF page no longer says parsing needs an `application/pdf` response. --- crates/crw-cli/src/commands/scrape.rs | 12 ++ crates/crw-renderer/src/http_only.rs | 116 ++++++++++-- crates/crw-renderer/src/lib.rs | 105 +++++++++++ .../crw-renderer/tests/binary_body_routing.rs | 166 ++++++++++++++++++ docs/docs/error-codes.md | 3 +- docs/docs/pdf-parsing.md | 2 +- docs/docs/troubleshooting.md | 6 +- 7 files changed, 390 insertions(+), 20 deletions(-) create mode 100644 crates/crw-renderer/tests/binary_body_routing.rs diff --git a/crates/crw-cli/src/commands/scrape.rs b/crates/crw-cli/src/commands/scrape.rs index cc933a61..9a1f136b 100644 --- a/crates/crw-cli/src/commands/scrape.rs +++ b/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..."); diff --git a/crates/crw-renderer/src/http_only.rs b/crates/crw-renderer/src/http_only.rs index 4ccb2feb..d46fd94d 100644 --- a/crates/crw-renderer/src/http_only.rs +++ b/crates/crw-renderer/src/http_only.rs @@ -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 @@ -977,18 +975,44 @@ impl PageFetcher for HttpFetcher { // 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. - let sniffed_pdf = bytes.starts_with(b"%PDF-"); - if sniffed_pdf && !is_pdf { + 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. + // 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()); } - let (html, raw_bytes) = if is_pdf || sniffed_pdf { + // 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 looks_binary(&bytes, header_charset.as_deref()) { + } 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): not HTML and not a PDF, \ + "{} ({} 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() @@ -1125,14 +1149,24 @@ fn sniff_meta_charset(bytes: &[u8]) -> Option { /// one, while ZIP containers (.docx/.xlsx/.pptx), images and archives hit one /// within a few bytes. /// -/// UTF-16/32 documents are legitimately NUL-rich, so a declared wide charset -/// opts out — `decode_html_bytes` handles those correctly. +/// 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 `` 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 { - if let Some(label) = header_charset { - let label = label.to_ascii_lowercase(); - if label.contains("utf-16") || label.contains("utf-32") || label.contains("ucs-") { - return false; - } + // `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) } @@ -1182,6 +1216,56 @@ mod tests { 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 diff --git a/crates/crw-renderer/src/lib.rs b/crates/crw-renderer/src/lib.rs index 3941ff8d..847e63cd 100644 --- a/crates/crw-renderer/src/lib.rs +++ b/crates/crw-renderer/src/lib.rs @@ -4130,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, + _w: Option, + _d: crw_core::Deadline, + ) -> CrwResult { + 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] diff --git a/crates/crw-renderer/tests/binary_body_routing.rs b/crates/crw-renderer/tests/binary_body_routing.rs new file mode 100644 index 00000000..87903f51 --- /dev/null +++ b/crates/crw-renderer/tests/binary_body_routing.rs @@ -0,0 +1,166 @@ +//! What the HTTP tier does with a body that is not HTML. +//! +//! The tier used to route on the declared `Content-Type` alone: `application/pdf` +//! went to the PDF parser and EVERYTHING else was UTF-8-lossy'd into the HTML +//! extractor. So a .docx came back as `success: true` with markdown beginning +//! `PK\x03\x04...[Content_Types].xml`, and a real PDF served as +//! `application/octet-stream` (what S3 and `Content-Disposition: attachment` +//! endpoints send) came back as megabytes of raw `%PDF-1.5 ... /FlateDecode` +//! source text. +//! +//! These drive `FallbackRenderer::fetch` against a real origin, because the unit +//! tests next to `looks_binary` cover the helper only: deleting its call site, +//! or the `%PDF-` relabel, leaves every one of them green. + +use std::collections::HashMap; + +use crw_core::Deadline; +use crw_core::config::{RendererConfig, RendererMode, StealthConfig}; +use crw_core::error::CrwError; +use crw_core::types::FetchResult; +use crw_renderer::FallbackRenderer; +use wiremock::matchers::method; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +fn renderer() -> FallbackRenderer { + let cfg = RendererConfig { + mode: RendererMode::None, + ..Default::default() + }; + FallbackRenderer::new(&cfg, "crw-test", None, &StealthConfig::default()) + .expect("renderer builds in http-only mode") +} + +/// Serve `body` under `content_type` from a throwaway origin and fetch it. +async fn fetch_body(body: Vec, content_type: &str) -> Result { + // SAFETY: one process per tests/*.rs file, so this binary owns its env. + unsafe { + std::env::set_var("CRW_ALLOW_LOOPBACK_FOR_TESTS", "1"); + } + let origin = MockServer::start().await; + Mock::given(method("GET")) + .respond_with( + ResponseTemplate::new(200) + .set_body_bytes(body) + .insert_header("content-type", content_type), + ) + .mount(&origin) + .await; + renderer() + .fetch( + &origin.uri(), + &HashMap::new(), + Some(false), + None, + Some("auto"), + Deadline::from_request_ms(15_000), + ) + .await +} + +/// The opening bytes of any .docx/.xlsx/.pptx, plus enough of a member name to +/// look like the real thing in a failure message. +fn zip_container() -> Vec { + let mut b = b"PK\x03\x04\x14\x00\x08\x00\x00\x00".to_vec(); + b.extend_from_slice(b"[Content_Types].xml"); + b.extend_from_slice(&[0u8; 64]); + b +} + +#[tokio::test] +async fn a_zip_container_is_refused_instead_of_extracted_as_html() { + let err = fetch_body( + zip_container(), + "application/vnd.openxmlformats-officedocument.wordprocessingml.document", + ) + .await + .expect_err("a ZIP container has nothing to extract"); + + match err { + CrwError::UnsupportedContentType(msg) => { + assert!( + msg.contains("wordprocessingml"), + "the caller needs the actual content type to act on: {msg}" + ); + } + other => panic!("expected UnsupportedContentType, got {other:?}"), + } +} + +#[tokio::test] +async fn a_pdf_served_as_octet_stream_is_relabelled_so_the_parser_engages() { + // `crw-crawl` gates its PDF branch on `content_type == "application/pdf"`, + // so the relabel is the whole mechanism: without it the parser never runs + // and the caller gets the raw source instead of the document. + let pdf = + b"%PDF-1.5\n1 0 obj\n<< /Type /Catalog >>\nendobj\ntrailer\n<< /Root 1 0 R >>\n%%EOF\n"; + let r = fetch_body(pdf.to_vec(), "application/octet-stream") + .await + .expect("a PDF is fetchable whatever the origin calls it"); + + assert_eq!(r.content_type.as_deref(), Some("application/pdf")); + assert_eq!(r.rendered_with.as_deref(), Some("pdf")); + assert!( + r.raw_bytes.is_some_and(|b| b.starts_with(b"%PDF-")), + "the parser needs the bytes, not a decoded string" + ); + assert!( + r.html.is_empty(), + "a PDF has no DOM, so nothing must be handed to the HTML extractor" + ); +} + +#[tokio::test] +async fn a_declared_html_body_survives_a_stray_nul() { + // A NUL in HTML is not a reason to refuse the page: a real browser maps it + // to U+FFFD and renders normally, so refusing would both cost a page we + // scrape today and hand any origin a one-byte way to shut the ladder down. + let mut body = b"

Still a page

".to_vec(); + body.push(0); + body.extend_from_slice(b"tail

"); + + let r = fetch_body(body, "text/html; charset=utf-8") + .await + .expect("a NUL does not make an HTML page unscrapable"); + assert!(r.html.contains("Still a page")); +} + +#[tokio::test] +async fn a_utf16_page_declared_as_unicode_still_decodes() { + // `unicode` and `csunicode` are UTF-16LE to `encoding_rs`, which is what + // decodes the body, and classic IIS still emits the former. A substring + // test on the label misses both and turns the page into a hard error. + let body: Vec = "

Wide load

" + .encode_utf16() + .flat_map(u16::to_le_bytes) + .collect(); + + // Served as `text/plain`, NOT `text/html`, on purpose: a declared HTML-ish + // type skips the binary check entirely, so serving it as `text/html` would + // pass with the UTF-16 exemption deleted and prove nothing. + for label in ["unicode", "csunicode", "utf-16le"] { + let r = fetch_body(body.clone(), &format!("text/plain; charset={label}")) + .await + .unwrap_or_else(|e| panic!("charset={label} must decode, got {e:?}")); + assert!( + r.html.contains("Wide load"), + "charset={label} decoded to the wrong thing" + ); + } +} + +#[tokio::test] +async fn a_binary_body_with_no_declared_type_is_still_refused() { + // The gate skips the byte check for a DECLARED HTML-ish type. An origin + // that declares nothing, or sends a bare `Content-Type:`, has not declared + // HTML, and an unlabelled body is exactly what a byte sniff is for. + for content_type in ["", "application/octet-stream"] { + let err = fetch_body(zip_container(), content_type) + .await + .expect_err("an unlabelled ZIP container still has nothing to extract"); + assert!( + matches!(err, CrwError::UnsupportedContentType(_)), + "content-type {content_type:?} got {err:?}" + ); + } +} diff --git a/docs/docs/error-codes.md b/docs/docs/error-codes.md index 6db2c54b..f2b78765 100644 --- a/docs/docs/error-codes.md +++ b/docs/docs/error-codes.md @@ -24,7 +24,7 @@ You need both to debug real scraping failures. | 400 | Invalid request parameters (bad URL, invalid JSON body, invalid selector) | | 401 | Invalid or missing API key | | 404 | Endpoint not found | -| 422 | Validation failed (unknown format, invalid schema, extraction error) | +| 422 | Validation failed (unknown format, invalid schema, extraction error), or the origin returned a body that is not a page (`unsupported_content_type`) | | 429 | Rate limit or credit quota exceeded | | 502 | Engine internal error | | 503 | Server at capacity | @@ -37,6 +37,7 @@ You need both to debug real scraping failures. | HTTP `200` with `warning` | The request succeeded, but the target result is degraded | Inspect `warning` and `metadata.statusCode` | | HTTP `400` | Your request body is invalid | Fix fields, selectors, or schema | | HTTP `422` | The request shape is valid JSON but semantically invalid | Check format names, schema, or extraction config | +| `unsupported_content_type` | The origin served an office document, image or archive rather than a page | Fetch the file yourself, or point the request at a PDF or an HTML page | | HTTP `429` | Rate limit or credit ceiling hit | Back off and honor `Retry-After` | | HTTP `502` / `504` | Upstream or timeout issue | Retry with backoff | diff --git a/docs/docs/pdf-parsing.md b/docs/docs/pdf-parsing.md index 565ba33a..ede6e09a 100644 --- a/docs/docs/pdf-parsing.md +++ b/docs/docs/pdf-parsing.md @@ -434,5 +434,5 @@ sandbox = false # isolate each parse in a child process ## When to use something else - Use [Scrape](#scraping) when the document is a web page, not a binary file -- Use [Extract](#extract) when you already have a URL to a PDF (scrape fetches and parses automatically when the response is `application/pdf`) +- Use [Extract](#extract) when you already have a URL to a PDF (scrape fetches and parses it automatically, whether the origin labels it `application/pdf` or ships it as `application/octet-stream`, which is what S3 and `Content-Disposition: attachment` endpoints send) - Use [Crawl](#crawling) when you need to discover PDFs across an entire site before parsing them diff --git a/docs/docs/troubleshooting.md b/docs/docs/troubleshooting.md index d9c6d5dc..fdbd4055 100644 --- a/docs/docs/troubleshooting.md +++ b/docs/docs/troubleshooting.md @@ -153,8 +153,9 @@ curl -X POST https://api.fastcrw.com/v1/scrape \ **Error code mapping** (from `crw-server/src/error.rs`): ``` -CrwError::TargetUnreachable → HTTP 422, error_code: "target_unreachable" -CrwError::ExtractionError → HTTP 422, error_code: "extraction_error" +CrwError::TargetUnreachable → HTTP 422, error_code: "target_unreachable" +CrwError::ExtractionError → HTTP 422, error_code: "extraction_error" +CrwError::UnsupportedContentType → HTTP 422, error_code: "unsupported_content_type" ``` If the target URL is correct and externally reachable, the issue is intermittent — retry with backoff. If it is consistent, the target host has a problem unrelated to fastCRW. @@ -411,6 +412,7 @@ if data["balance"] < 100: | `invalid_url` | — | Reserved — not emitted in practice; invalid URLs are returned as `invalid_request` (HTTP 400) by all server routes | | `target_unreachable` | 422 | DNS failure, connection refused, host down | | `extraction_error` | 422 | LLM extraction failed or CSS/XPath selector invalid | +| `unsupported_content_type` | 422 | The origin answered with a body that is neither a web page nor a PDF (an office document, an image, an archive). Not retryable: no renderer turns one into a page | | `http_error` | 502, or 200 with `success: false` | The origin answered with a `>= 400` status and what came back is its error page. The body is kept in `data` so you can read the error page and `metadata.statusCode`. A large page served under an error status is still returned as a success — some sites answer 403/404 while serving the real content. If the page is also recognised as an anti-bot wall, `anti_bot` wins instead and the body is cleared | | `no_usable_content` | 200 with `success: false` | The fetch worked and produced nothing you asked for — a parked domain, an un-hydrated JS shell, an error stub, an oversized PDF the decompression-bomb guard refused, or any requested format that came back empty. Not an anti-bot block | | `timeout` | 504 | Engine or upstream search timed out |