Comment and Documentation Cleanup

- Cap every inline `//` comment block at 3 lines; distil the rationale
  rather than delete it. Enforced by a new comment-guard CI check.
- Fix stale and orphaned rustdoc: broken intra-doc links (renamed or
  removed items), doc blocks that had merged onto the wrong item (leaving
  the intended item undocumented), and doc-contradicts-code drift.
- Add a doc-links CI check (cargo doc, deny broken intra-doc links) so
  both classes of comment rot are caught going forward.
This commit is contained in:
Matthew Jackson
2026-08-27 12:07:53 -07:00
parent 9ed6f86255
commit 9f43ae94d3
143 changed files with 4883 additions and 10834 deletions
+11 -17
View File
@@ -100,10 +100,9 @@ fn aacs_decrypt_unit_roundtrip() {
plain[offset] = 0x47; // TS sync byte
offset += 192;
}
// Flag the unit encrypted via the CPI bits (byte 0) — the authoritative gate
// `decrypt_unit` consults. Set BEFORE snapshotting `expected`: decrypt
// preserves the plaintext header, so the round-tripped unit carries these
// bits too.
// Flag the unit encrypted via the CPI bits (byte 0), the gate `decrypt_unit`
// consults. Set before snapshotting `expected`: decrypt preserves the
// plaintext header, so the round-tripped unit carries these bits too.
plain[0] |= 0xC0;
// Save original plaintext for comparison
@@ -514,10 +513,9 @@ fn css_roundtrip_with_snapshot() {
/// Multiple key/seed combinations produce different outputs.
#[test]
fn css_roundtrip_multiple_keys() {
// The content cipher (dvdcss_unscramble) seeds its LFSRs from
// `key XOR seed`, so the cases must have DISTINCT `key XOR seed` values to
// produce distinct keystreams. (key=0/seed=0 and key=FF/seed=FF both give
// key^seed=0 and are correctly identical — excluded here.)
// The cipher seeds its LFSRs from `key XOR seed`, so cases need DISTINCT
// key^seed values to produce distinct keystreams (key=0/seed=0 and
// key=FF/seed=FF both give key^seed=0 and are excluded here).
let cases: &[([u8; 5], [u8; 5])] = &[
(
[0x00, 0x00, 0x00, 0x00, 0x00],
@@ -647,11 +645,9 @@ fn css_keyless_recovery_validates_cracked_key() {
This is expected: synthetic sectors lack the TAB1 output encoding \
present in real CSS-encrypted DVD sectors."
);
// Never let this test pass vacuously: when the attack can't converge on
// synthetic data, still assert always-true properties of the CSS keystream
// so a real regression is caught on every run — descramble_sector is
// DETERMINISTIC (same key/seed/data → same output) and NON-TRIVIAL (it
// actually transforms the payload, not a silent no-op).
// Don't pass vacuously: if the attack can't converge on synthetic data,
// still assert descramble_sector is deterministic and non-trivial
// (transforms the payload) so a real regression is still caught.
for (key, seed) in candidates {
let mut base = vec![0x00u8; 2048];
base[0x14] = 0x30;
@@ -713,10 +709,8 @@ fn css_recover_title_key_with_exact_plaintext() {
The LFSR0 recovery phase may not converge for this combination.",
title_key, seed
);
// Never pass vacuously: when LFSR0 recovery can't converge on this
// synthetic sector, still assert always-true properties of the cipher so
// a real regression is caught on every run — descramble_sector is
// DETERMINISTIC and NON-TRIVIAL (actually transforms the payload).
// Don't pass vacuously: if recovery didn't converge, still assert
// descramble_sector is deterministic and non-trivial (transforms payload).
let mut a = original.clone();
let mut b = original.clone();
css::lfsr::descramble_sector(&title_key, &mut a);
+12 -26
View File
@@ -378,18 +378,9 @@ fn resolve_encryption_no_aacs_dir() {
assert!(disc.aacs.is_none(), "aacs should be None without /AACS dir");
}
// ── Batch count arithmetic tests ──────────────────────────────────────────
// Regression tests for the u16 truncation bug in the prefetch producer's
// per-batch sector count (`src/sector/prefetched.rs`): when
// `(remaining as u16).min(batch_sectors)` was used instead of
// `remaining.min(batch_sectors as u32) as u16`, any remaining count that is a
// multiple of 65536 truncated to 0.
//
// These tests used to assert against `safe_batch_count`/`buggy_batch_count`
// copies defined in THIS file, so the production expression could be reverted
// with every one of them staying green. They now drive the real producer
// through the public `PrefetchedSectorSource` API and assert on the sector
// count of the batch it actually emits.
// ── Batch count arithmetic: regression for a u16 truncation bug where
// `(remaining as u16).min(batch_sectors)` truncated to 0 when remaining was a
// multiple of 65536. Drives the real producer via the public API, not a local copy.
/// Endless zero-filled source: every read succeeds with the full requested
/// span, so the producer's batch size is the only thing the returned byte
@@ -458,10 +449,9 @@ fn batch_count_exact_boundary() {
#[test]
fn batch_count_u16_overflow_regression() {
// THE BUG: remaining is a multiple of 65536 → `remaining as u16` is 0, so
// the batch collapses (the unit-alignment clamp below it then floors the
// batch at one 3-sector AACS unit — a 20x throughput cliff on exactly the
// disc sizes that hit it, and an outright stall before that clamp existed).
// THE BUG: remaining a multiple of 65536 → `remaining as u16` is 0, so the
// batch collapses (a 20x throughput cliff post-clamp, an outright stall
// before the unit-alignment clamp existed).
let remaining: u32 = 47533152 - 19552; // = 47513600 = 725 * 65536
assert_eq!(remaining, 47513600);
assert_eq!(
@@ -572,12 +562,9 @@ fn scan_encrypted_resolves_no_keys() {
#[test]
fn aacs_dir_alone_marks_the_disc_encrypted_and_reports_the_capture_error() {
// Encryption detection is an OR over the two on-disc AACS locations:
// `/AACS` (Blu-ray / UHD, ECMA-167 root) and `/BDMV/AACS` (the BDMV-nested
// variant). This fixture carries ONLY `/AACS`, the standard retail layout,
// so a detector that required BOTH would call a genuinely encrypted disc
// clear — the worst possible failure here, because a "clear" disc is muxed
// straight through and ships ciphertext as if it were video, at exit 0.
// Detection is an OR over `/AACS` and `/BDMV/AACS`. This fixture carries
// only `/AACS` (the standard retail layout); requiring BOTH would call it
// clear — the worst failure, since a "clear" disc muxes ciphertext as video.
let mut reader = MockSectorReader::new();
build_udf_with_aacs_dir(&mut reader);
@@ -587,10 +574,9 @@ fn aacs_dir_alone_marks_the_disc_encrypted_and_reports_the_capture_error() {
disc.encrypted,
"a disc carrying /AACS is encrypted even though /BDMV/AACS is absent"
);
// Encrypted => the scan attempts the (lookup-free) AACS input capture. This
// fixture's /AACS is empty, so that capture fails and the failure must be
// PRESERVED on the disc: callers render it, and its absence is what a scan
// that never attempted the capture at all would look like.
// Encrypted => scan attempts the AACS input capture. This fixture's /AACS
// is empty, so capture fails and the failure must be PRESERVED on the disc
// (its absence would look identical to never having attempted capture).
assert!(
disc.aacs_error.is_some(),
"the failed AACS capture on an encrypted disc must be surfaced, not dropped"
+3 -5
View File
@@ -294,11 +294,9 @@ fn fvi_sink_indexes_real_mpeg2_pipeline_output() {
vec![0, 3],
"stamped src sectors must reach the .fvi in arrival order; got {src_sectors:?}"
);
// Per FVI_FORMAT.md §9, `src.byte` is the offset of the AU's first byte
// WITHIN its 2048-byte `src.sector`, so it is always < 2048. The two
// stamped sources sit 14 bytes into their sector (just past the pack
// header), so the within-sector byte is exact (14) — provenance is
// byte-exact, carried unchanged from demux.
// Per FVI_FORMAT.md §9, `src.byte` is the AU's offset WITHIN its 2048-byte
// `src.sector` (always < 2048). These sources sit 14 bytes into their
// sector (past the pack header), so byte must be exactly 14.
for r in &records {
if let Some(byte) = r["src"]["byte"].as_u64() {
assert!(
+3 -16
View File
@@ -25,22 +25,9 @@ fn decrypt_sectors_with_none_keys_is_noop() {
/// Test: decrypt_sectors with CSS keys descrambles sectors.
#[test]
fn css_decrypt_of_an_uncrackable_sector_still_descrambles() {
// A scrambled sector whose header is uniformly periodic yields a crib, so
// the supplied key IS checked — and this arbitrary key is not the right
// one, so the crib check rejects it and the re-crack from this synthetic
// body finds nothing.
//
// That combination does NOT fail the rip. `attack_crib` is a heuristic: it
// predicts that a periodic header run continues past 0x80, and when that
// prediction does not hold it reports a mismatch even for a CORRECT key —
// whereupon the re-crack fails because the crib was never valid. Crib
// mismatch plus crack failure is the signature of a crib false positive,
// and the cached key stays the best available evidence.
//
// This test previously asserted DecryptFailed, matching a round-9 change
// that made real DVDs unrippable. CSS is not AACS: an AACS
// unit key either opens a unit or does not, whereas a CSS title key is
// recovered from data whose recoverability varies sector by sector.
// Wrong key: the header crib rejects it and re-crack finds nothing, but that
// is not a rip failure — `attack_crib` is a heuristic, so this mismatch just
// means the cached key stands (CSS recoverability varies per sector, unlike AACS).
let mut sector = vec![0xFFu8; 2048];
// Scrambled DVD sectors are MPEG-2 PS packs. The descramble policy requires
// the pack start code as well as the flag bits, because byte 0x14 means
+3 -5
View File
@@ -145,11 +145,9 @@ fn scan_iso_matches_manual_scan_image_path() {
assert_eq!(disc.encrypted, manual.encrypted, "encrypted flag");
assert_eq!(disc.format, manual.format, "disc format");
// Independent expectations (not parity against a re-run of the same
// composition): the scanned Disc must match KNOWN properties of the fixture
// itself — its capacity (max LBA + 1), its PVD volume id ("TEST_DISC"), and
// that a UDF with no /AACS directory is unencrypted. These would fail even if
// scan_iso and the manual path drifted together.
// Independent expectations, not parity against a re-run: the scanned Disc
// must match KNOWN fixture properties (capacity, PVD volume id, unencrypted
// with no /AACS) — these would fail even if both scan paths drifted together.
assert_eq!(disc.capacity_sectors, expected_capacity, "capacity value");
assert_eq!(
disc.volume_id, "TEST_DISC",
+2 -4
View File
@@ -338,10 +338,8 @@ fn test_healthy_short_transfer_reports_partial_bytes() {
}
// ── 13. Error::Display does not leak English in the SCSI variant ──────────
//
// Library rule (project docs): no English in error display, only "E{code}"
// + structured data. Regression guard for the SCSI variant specifically
// because it's the most-emitted error in the rip path.
// Library rule: no English in error display, only "E{code}" + structured data.
// Guards the SCSI variant since it's the most-emitted error in the rip path.
#[test]
fn test_scsi_error_display_format_is_codes_only() {
+2 -3
View File
@@ -629,9 +629,8 @@ fn mkvstream_meta_sets_title() {
#[test]
fn mkvstream_roundtrip_bdts() {
// Write BD-TS packets through MkvStream and verify the pipeline works.
// Without real codec headers (SPS/PPS), the muxer stays in scanning phase.
// With a title that has no video streams (audio-only), codec scanning is
// skipped and the muxer enters streaming mode immediately, producing EBML output.
// Without real codec headers (SPS/PPS) the muxer would stay in scanning
// phase, but an audio-only title skips codec scanning and streams EBML output immediately.
let dt = DiscTitle {
playlist: "Audio Only".into(),
+3 -8
View File
@@ -352,14 +352,9 @@ fn read_filesystem_with_subdirectory() {
let mut reader = MockSectorReader::new();
let partition_start: u32 = 512;
// Layout (all relative to partition_start which equals metadata_start):
// meta LBA 0 = FSD
// meta LBA 1 = root ICB
// meta LBA 2 = root dir data
// meta LBA 3 = BDMV ICB
// meta LBA 4 = BDMV dir data
// meta LBA 5 = test.mpls file ICB
// meta LBA 10 = test.mpls file data (partition-relative)
// Layout, meta LBAs relative to partition_start (== metadata_start): 0=FSD,
// 1=root ICB, 2=root dir data, 3=BDMV ICB, 4=BDMV dir data, 5=test.mpls
// ICB, 10=test.mpls file data.
reader.set_sector(256, make_avdp_sector(32));
reader.set_sector(32, make_pvd_sector("DISC_WITH_BDMV"));