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]] diff --git a/src/registry/npm.rs b/src/registry/npm.rs index cff7928..3c48487 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, @@ -471,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 = @@ -1232,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( @@ -1245,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])), }, }, ); @@ -1257,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. 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") }