cantino / mcfly

Fly through your shell history. Great Scott!
MIT License
6.9k stars 178 forks source link

Crashes when non-unicode characters are present in `.bash_history` #14

Closed theotherjimmy closed 5 years ago

theotherjimmy commented 5 years ago

I happened to have created a directory a while back that is invalid in any encoding scheme available in python, for testing. The mkdir and cd are then in my bash history file, and contain invalid unicode. I was able to reduce the reproducing example down to:

$ cat ~/.bash_history
cd �abc\\uXXXX/
$ xxd ~/.bash_history
00000000: 6364 2080 6162 635c 5c75 5858 5858 2f0a  cd .abc\\uXXXX/.

which causes the following crash:

thread 'main' panicked at '"/tmp/mcfly.HE5x" file not found: Custom { kind: InvalidData, error: StringError("stream did not contain valid UTF-8") }', libcore/result.rs:1009:5
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::panicking::default_hook::{{closure}}
             at libstd/sys_common/backtrace.rs:71
             at libstd/sys_common/backtrace.rs:59
             at libstd/panicking.rs:211
   2: std::panicking::rust_panic_with_hook
             at libstd/panicking.rs:227
             at libstd/panicking.rs:476
   3: std::panicking::continue_panic_fmt
             at libstd/panicking.rs:390
   4: rust_begin_unwind
             at libstd/panicking.rs:325
   5: core::panicking::panic_fmt
             at libcore/panicking.rs:77
   6: core::result::unwrap_failed
   7: mcfly::bash_history::full_history
   8: mcfly::bash_history::last_history_line
   9: mcfly::settings::Settings::parse_args
  10: mcfly::main
  11: std::rt::lang_start::{{closure}}
  12: main
  13: __libc_start_main
  14: _start

I tried the following diff, and ran into further errors:

# git diff
diff --git a/src/bash_history.rs b/src/bash_history.rs
index dcbc34c..014deec 100644
--- a/src/bash_history.rs
+++ b/src/bash_history.rs
@@ -12,12 +12,12 @@ pub fn bash_history_file_path() -> PathBuf {
 }

 pub fn full_history(path: &PathBuf) -> Vec<String> {
-    let bash_history_contents =
-        fs::read_to_string(&path).expect(format!("{:?} file not found", &path).as_str());
+    let bash_history_contents = fs::read_to_string(&path).unwrap_or_default();

     let timestamp_regex = Regex::new(r"\A#\d{10}").unwrap();

     bash_history_contents
+        .as_str()
         .split("\n")
         .filter(|line| !timestamp_regex.is_match(line) && !line.is_empty())
         .map(String::from)

After the above diff:

thread 'main' panicked at '"/tmp/mcfly.s88g" file not found: Custom { kind: InvalidData, error: StringError("stream did not contain valid UTF-8") }', libcore/result.rs:1009:5
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::panicking::default_hook::{{closure}}
             at libstd/sys_common/backtrace.rs:71
             at libstd/sys_common/backtrace.rs:59
             at libstd/panicking.rs:211
   2: std::panicking::rust_panic_with_hook
             at libstd/panicking.rs:227
             at libstd/panicking.rs:476
   3: std::panicking::continue_panic_fmt
             at libstd/panicking.rs:390
   4: rust_begin_unwind
             at libstd/panicking.rs:325
   5: core::panicking::panic_fmt
             at libcore/panicking.rs:77
   6: core::result::unwrap_failed
   7: mcfly::settings::Settings::parse_args
   8: mcfly::main
   9: std::rt::lang_start::{{closure}}
  10: main
  11: __libc_start_main
  12: _start
cantino commented 5 years ago

Thanks, I'll look at fixing this.

cantino commented 5 years ago

Mind verifying that 04d35aa747e8e4bd9df9e2b5575cdcaae3dbebdb fixes this for you?

cantino commented 5 years ago

Fix released in https://github.com/cantino/mcfly/releases/tag/v0.2.4

theotherjimmy commented 5 years ago

Confirmed fixed on master.

cantino commented 5 years ago

Thanks!