From b0055f3f8270777e9192d3591bf97d677c999646 Mon Sep 17 00:00:00 2001 From: kridaydave Date: Tue, 13 Oct 2026 23:23:47 +0530 Subject: [PATCH 1/3] Stop allocating whole documents just to measure them Two hot paths built a copy of their input and threw it away. packument_size_estimate serialised every signature block to a String purely to read its length. The obvious spelling is the same one serde_json uses internally, so a counting io::Write gives a byte-for-byte identical count with no 64 MiB transient String behind it. The arithmetic is untouched and the exact-value test still pins it. tests/support/tarball.rs staged every fixture tar in a Vec, handed the whole thing to the encoder, and squeezed it at level 6. The bomb fixture is 200 MiB of zeroes and the entry-count fixture is 100_001 headers; in an opt-level 0 build that compression was most of the test. Write the tar straight into the encoder and compress at level 1. The only ratio any test asserts is that the bomb still arrives under 1 MiB. Measured on the dev box, tests/scenarios.rs: 24.26s -> 16.11s wall, 334 MB -> 30 MB peak RSS. gzip_bomb alone: 5.46s/209MB -> 1.50s/19MB. Commit made by claude-sonnet-4-6 in opencode on behalf of Kriday. --- src/registry/npm.rs | 36 ++++++++++++++++++++----- tests/support/tarball.rs | 58 +++++++++++++++++++++++++++------------- 2 files changed, 69 insertions(+), 25 deletions(-) diff --git a/src/registry/npm.rs b/src/registry/npm.rs index cff7928..94b93cf 100644 --- a/src/registry/npm.rs +++ b/src/registry/npm.rs @@ -298,18 +298,42 @@ fn packument_size_estimate(p: &Packument) -> u64 { // shape. Omitting it made a packument carrying a 32 MiB block // measure 1050 bytes, and a 64 MiB ceiling that does not bind is // worse than no ceiling: it looks like a bound. - let signatures = v - .dist - .signatures - .as_ref() - .and_then(|sig| serde_json::to_string(sig).ok()) - .map_or(0, |s| s.len() as u64); + let signatures = v.dist.signatures.as_ref().map_or(0, serialised_len); 512 + v.dist.tarball.len() as u64 + signatures }) .sum(); p.name.len() as u64 + 512 + versions } +/// Serialised length of a value without building the string. +/// +/// `serde_json::to_string(..).len()` is the obvious spelling and it allocates +/// the whole document to measure it: a packument carrying a 64 MiB signature +/// block paid 64 MiB of transient `String` on every call, on the hot path of +/// every memo insert. `to_writer` runs the same `CompactFormatter` over the +/// same `Value`, so the byte count is identical -- `String`, `Vec` and a +/// counting sink are the same stream -- and only the destination differs. +/// +/// Serialisation can still fail on a `Value` that cannot be written, so a +/// failed write counts as zero exactly as the old `.ok()` did. +fn serialised_len(v: &serde_json::Value) -> u64 { + struct CountingWriter(u64); + + impl std::io::Write for CountingWriter { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + self.0 += buf.len() as u64; + Ok(buf.len()) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } + + let mut counter = CountingWriter(0); + serde_json::to_writer(&mut counter, v).map_or(0, |()| counter.0) +} + #[derive(Default)] struct PackumentMemo { by_name: std::collections::HashMap, diff --git a/tests/support/tarball.rs b/tests/support/tarball.rs index cd119ad..220f167 100644 --- a/tests/support/tarball.rs +++ b/tests/support/tarball.rs @@ -133,7 +133,11 @@ impl Entry { /// Gzip a raw tar stream: the shape every npm tarball arrives in. pub fn archive(entries: &[Entry]) -> Vec { - let mut tar: Vec = Vec::new(); + // Written straight into the encoder. The entry-count fixture carries + // 100_001 headers and the bomb carries 200 MiB of payload, so staging the + // tar in a `Vec` first held a second full copy of every fixture (the bomb + // peaked at 209 MB) and copied it twice before the encoder ever saw it. + let mut encoder = flate2::write::GzEncoder::new(Vec::new(), fixture_compression()); for entry in entries { let size = match &entry.payload { Payload::Bytes(b) => b.len() as u64, @@ -143,43 +147,59 @@ pub fn archive(entries: &[Entry]) -> Vec { EntryKind::Symlink | EntryKind::Hardlink => entry.link_target.as_str(), _ => "", }; - tar.extend_from_slice(&ustar_header( - &entry.path, - entry.kind, - size, - link, - entry.mode, - )); + encoder + .write_all(&ustar_header( + &entry.path, + entry.kind, + size, + link, + entry.mode, + )) + .expect("write ustar header into fixture tarball"); match &entry.payload { Payload::Bytes(bytes) => { - tar.extend_from_slice(bytes); - pad_to_block(&mut tar, bytes.len() as u64); + encoder + .write_all(bytes) + .expect("write payload into fixture tarball"); + pad_to_block(&mut encoder, bytes.len() as u64); } Payload::Zeroes { len, chunk } => { let mut written: u64 = 0; while written < *len { let take = chunk.len().min((*len - written) as usize); - tar.extend_from_slice(&chunk[..take]); + encoder + .write_all(&chunk[..take]) + .expect("write zeroes into fixture tarball"); written += take as u64; } - pad_to_block(&mut tar, *len); + pad_to_block(&mut encoder, *len); } } } - tar.extend_from_slice(&[0u8; 1024]); - - let mut encoder = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::default()); - encoder.write_all(&tar).expect("gzip fixture tarball"); + encoder.write_all(&[0u8; 1024]).expect("write tar trailer"); encoder.finish().expect("finish gzip fixture tarball") } +/// Compression level for fixture tarballs. +/// +/// `fast` rather than `default`, and nothing here can tell the difference: +/// every consumer of a fixture infers it, and the only ratio any test asserts +/// is that a bomb of zeroes still arrives under 1 MiB, which holds at every +/// level. Level 6 on 100 MiB of synthetic tar in an `opt-level = 0` build was +/// the whole cost of `tar_entry_count_over_the_cap_is_refused`: 16.01 s of +/// which the review under test was a fraction. +fn fixture_compression() -> flate2::Compression { + flate2::Compression::fast() +} + /// Zero padding to the next 512-byte boundary. pub fn pad_to_512(len: usize) -> Vec { vec![0u8; (512 - len % 512) % 512] } -fn pad_to_block(buf: &mut Vec, written: u64) { - buf.extend_from_slice(&pad_to_512(written as usize)); +fn pad_to_block(buf: &mut W, written: u64) { + buf.write_all(&pad_to_512(written as usize)) + .expect("write tar block padding"); } /// A raw ustar header with an explicit typeflag, for the archive shapes @@ -238,7 +258,7 @@ fn ustar_header_raw(name: &str, typeflag: u8, size: u64, link_target: &str, mode /// Gzip raw tar bytes: the wire shape of every npm tarball. Paired with /// [`raw_header`] for archive shapes a builder cannot express. pub fn gzip(raw: &[u8]) -> Vec { - let mut encoder = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::default()); + let mut encoder = flate2::write::GzEncoder::new(Vec::new(), fixture_compression()); encoder.write_all(raw).expect("gzip fixture tarball"); encoder.finish().expect("finish gzip fixture tarball") } From c223fc4f9e5dc395a4aabb8b85779e19173a8c7b Mon Sep 17 00:00:00 2001 From: kridaydave Date: Wed, 14 Oct 2026 22:17:14 +0530 Subject: [PATCH 2/3] Size the memo fixture to its job instead of four times it the_packument_memo_is_bounded_and_the_estimate_is_not_a_fig_leaf built 64 versions carrying two 2 MiB signature blocks each, 256 MiB of payload, twice-estimated and once walked. 768 KiB blocks put it at 96 MiB, still 1.5x the 64 MiB ceiling the test exists to prove refuses. The 4x margin was not what pinned the ceiling, and I checked before cutting it. the_packument_memo_ceilings_are_the_documented_sizes pins both constants as values, so a mutant that changes 64 * 1024 * 1024 dies there whatever this fixture weighs. the_per_document_ceiling_is_strict pins the one-byte comparison by handing the ceilings in and using a 1057-byte document -- its own comment says the real ceiling "cannot express this without a 64 MiB fixture", which is why that helper takes them. the_whole_memo_ceiling_is_strict_and_adds_rather_than_multiplies covers sum-versus-product on the total. And the_packument_estimate_is_exact still pins the arithmetic, against to_string, so the counting writer keeps its independent oracle. Every assertion is still here and still means something. The floor moved from a loose "> 200 MiB" to >= SIGNATURE_FIXTURE_BYTES, which is tighter, not looser: an estimate that dropped the signature term would land near 64 KB and fail either way. Paired A/B on a quiet box, alternating, three rounds: OLD 256 MiB 11.85 / 11.88 / 12.02 s 277 MB peak RSS NEW 96 MiB 4.52 / 4.46 / 4.45 s 112 MB peak RSS Commit made by claude-sonnet-4-6 in opencode on behalf of Kriday. --- src/registry/npm.rs | 42 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/src/registry/npm.rs b/src/registry/npm.rs index 94b93cf..3c48487 100644 --- a/src/registry/npm.rs +++ b/src/registry/npm.rs @@ -495,6 +495,18 @@ mod tests { /// sha512 SRI of the bytes "tarball-content". const TEST_SRI: &str = "sha512-dWJ6JIJkmHG8N3fH1b/hbpmBQ7wKIpEw3zsVl2873OtFXh9QhR1KUU3uojohuIJ/xd+hb1R0q/57C8sMt4tstQ=="; + /// One signature block in the over-ceiling packument fixture, and how much + /// the fixture carries in total: 64 versions x 2 blocks x 768 KiB = 96 MiB, + /// against a 64 MiB per-document ceiling. + /// + /// Enough margin that the refusal the fixture asserts cannot be an + /// off-by-a-bit, and small enough that building and serialising it costs a + /// fraction of the 256 MiB this replaced. What that size was carrying, and + /// what still pins the ceiling without it, is in + /// `the_packument_memo_is_bounded_and_the_estimate_is_not_a_fig_leaf`. + const SIGNATURE_BLOCK_BYTES: usize = 768 * 1024; + const SIGNATURE_FIXTURE_BYTES: u64 = 64 * 2 * SIGNATURE_BLOCK_BYTES as u64; + #[test] fn list_versions_orders_and_limits() { let versions: BTreeMap = @@ -1256,9 +1268,30 @@ mod tests { /// and the memo as a whole. A packument over the first is not cached at /// all; exceeding the second clears what is held, so a recursive review /// cannot grow the memo without limit. + /// + /// The fixture carries [`SIGNATURE_FIXTURE_BYTES`] of signature payload, which + /// is 1.5x the per-document ceiling. It used to be 256 MiB (4x), and that + /// size was not carrying more coverage: + /// + /// * the ceiling constants are pinned as values by + /// `the_packument_memo_ceilings_are_the_documented_sizes`, so a mutant + /// that changes `64 * 1024 * 1024` dies there whatever this fixture is; + /// * the one-byte comparison (`>` against `>=`) is pinned by + /// `the_per_document_ceiling_is_strict`, which passes the ceilings in and + /// needs a 1057-byte document -- its own comment says the real ceiling + /// "cannot express this without a 64 MiB fixture"; + /// * the whole-memo ceiling, including sum-versus-product, is pinned by + /// `the_whole_memo_ceiling_is_strict_and_adds_rather_than_multiplies`; + /// * the estimate's own arithmetic is pinned by `the_packument_estimate_is_exact`. + /// + /// What this fixture is for is the wiring: a large, many-version packument + /// that goes through the production `insert` with the real ceiling and is + /// refused. It still does that. What it no longer buys is a 4x margin over + /// the ceiling, which mattered only against a mutant that raised the + /// constant *and* the assertion pinning it in the same build. #[test] fn the_packument_memo_is_bounded_and_the_estimate_is_not_a_fig_leaf() { - let big = "x".repeat(2 * 1024 * 1024); + let block = "x".repeat(SIGNATURE_BLOCK_BYTES); let mut versions = BTreeMap::new(); for i in 0..64 { versions.insert( @@ -1269,7 +1302,7 @@ mod tests { dist: Dist { tarball: "https://example.invalid/p.tgz".into(), integrity: None, - signatures: Some(serde_json::json!([big, big])), + signatures: Some(serde_json::json!([block, block])), }, }, ); @@ -1281,8 +1314,9 @@ mod tests { }; let size = packument_size_estimate(&p); assert!( - size > 200 * 1024 * 1024, - "a packument carrying 256 MiB of signature blocks must not estimate to {size} bytes" + size >= SIGNATURE_FIXTURE_BYTES, + "a packument carrying {} signature bytes must not estimate to {size} bytes", + SIGNATURE_FIXTURE_BYTES ); // Over the per-document ceiling: not cached. From 44089e8b60ad28d78024ccca81511d17f64664b2 Mon Sep 17 00:00:00 2001 From: kridaydave Date: Wed, 14 Oct 2026 23:03:05 +0530 Subject: [PATCH 3/3] fix(deps): bump rustls to 0.23.46 for the TLS session secret advisory --- Cargo.lock | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5128754..34c35b4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -429,7 +429,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -1092,14 +1092,14 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] name = "rustls" -version = "0.23.45" +version = "0.23.46" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0d41d731c7d2f962d1ccc364cec258de3c0e93b38c2fb3ba97ac74513048d634" +checksum = "48e13bd8c0e9365c43cfa5c9e8f9ad49d3c8444926c9aac819e0e4dc503c8fdf" dependencies = [ "log", "once_cell", @@ -1322,7 +1322,7 @@ dependencies = [ "getrandom 0.4.3", "once_cell", "rustix", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -1564,7 +1564,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]]