Skip to content

Commit 5839942

Browse files
author
Peter J McDade
committed
stacked_table: skip non-segment file names in heads/
Foreign files like AppleDouble `._*` (which appear when a macOS-created archive is extracted on another system) or `.DS_Store` landing in store/extra/heads made every metadata read fail until removed by hand; a non-UTF8 name panicked outright. gc() already tolerates foreign names in the table directory, and SimpleOpHeadsStore::get_op_heads tolerates non-hex UTF-8 names in its own heads directory (though it still errors on non-UTF8 ones). Apply the same idea to get_head_tables(): ignore any entry whose name isn't exactly SEGMENT_FILE_NAME_LENGTH ASCII hex characters, logging each skip at warn level (visible under --debug/JJ_LOG), and never touch the foreign file itself. If every entry in heads/ turns out to be invalid, return the new TableStoreError::NoValidHeads instead of silently manufacturing a fresh empty table, which would otherwise mask lost head metadata. A genuinely empty heads/ (no entries at all) keeps its existing behavior. Fixes #9775 Assisted-by: Claude:claude-fable-5
1 parent 5f56e2c commit 5839942

2 files changed

Lines changed: 125 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,13 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
6262
[#9711](https://github.com/jj-vcs/jj/issues/9711)
6363
[#8884](https://github.com/jj-vcs/jj/issues/8884)
6464

65+
* A foreign file (e.g. `.DS_Store`, or an AppleDouble `._*` file that appears
66+
when a macOS-created archive is extracted on another system) in
67+
`store/extra/heads` is now ignored instead of breaking every command that
68+
reads commit metadata, or panicking on a non-UTF8 file name. Skipped
69+
entries are logged at `warn` level (visible with `--debug` or `JJ_LOG`).
70+
[#9775](https://github.com/jj-vcs/jj/issues/9775)
71+
6572
## [0.44.0] - 2026-08-05
6673

6774
### Release highlights

lib/src/stacked_table.rs

Lines changed: 118 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,14 @@ pub enum TableStoreError {
413413
#[source]
414414
err: io::Error,
415415
},
416+
#[error(
417+
"Heads directory '{dir}' contains no valid head, {invalid_entries} invalid entries were \
418+
skipped"
419+
)]
420+
NoValidHeads {
421+
dir: PathBuf,
422+
invalid_entries: usize,
423+
},
416424
#[error("Failed to lock table store")]
417425
Lock(#[source] FileLockError),
418426
}
@@ -507,14 +515,28 @@ impl TableStore {
507515
}
508516

509517
fn get_head_tables(&self) -> TableStoreResult<Vec<Arc<ReadonlyTable>>> {
518+
let heads_dir = self.dir.join("heads");
510519
let mut tables = vec![];
511-
for head_entry in
512-
std::fs::read_dir(self.dir.join("heads")).map_err(TableStoreError::LoadHeads)?
513-
{
514-
let head_file_name = head_entry.map_err(TableStoreError::LoadHeads)?.file_name();
515-
let table = self.load_table(head_file_name.to_str().unwrap().to_string())?;
520+
let mut invalid_entries = 0;
521+
for head_entry in std::fs::read_dir(&heads_dir).map_err(TableStoreError::LoadHeads)? {
522+
let head_entry = head_entry.map_err(TableStoreError::LoadHeads)?;
523+
let head_file_name = head_entry.file_name();
524+
let Some(name) = head_file_name.to_str().filter(|name| {
525+
name.len() == SEGMENT_FILE_NAME_LENGTH && hex_util::decode_hex(name).is_some()
526+
}) else {
527+
tracing::warn!(?head_file_name, "skipping invalid head file name");
528+
invalid_entries += 1;
529+
continue;
530+
};
531+
let table = self.load_table(name.to_string())?;
516532
tables.push(table);
517533
}
534+
if tables.is_empty() && invalid_entries > 0 {
535+
return Err(TableStoreError::NoValidHeads {
536+
dir: heads_dir,
537+
invalid_entries,
538+
});
539+
}
518540
Ok(tables)
519541
}
520542

@@ -824,4 +846,95 @@ mod tests {
824846
assert_eq!(table.get_value(b"abc"), Some(b"value".as_slice()));
825847
Ok(())
826848
}
849+
850+
#[test]
851+
fn stacked_table_store_ignores_foreign_head_files() -> TestResult {
852+
let temp_dir = new_temp_dir();
853+
let store = TableStore::init(temp_dir.path().to_path_buf(), 3);
854+
let mut mut_table = store.get_head()?.start_mutation();
855+
mut_table.add_entry(b"abc".to_vec(), b"value".to_vec());
856+
store.save_table(mut_table)?;
857+
858+
let heads_dir = temp_dir.path().join("heads");
859+
let ds_store = heads_dir.join(".DS_Store");
860+
let apple_double = heads_dir.join(format!("._{}", "a".repeat(SEGMENT_FILE_NAME_LENGTH)));
861+
// Right length, but not hex: exercises the hex-validity half of the
862+
// predicate (the other junk names here are already rejected on
863+
// length alone).
864+
let right_length_non_hex = heads_dir.join("g".repeat(SEGMENT_FILE_NAME_LENGTH));
865+
std::fs::write(&ds_store, "")?;
866+
std::fs::write(&apple_double, "")?;
867+
std::fs::write(&right_length_non_hex, "")?;
868+
869+
// Load a fresh store so we don't rely on the in-memory head cache.
870+
let store = TableStore::load(temp_dir.path().to_path_buf(), 3);
871+
let table = store.get_head()?;
872+
assert_eq!(table.get_value(b"abc"), Some(b"value".as_slice()));
873+
874+
assert!(ds_store.exists());
875+
assert!(apple_double.exists());
876+
assert!(right_length_non_hex.exists());
877+
Ok(())
878+
}
879+
880+
#[test]
881+
fn stacked_table_store_errors_when_all_heads_are_invalid() -> TestResult {
882+
let temp_dir = new_temp_dir();
883+
let store = TableStore::init(temp_dir.path().to_path_buf(), 3);
884+
let mut mut_table = store.get_head()?.start_mutation();
885+
mut_table.add_entry(b"abc".to_vec(), b"value".to_vec());
886+
let table = store.save_table(mut_table)?;
887+
888+
// Replace the only (valid) head with junk, simulating a heads/
889+
// directory that holds nothing but foreign files.
890+
let heads_dir = temp_dir.path().join("heads");
891+
std::fs::remove_file(heads_dir.join(table.name()))?;
892+
std::fs::write(heads_dir.join(".DS_Store"), "")?;
893+
894+
let store = TableStore::load(temp_dir.path().to_path_buf(), 3);
895+
assert!(store.get_head().is_err());
896+
Ok(())
897+
}
898+
899+
// APFS rejects non-UTF8 file names outright (the `fs::write` below would
900+
// fail with EILSEQ), so this can only run where the filesystem allows
901+
// them.
902+
#[cfg(target_os = "linux")]
903+
#[test]
904+
fn stacked_table_store_ignores_non_utf8_head_file_name() -> TestResult {
905+
use std::ffi::OsString;
906+
use std::os::unix::ffi::OsStringExt as _;
907+
908+
let temp_dir = new_temp_dir();
909+
let store = TableStore::init(temp_dir.path().to_path_buf(), 3);
910+
let mut mut_table = store.get_head()?.start_mutation();
911+
mut_table.add_entry(b"abc".to_vec(), b"value".to_vec());
912+
store.save_table(mut_table)?;
913+
914+
let heads_dir = temp_dir.path().join("heads");
915+
let non_utf8_name = OsString::from_vec(vec![0x66, 0x6f, 0x80]);
916+
std::fs::write(heads_dir.join(&non_utf8_name), "")?;
917+
918+
let store = TableStore::load(temp_dir.path().to_path_buf(), 3);
919+
let table = store.get_head()?;
920+
assert_eq!(table.get_value(b"abc"), Some(b"value".as_slice()));
921+
Ok(())
922+
}
923+
924+
#[test]
925+
fn stacked_table_store_ignores_wrong_length_head_file_name() -> TestResult {
926+
let temp_dir = new_temp_dir();
927+
let store = TableStore::init(temp_dir.path().to_path_buf(), 3);
928+
let mut mut_table = store.get_head()?.start_mutation();
929+
mut_table.add_entry(b"abc".to_vec(), b"value".to_vec());
930+
store.save_table(mut_table)?;
931+
932+
let heads_dir = temp_dir.path().join("heads");
933+
std::fs::write(heads_dir.join("deadbeef"), "")?;
934+
935+
let store = TableStore::load(temp_dir.path().to_path_buf(), 3);
936+
let table = store.get_head()?;
937+
assert_eq!(table.get_value(b"abc"), Some(b"value".as_slice()));
938+
Ok(())
939+
}
827940
}

0 commit comments

Comments
 (0)