1
Fork 0
mirror of https://github.com/thegeneralist01/archivr synced 2026-07-21 18:55:36 +02:00

fix(core): address Codex review findings in reader image post-processor

P1 — Enforce size cap before buffering (singlefile.rs fetch_image_as_data_uri):
resp.bytes() buffered the entire response before checking MAX_BYTES, enabling
OOM on oversized or attacker-controlled images. Fix: reject via Content-Length
header when present, then stream at most max_bytes+1 bytes with Read::take so
the cap is enforced without materialising the full body first.

P2 — Decode HTML entities in extracted image URLs (inline_archivr_img_srcs):
The browser's HTML serialiser encodes & as & in attribute values, so CDN
signed URLs with query parameters (e.g. ?a=1&b=2) arrived as &-escaped
strings. reqwest sent the wrong URL, breaking signed or transformed CDN
images. Fix: unescape & < > " before passing to the fetcher.

P2 — Preserve authentication for same-origin lazy images (inline_archivr_img_srcs):
The post-processor created a bare reqwest client with no cookies, causing 401/403
for reader-mode captures of auth-gated sites whose lazy images are same-origin.
Fix: pass capture_url and cookies into inline_archivr_img_srcs; attach a Cookie
header only when domain_from_url(img_url) == domain_from_url(capture_url) and
cookies is non-empty. Third-party image hosts and Freedium fetches (which receive
empty cookies in capture.rs) are unaffected.
This commit is contained in:
TheGeneralist 2026-07-19 15:09:38 +02:00
parent 571da72649
commit 8c19d90e94
Signed by: thegeneralist01
SSH key fingerprint: SHA256:pp9qddbCNmVNoSjevdvQvM5z0DHN7LTa8qBMbcMq/R4

View file

@ -672,7 +672,7 @@ fn save_with(
// Post-process: fetch and inline images that SingleFile couldn't inline from the // Post-process: fetch and inline images that SingleFile couldn't inline from the
// page resource cache (resources introduced via DOM manipulation at before-capture // page resource cache (resources introduced via DOM manipulation at before-capture
// time are not tracked). Browser script stamped data-archivr-src on those images. // time are not tracked). Browser script stamped data-archivr-src on those images.
inline_archivr_img_srcs(&out_file); inline_archivr_img_srcs(&out_file, url, cookies);
let title = extract_html_title(&out_file); let title = extract_html_title(&out_file);
let html_hash = hash_file(&out_file)?; let html_hash = hash_file(&out_file)?;
@ -845,8 +845,10 @@ fn mime_to_favicon_ext(mime: &str) -> Option<&'static str> {
/// `src` is not already a large inlined image, fetches those URLs with blocking /// `src` is not already a large inlined image, fetches those URLs with blocking
/// reqwest, and replaces `src` with `data:<mime>;base64,…`. Non-fatal: any failure /// reqwest, and replaces `src` with `data:<mime>;base64,…`. Non-fatal: any failure
/// is logged and the image is left as-is. Guardrails: 10 s timeout, 5 redirects, /// is logged and the image is left as-is. Guardrails: 10 s timeout, 5 redirects,
/// `Content-Type: image/*` required, 20 MiB body cap. /// `Content-Type: image/*` required, 20 MiB body cap (enforced via `Content-Length`
fn inline_archivr_img_srcs(path: &Path) { /// and a bounded read). Cookies are forwarded only when the image host matches the
/// captured page's domain; Freedium fetches pass empty cookies so this is a no-op.
fn inline_archivr_img_srcs(path: &Path, capture_url: &str, cookies: &HashMap<String, String>) {
const MAX_BYTES: usize = 20 * 1024 * 1024; const MAX_BYTES: usize = 20 * 1024 * 1024;
let html = match std::fs::read_to_string(path) { let html = match std::fs::read_to_string(path) {
@ -867,9 +869,11 @@ fn inline_archivr_img_srcs(path: &Path) {
Err(_) => return, Err(_) => return,
}; };
let re_img = Regex::new(r"(?si)<img\b[^>]*>").unwrap(); let capture_domain = domain_from_url(capture_url);
let re_archivr = Regex::new(r#"(?i)\bdata-archivr-src="([^"]+)""#).unwrap();
let re_src = Regex::new(r#"(?i)\bsrc="([^"]*)""#).unwrap(); let re_img = Regex::new(r"(?si)<img\b[^>]*>").unwrap();
let re_archivr = Regex::new(r#"(?i)\bdata-archivr-src="([^"]+)""#).unwrap();
let re_src = Regex::new(r#"(?i)\bsrc="([^"]*)""#).unwrap();
let re_rm_archivr = Regex::new(r#"(?i)\s*data-archivr-src="[^"]*""#).unwrap(); let re_rm_archivr = Regex::new(r#"(?i)\s*data-archivr-src="[^"]*""#).unwrap();
let re_set_src = Regex::new(r#"(?i)\bsrc="[^"]*""#).unwrap(); let re_set_src = Regex::new(r#"(?i)\bsrc="[^"]*""#).unwrap();
@ -879,16 +883,44 @@ fn inline_archivr_img_srcs(path: &Path) {
for m in re_img.find_iter(&html) { for m in re_img.find_iter(&html) {
let tag = m.as_str(); let tag = m.as_str();
let img_url = match re_archivr.captures(tag).map(|c| c[1].to_string()) { let raw_url = match re_archivr.captures(tag).map(|c| c[1].to_string()) {
Some(u) => u, Some(u) => u,
None => continue, None => continue,
}; };
// HTML-decode common entities that the browser's serialiser encodes in
// attribute values (e.g. & → &amp; in CDN signed/query URLs).
let img_url = raw_url
.replace("&amp;", "&")
.replace("&lt;", "<")
.replace("&gt;", ">")
.replace("&quot;", "\"");
// Already a large inlined image — leave it alone. // Already a large inlined image — leave it alone.
let cur_src = re_src.captures(tag).map(|c| c[1].to_string()).unwrap_or_default(); let cur_src = re_src.captures(tag).map(|c| c[1].to_string()).unwrap_or_default();
if cur_src.starts_with("data:image/") && cur_src.len() > 1000 { if cur_src.starts_with("data:image/") && cur_src.len() > 1000 {
continue; continue;
} }
let data_uri = match fetch_image_as_data_uri(&client, &img_url, MAX_BYTES) {
// Forward cookies only when the image host exactly matches the captured
// page's domain. Avoids leaking credentials to third-party image hosts.
let img_domain = domain_from_url(&img_url);
let cookie_header = if !capture_domain.is_empty()
&& !img_domain.is_empty()
&& img_domain == capture_domain
&& !cookies.is_empty()
{
Some(
cookies
.iter()
.map(|(k, v)| format!("{k}={v}"))
.collect::<Vec<_>>()
.join("; "),
)
} else {
None
};
let data_uri = match fetch_image_as_data_uri(&client, &img_url, MAX_BYTES, cookie_header) {
Ok(u) => u, Ok(u) => u,
Err(e) => { Err(e) => {
eprintln!("warn: reader image inline ({img_url}): {e}"); eprintln!("warn: reader image inline ({img_url}): {e}");
@ -917,13 +949,19 @@ fn inline_archivr_img_srcs(path: &Path) {
} }
/// Fetches `url` and returns a `data:<mime>;base64,…` URI. /// Fetches `url` and returns a `data:<mime>;base64,…` URI.
/// Requires `Content-Type: image/*`, enforces `max_bytes` cap and 5-redirect limit. /// Requires `Content-Type: image/*`, enforces `max_bytes` cap via `Content-Length`
/// rejection and a bounded streaming read. Attaches `cookie_header` when supplied.
fn fetch_image_as_data_uri( fn fetch_image_as_data_uri(
client: &reqwest::blocking::Client, client: &reqwest::blocking::Client,
url: &str, url: &str,
max_bytes: usize, max_bytes: usize,
cookie_header: Option<String>,
) -> Result<String> { ) -> Result<String> {
let resp = client.get(url).send().context("request failed")?; let mut req = client.get(url);
if let Some(cookie) = cookie_header {
req = req.header(reqwest::header::COOKIE, cookie);
}
let resp = req.send().context("request failed")?;
if !resp.status().is_success() { if !resp.status().is_success() {
bail!("HTTP {}", resp.status()); bail!("HTTP {}", resp.status());
} }
@ -937,11 +975,23 @@ fn fetch_image_as_data_uri(
if !mime.starts_with("image/") { if !mime.starts_with("image/") {
bail!("non-image Content-Type: {mime}"); bail!("non-image Content-Type: {mime}");
} }
let bytes = resp.bytes().context("reading body")?; // Reject early when the server advertises a body larger than the cap.
if bytes.len() > max_bytes { if let Some(cl) = resp.content_length() {
bail!("image too large: {} bytes", bytes.len()); if cl as usize > max_bytes {
bail!("Content-Length {} exceeds {} MiB cap", cl, max_bytes / (1024 * 1024));
}
} }
Ok(format!("data:{};base64,{}", mime, B64.encode(&bytes))) // Stream at most max_bytes + 1 bytes so we never buffer an unbounded body.
// Reading one byte past the limit lets us distinguish "exactly at cap" from
// "over cap" without a separate Content-Length check.
let mut buf = Vec::new();
resp.take((max_bytes as u64) + 1)
.read_to_end(&mut buf)
.context("reading body")?;
if buf.len() > max_bytes {
bail!("image too large: > {} MiB", max_bytes / (1024 * 1024));
}
Ok(format!("data:{};base64,{}", mime, B64.encode(&buf)))
} }
#[cfg(test)] #[cfg(test)]