From 1948268a09e133b69f0ca12123e564466f9eccfb Mon Sep 17 00:00:00 2001 From: jrgutier Date: Fri, 31 Jul 2026 14:38:58 -0500 Subject: [PATCH] Refuse a torn export.pdb instead of converting it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A conversion published two playlist entries that the settled export.pdb does not contain — tracks 444 and 2984, spliced into "Organic House - Top 100" and "walker-royce". convert exited 0; only a later verify found them. What the drive shows -------------------- rekordbox last wrote export.pdb at 12:09:38 UTC on 29 Jul. convert started 14 seconds later at 12:09:52 and finished 12:12:21; verify ran at 12:12:24. The pdb has not been modified since 12:09:38, so convert and verify read the same file and the source did not change between them — which rules out the stale-source explanation this was previously attributed to. Parsed today, that unchanged file yields exactly what verify expected: 65 entries in "Organic House - Top 100", and tracks 444 and 2984 in no playlist at all. The parse is deterministic. So the read at 12:09:52 saw a page image that no longer exists, 14 seconds after rekordbox's last recorded write to removable media where mtime reflects a metadata flush rather than the final byte landing. A stale slot read as present is the mechanism: it parses cleanly, lands in a coherent chain, and downstream nothing can tell it from a real entry. The header already knew ----------------------- Each page declares num_rows alongside num_row_offsets, and the walker decoded that field and discarded it. It is now enforced against the count of present bits, so a page offering more rows than it claims to hold is refused rather than converted. Row offsets are also bounded to the heap between the page header and the backward-growing index, instead of merely to the page. Both invariants were validated against a real 3,673-track export before being enforced: 997/997 data pages agree, every present row body sits inside the heap, and the 104 tombstoned slots in that file still parse as the deletions they are. The same file parses byte-identically with the gate active — 3,673 tracks, 45 playlists, 3,872 entries. Coverage is honest, not total: a torn image whose header was written before the pages it describes satisfies both checks and still passes. That residue is why the plan also carries an independent pre-publish oracle. 699 tests (3 new), 88% branch coverage, ruff + mypy clean. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01V3aF4y4GdsuJta2bfYUZ8w --- CHANGELOG.md | 19 ++++++ src/rb2engine/reader/pdb.py | 69 +++++++++++++++---- tests/unit/test_pdb.py | 132 ++++++++++++++++++++++++++++++++++++ 3 files changed, 208 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2565307..1ed8762 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Fixed +- **G1d — refuse a torn `export.pdb` instead of converting it.** A conversion on + a real stick published two playlist entries that the settled `export.pdb` does + not contain. Forensics on the drive established what happened: rekordbox last + wrote the pdb at 12:09:38 UTC, `convert` started 14 seconds later at 12:09:52, + and the file has not been modified since — so `convert` and the `verify` that + caught it read the *same* file, and the difference arose while reading a pdb + that was still settling. A stale slot read as present parses cleanly, lands in + a coherent chain, and is indistinguishable downstream from a real entry. + + The page header already carries the contradiction: it declares `num_rows`, and + the reader decoded that field and threw it away. The parser now checks that the + present-bit count matches `num_rows`, and that every row offset points into the + heap between the page header and the backward-growing row index. Both invariants + hold on all 997 data pages of a real 3,673-track export, and ordinary deleted + slots — 104 of them in that same file — still parse normally. + + Scope, stated plainly: this catches the demonstrated signature class. A torn + image whose header was written before the pages it describes satisfies both + checks and would still pass. - `verify` paired each source playlist with the wrong Engine list in three ways, every one of which invents discrepancies on a correct conversion — the failure mode that trains you to ignore verify. Same-named playlists in different diff --git a/src/rb2engine/reader/pdb.py b/src/rb2engine/reader/pdb.py index 3cf774d..9862871 100644 --- a/src/rb2engine/reader/pdb.py +++ b/src/rb2engine/reader/pdb.py @@ -213,18 +213,50 @@ def _is_data_page(page_flags: int) -> bool: return (page_flags & 0x40) == 0 +# num_rows is 11 bits; a page holding more present rows than that cannot state +# its own count, so the cross-check below is skipped rather than mis-fired. +_NUM_ROWS_MAX = 0x7FF + + def _iter_present_row_bases( - page: bytes, len_page: int, num_row_offsets: int + page: bytes, + len_page: int, + num_row_offsets: int, + num_rows: int = -1, + page_index: int = -1, ) -> list[int]: - """Absolute offsets within `page` of each *present* row body.""" + """Absolute offsets within `page` of each *present* row body. + + G1d — torn/mid-export page images. The row index grows backward from the + end of the page, so a row body lives strictly between the page header and + the index. Two structural facts are checked here rather than assumed: + + * every present offset points into that heap region, and + * the number of present bits equals the ``num_rows`` the header declares. + + Both hold on all 997 data pages of a real 3,673-track export. The second + exists because a conversion once published playlist entries the settled + ``export.pdb`` does not contain: a stale slot read as present parses + cleanly, lands in a coherent chain, and is indistinguishable downstream + from a real entry. The page header already carries the count that + contradicts it; the walker simply discarded it. + """ if num_row_offsets <= 0: return [] num_groups = (num_row_offsets - 1) // 16 + 1 + index_bytes = num_groups * ROW_GROUP_SIZE + heap_end = len_page - index_bytes + if heap_end <= PAGE_HEADER_SIZE: + raise UnsupportedFormatError( + f"export.pdb page {page_index}: num_row_offsets={num_row_offsets} " + f"needs {index_bytes} bytes of row index, which does not fit in a " + f"{len_page}-byte page" + ) + bases: list[int] = [] + present_count = 0 for g in range(num_groups): group_base = len_page - (g * ROW_GROUP_SIZE) - if group_base - 4 < PAGE_HEADER_SIZE: - break present_flags = struct.unpack_from("> r) & 1 == 0: continue # deleted / absent — honour the bitmask - ofs_pos = group_base - (6 + 2 * r) - if ofs_pos < 0: - continue - ofs_row = struct.unpack_from(" None: f"\nreal_stick: tracks={len(lib.tracks)} playlists={len(lib.playlists)} " f"elapsed={elapsed:.3f}s sample={sample.title!r} path={sample.resolved_path}" ) + + +# --------------------------------------------------------------------------- +# G1d — torn/mid-export page images +# +# A real conversion published two playlist entries that the settled export.pdb +# does not contain (tracks 444 and 2984, 2026-07-29). The file was unchanged +# between that convert and the verify that caught it, so the difference arose +# at read time: convert started 14 s after rekordbox's last write to the pdb. +# A page whose present-bit set disagrees with its declared num_rows is the +# signature that resurrects a stale row, and the reader decoded num_rows and +# then discarded it. These tests pin the gate. +# --------------------------------------------------------------------------- + + +def _resurrect_slot( + page: bytes, len_page: int, slot: int, heap_offset: int +) -> bytes: + """Set slot's present bit and point it at a row body, leaving num_rows alone. + + This is what a torn page image looks like to the walker: one more row + reachable than the page header says it holds. + """ + buf = bytearray(page) + g, r = divmod(slot, 16) + group_base = len_page - (g * 0x24) + present = struct.unpack_from(" Path: + pages = { + 0: _file_header( + len_page=len_page, + tables=[(0, 1, 1), (7, 2, 2), (8, 3, 3)], + ), + 1: _build_nondata_page( + len_page=len_page, page_index=1, page_type=0, next_page=99 + ), + 2: _build_data_page( + len_page=len_page, + page_index=2, + page_type=7, + next_page=99, + row_blobs=[ + _playlist_tree_row( + parent_id=0, sort_order=0, pl_id=1, is_folder=False, name="Set A" + ) + ], + ), + 3: entry_page, + } + path = tmp_path / "torn.pdb" + _write_pdb(path, len_page, pages) + return path + + +def test_g1d_resurrected_row_beyond_num_rows_raises(tmp_path: Path) -> None: + """A present bit the page header does not account for must be refused. + + WHY: this is the observed failure. The stale entry parses cleanly and lands + in a coherent chain, so nothing downstream can tell it from a real one — + the conversion exits 0 and publishes a library with a track the DJ removed. + Refusing the parse is the only point where the two disagree. + """ + len_page = 512 + rows: list[bytes | None] = [ + _playlist_entry_row(0, track_id=10, playlist_id=1), + None, # deleted slot — its bytes are still in the heap + _playlist_entry_row(1, track_id=20, playlist_id=1), + ] + page = _build_data_page( + len_page=len_page, page_index=3, page_type=8, next_page=99, row_blobs=rows + ) + # Resurrect slot 1, pointing it at the first row body. num_rows still says 2. + torn = _resurrect_slot(page, len_page, slot=1, heap_offset=0) + path = _torn_pdb(tmp_path, torn, len_page) + + with pytest.raises(UnsupportedFormatError) as exc: + parse_export_pdb(path, tmp_path) + assert "num_rows" in str(exc.value) or "present" in str(exc.value) + + +def test_g1d_row_offset_inside_row_index_raises(tmp_path: Path) -> None: + """A row offset pointing into the backward-growing index is not a row. + + WHY: across 997 pages of a real 3,673-track export every present row body + sits between the page header and the index; nothing legitimate points into + the index area. The reader only bounded offsets by the page size, so stale + index bytes could be parsed as a row. + """ + len_page = 512 + rows: list[bytes | None] = [ + _playlist_entry_row(0, track_id=10, playlist_id=1), + _playlist_entry_row(1, track_id=20, playlist_id=1), + ] + page = _build_data_page( + len_page=len_page, page_index=3, page_type=8, next_page=99, row_blobs=rows + ) + buf = bytearray(page) + # Point slot 0 into the row-index region (heap-relative → absolute ≥ heap end). + struct.pack_into(" None: + """The gate must not fire on ordinary deleted slots. + + WHY: tombstones are normal — a real export carried 104 of them. A gate that + rejected those would refuse every library the tool exists to convert. + """ + len_page = 512 + rows: list[bytes | None] = [ + _playlist_entry_row(0, track_id=10, playlist_id=1), + None, + _playlist_entry_row(1, track_id=20, playlist_id=1), + ] + page = _build_data_page( + len_page=len_page, page_index=3, page_type=8, next_page=99, row_blobs=rows + ) + path = _torn_pdb(tmp_path, page, len_page) + + lib = parse_export_pdb(path, tmp_path) + + assert lib.playlists[0].track_rb_ids == [10, 20]