fix(crawler): budget image tail against actual sniff-prefix length
The per-image download cap subtracted the constant SNIFF_PREFIX_BYTES (64) from the budget instead of the bytes actually drained into the prefix. Because the prefix loop appends whole chunks, a single ~16 KiB first chunk fills the 64-byte sniff window in one drain, so the tail could then add another near-full cap — storing up to ~2× max_image_bytes per page. Capture prefix.len() before the prefix is moved into the stream and budget the tail as `max_image_bytes - prefix_len` via a small `remaining_after_prefix` helper (unit-tested for the overshoot and saturation cases). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2
backend/Cargo.lock
generated
2
backend/Cargo.lock
generated
@@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
|
||||
|
||||
[[package]]
|
||||
name = "mangalord"
|
||||
version = "0.93.3"
|
||||
version = "0.93.4"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"argon2",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
[package]
|
||||
name = "mangalord"
|
||||
version = "0.93.3"
|
||||
version = "0.93.4"
|
||||
edition = "2021"
|
||||
default-run = "mangalord"
|
||||
|
||||
|
||||
@@ -421,6 +421,15 @@ pub(crate) struct StoredPage {
|
||||
/// "first 16 bytes" diagnostic in the error path is useful.
|
||||
const SNIFF_PREFIX_BYTES: usize = 64;
|
||||
|
||||
/// Bytes still admissible for the streaming tail after the sniff prefix
|
||||
/// has been drained. The prefix already counts against the per-image cap,
|
||||
/// so the tail budget is `max_image_bytes - prefix_len` using the
|
||||
/// **actual** drained length — never a constant — so `prefix_len + tail`
|
||||
/// can never exceed the cap.
|
||||
fn remaining_after_prefix(max_image_bytes: usize, prefix_len: usize) -> usize {
|
||||
max_image_bytes.saturating_sub(prefix_len)
|
||||
}
|
||||
|
||||
/// Download a single page image, validate it's really an image, and
|
||||
/// stream it to storage. Returns the storage key + content type. Does
|
||||
/// not touch the DB — persistence is batched into one short transaction
|
||||
@@ -494,11 +503,18 @@ async fn download_and_store_page(
|
||||
// straight to storage. The cap is enforced via a running total in
|
||||
// the stream adapter so a server that omits Content-Length still
|
||||
// can't exhaust memory.
|
||||
// Budget the streaming tail against the bytes *actually* drained into
|
||||
// the prefix, not the constant `SNIFF_PREFIX_BYTES`. The prefix loop
|
||||
// appends whole chunks, so a single 16 KiB first chunk fills the
|
||||
// 64-byte sniff window in one drain — charging only 64 bytes would
|
||||
// then let the tail add another full `max_image_bytes`, storing up to
|
||||
// ~2× the cap. Capture the length before `prefix` is moved into the
|
||||
// stream below.
|
||||
let prefix_len = prefix.len();
|
||||
let prefix_stream = futures_util::stream::once(async move {
|
||||
Ok::<bytes::Bytes, StorageError>(prefix)
|
||||
});
|
||||
let prefix_len = SNIFF_PREFIX_BYTES.min(max_image_bytes);
|
||||
let mut remaining = max_image_bytes.saturating_sub(prefix_len);
|
||||
let mut remaining = remaining_after_prefix(max_image_bytes, prefix_len);
|
||||
let url_for_err = url.clone();
|
||||
let rest_stream = body.map(move |frame| match frame {
|
||||
Ok(chunk) => {
|
||||
@@ -644,6 +660,30 @@ mod tests {
|
||||
assert!(guard_nav_url("http://manga-host.test/c/1/p/2").is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tail_budget_uses_actual_prefix_length_not_constant() {
|
||||
// A single 16 KiB first chunk fills the 64-byte sniff window in one
|
||||
// drain, so the streaming tail budget must be `cap - 16KiB`, not
|
||||
// `cap - 64`. The constant-64 math previously admitted a further
|
||||
// ~full cap on top of the already-drained prefix (~2× overshoot).
|
||||
let cap = 20 * 1024;
|
||||
let prefix_len = 16 * 1024; // one real chunk, well over SNIFF_PREFIX_BYTES
|
||||
let remaining = remaining_after_prefix(cap, prefix_len);
|
||||
assert_eq!(remaining, cap - prefix_len);
|
||||
// Invariant: prefix + admitted tail never exceeds the cap.
|
||||
assert!(prefix_len + remaining <= cap);
|
||||
// And it's strictly tighter than the old constant-64 budget.
|
||||
assert!(remaining < cap - SNIFF_PREFIX_BYTES);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tail_budget_saturates_when_prefix_hits_cap() {
|
||||
// Body shorter than the sniff window: prefix drained == cap, tail
|
||||
// budget is zero (no negative underflow).
|
||||
assert_eq!(remaining_after_prefix(50, 50), 0);
|
||||
assert_eq!(remaining_after_prefix(50, 64), 0);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cleanup_orphans_deletes_written_keys() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "mangalord-frontend",
|
||||
"version": "0.93.3",
|
||||
"version": "0.93.4",
|
||||
"private": true,
|
||||
"type": "module",
|
||||
"scripts": {
|
||||
|
||||
Reference in New Issue
Block a user