From 6103ca3e779463126127987d9a67fb066e14be9b Mon Sep 17 00:00:00 2001 From: luhrhenz Date: Thu, 8 Oct 2026 11:24:19 +0100 Subject: [PATCH 1/3] fix(file_store): prevent infinite loop on zero-width changeset types When a changeset type deserializes from zero bytes (e.g. `()`), `EntryIter::next` never advanced the file cursor, causing `Store::load` to loop forever. Check that the stream position actually moved after a successful deserialize; if it did not, terminate the iterator. Closes #2307 --- crates/file_store/src/entry_iter.rs | 12 ++++++++++- crates/file_store/tests/test_zero_width.rs | 24 ++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 crates/file_store/tests/test_zero_width.rs diff --git a/crates/file_store/src/entry_iter.rs b/crates/file_store/src/entry_iter.rs index 8b284f1814..89fbc77c0f 100644 --- a/crates/file_store/src/entry_iter.rs +++ b/crates/file_store/src/entry_iter.rs @@ -51,7 +51,17 @@ where let pos_before_read = self.db_file.stream_position()?; match bincode_options().deserialize_from(&mut self.db_file) { - Ok(changeset) => Ok(Some(changeset)), + Ok(changeset) => { + let pos_after_read = self.db_file.stream_position()?; + if pos_after_read == pos_before_read { + // The deserialized type consumed zero bytes (e.g. `()`). + // The file offset will never advance, so stop iterating + // to avoid an infinite loop. + self.finished = true; + return Ok(None); + } + Ok(Some(changeset)) + } Err(e) => { self.finished = true; let pos_after_read = self.db_file.stream_position()?; diff --git a/crates/file_store/tests/test_zero_width.rs b/crates/file_store/tests/test_zero_width.rs new file mode 100644 index 0000000000..145a23fcef --- /dev/null +++ b/crates/file_store/tests/test_zero_width.rs @@ -0,0 +1,24 @@ +use bdk_file_store::Store; +use std::time::Duration; + +const MAGIC: &[u8] = b"bdk_test_magic"; + +#[test] +fn load_terminates_on_zero_width_changeset() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("db"); + let mut bytes = MAGIC.to_vec(); + bytes.push(0xff); // any trailing byte + std::fs::write(&path, &bytes).unwrap(); + + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + let result = Store::<()>::load(MAGIC, &path); + let _ = tx.send(result.is_ok()); + }); + // `()` decodes as zero bytes, so the file offset never advances and `load` never reaches EOF. + assert!( + rx.recv_timeout(Duration::from_secs(5)).is_ok(), + "load did not terminate" + ); +} From 49d67669bf8024dbfe38f2093420b8f9fc82678f Mon Sep 17 00:00:00 2001 From: luhrhenz Date: Fri, 9 Oct 2026 14:37:02 +0100 Subject: [PATCH 2/3] docs(file_store): note relationship to the postcard migration in #2258 Clarify that this fix for the zero-width changeset hang is independent of the bincode -> postcard migration tracked in #2258. If #2258 lands first this check becomes moot, but the regression test is still worth keeping. --- crates/file_store/src/entry_iter.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/file_store/src/entry_iter.rs b/crates/file_store/src/entry_iter.rs index 89fbc77c0f..f5f616403f 100644 --- a/crates/file_store/src/entry_iter.rs +++ b/crates/file_store/src/entry_iter.rs @@ -56,7 +56,10 @@ where if pos_after_read == pos_before_read { // The deserialized type consumed zero bytes (e.g. `()`). // The file offset will never advance, so stop iterating - // to avoid an infinite loop. + // to avoid an infinite loop. This is independent of the + // bincode -> postcard migration tracked in #2258; if that + // lands first this check becomes moot, but the regression + // test in test_zero_width.rs is still worth keeping. self.finished = true; return Ok(None); } From 52cb53b05312ccdac2ab6e2a14e787513744aeed Mon Sep 17 00:00:00 2001 From: luhrhenz Date: Sat, 10 Oct 2026 10:33:53 +0100 Subject: [PATCH 3/3] test(file_store): cover an empty zero-width store, and scope the comment `()` decodes at end of file as well as before trailing bytes, so a store holding no entries at all hung on the same offset check. Cover it, and assert the loaded changeset rather than only that `load` returned. Also correct the comment above the check. It claimed the check becomes unnecessary once the postcard migration in #2258 lands. It does not: a collection of zero-sized elements with a corrupt length prefix loops inside the decoder and never reaches this point, and postcard has that same hole, so neither this check nor a byte-counting size limit catches it. Say what the check actually covers instead. --- crates/file_store/src/entry_iter.rs | 13 +++++++++---- crates/file_store/tests/test_zero_width.rs | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/crates/file_store/src/entry_iter.rs b/crates/file_store/src/entry_iter.rs index f5f616403f..ac6a789767 100644 --- a/crates/file_store/src/entry_iter.rs +++ b/crates/file_store/src/entry_iter.rs @@ -56,10 +56,15 @@ where if pos_after_read == pos_before_read { // The deserialized type consumed zero bytes (e.g. `()`). // The file offset will never advance, so stop iterating - // to avoid an infinite loop. This is independent of the - // bincode -> postcard migration tracked in #2258; if that - // lands first this check becomes moot, but the regression - // test in test_zero_width.rs is still worth keeping. + // to avoid an infinite loop. + // + // This only covers a changeset that itself decodes to zero + // bytes. A collection of zero-sized elements with a corrupt + // length prefix (e.g. `Vec<()>`) loops inside the decoder + // and never returns here, so it is not caught by this check + // or by a byte-counting size limit. `postcard` has the same + // hole, so the migration tracked in #2258 does not remove + // the need for this check. self.finished = true; return Ok(None); } diff --git a/crates/file_store/tests/test_zero_width.rs b/crates/file_store/tests/test_zero_width.rs index 145a23fcef..a8e6803a3f 100644 --- a/crates/file_store/tests/test_zero_width.rs +++ b/crates/file_store/tests/test_zero_width.rs @@ -22,3 +22,22 @@ fn load_terminates_on_zero_width_changeset() { "load did not terminate" ); } + +#[test] +fn load_terminates_on_empty_zero_width_store() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("db"); + // No entries at all, just the magic bytes. `()` still decodes at EOF, so this hangs + // without the offset check even though the file is well formed. + std::fs::write(&path, MAGIC).unwrap(); + + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + let _ = tx.send(Store::<()>::load(MAGIC, &path).map(|(_, changeset)| changeset)); + }); + let changeset = rx + .recv_timeout(Duration::from_secs(5)) + .expect("load did not terminate") + .expect("an empty store is not an error"); + assert!(changeset.is_none(), "an empty store has no changeset"); +}