From 1b9b893169c6cfe7353c9bb5d6620c02cec43206 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sun, 6 Oct 2024 00:22:15 +0200 Subject: [PATCH] After reading corrupted history entry, keep reading older entries Given a history like - cmd: echo OLD when: 1726157160 \x00\x00\x00- cmd: echo leading NUL bytes when: 1726157160 - cmd: echo NEW when: 1726157223 offset_of_next_item() happily records 3 items even though the second item is corrupted. decode_item() fails which makes the caller stop loading any older items -- we got knee capped. Avoid this horrible failure mode by skipping over these items already in offset computation. For now we still lose the corrupted item itself. In future we should probably try to delete the NUL bytes or avoid the corruption in the first place. See #10300 and others. --- src/history/file.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/history/file.rs b/src/history/file.rs index 40f326080..3240dd3c5 100644 --- a/src/history/file.rs +++ b/src/history/file.rs @@ -358,9 +358,7 @@ fn decode_item_fish_2_0(mut data: &[u8]) -> Option { let (advance, line) = read_line(data); let line = trim_start(line); // Check this early *before* anything else. - if !line.starts_with(b"- cmd") { - return None; - } + assert!(line.starts_with(b"- cmd")); let (_key, value) = extract_prefix_and_unescape_yaml(line)?; @@ -496,6 +494,15 @@ fn offset_of_next_item_fish_2_0( continue; } + if !line.starts_with(b"- cmd") { + FLOG!( + history, + "ignoring corrupted history entry around offset", + *cursor + ); + continue; + } + // At this point, we know `line` is at the beginning of an item. But maybe we want to // skip this item because of timestamps. A `None` cutoff means we don't care; if we do care, // then try parsing out a timestamp.