Wilfred / difftastic

a structural diff that understands syntax 🟥🟩
https://difftastic.wilfred.me.uk/
MIT License
21.18k stars 347 forks source link

Panic in line-numbers-0.3.0/src/lib.rs:105:13 #755

Closed wsy2220 closed 1 month ago

wsy2220 commented 3 months ago

Thanks for reporting a bug! Please include all of the following:

(1) A description of the issue. A screenshot is often helpful too.

$ RUST_BACKTRACE=full ./difft common.c common1.c 
thread 'main' panicked at 'Offset 17795 is out of bounds for a string of length 17794', /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/line-numbers-0.3.0/src/lib.rs:105:13
stack backtrace:
   0:     0x564fbd6aa925 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hb070b7fa7e3175df
   1:     0x564fbd4fa73e - core::fmt::write::hd5207aebbb9a86e9
   2:     0x564fbd6a6e75 - std::io::Write::write_fmt::h3bd699bbd129ab8a
   3:     0x564fbd6ac093 - std::panicking::default_hook::{{closure}}::h04cca40023d0eeca
   4:     0x564fbd6abd9f - std::panicking::default_hook::haa3ca8c310ed5402
   5:     0x564fbd6ac6bc - std::panicking::rust_panic_with_hook::h7b190ce1a948faac
   6:     0x564fbd6ac5c4 - std::panicking::begin_panic_handler::{{closure}}::hbafbfdc3e1b97f68
   7:     0x564fbd6aae2c - std::sys_common::backtrace::__rust_end_short_backtrace::hda93e5fef243b4c0
   8:     0x564fbd6ac312 - rust_begin_unwind
   9:     0x564fbd459793 - core::panicking::panic_fmt::h8d17ca1073d9a733
  10:     0x564fbd5cf5e9 - line_numbers::LinePositions::from_offset::h4bfaf5a83309e1ea
  11:     0x564fbd5cf634 - line_numbers::LinePositions::from_region::h687d9655a972be1f
  12:     0x564fbd56c184 - difft::line_parser::change_positions::hf7fcd5ecd86f352c
  13:     0x564fbd51b1e9 - difft::diff_file_content::h5249a5e92709a1e6
  14:     0x564fbd5195f9 - difft::diff_file::h965d9d057168ed30
  15:     0x564fbd518776 - difft::main::h6891f453f04e1683
  16:     0x564fbd51d253 - std::sys_common::backtrace::__rust_begin_short_backtrace::h3db357fe9e74446d
  17:     0x564fbd5773d9 - std::rt::lang_start::{{closure}}::hbc9d45e097bf4a77
  18:     0x564fbd6a0b7a - std::rt::lang_start_internal::h6ba1bb743c1e9df9
  19:     0x564fbd51d118 - main
  20:     0x7f1e3bf5dc8a - __libc_start_call_main
                               at ./csu/../sysdeps/nptl/libc_start_call_main.h:58:16
  21:     0x7f1e3bf5dd45 - __libc_start_main_impl
                               at ./csu/../csu/libc-start.c:360:3
  22:     0x564fbd475d1e - _start
  23:                0x0 - <unknown>

(2) A copy of what you're diffing. If you're diffing files, include the before and after files. If you're using difftastic with a VCS repository (e.g. git), include the URL and commit hash. panic.zip

(3) The version of difftastic you're using (see difft --version) and your operating system.

Difftastic 0.60.0 (1ac9553 2024-07-30, built with rustc 1.65.0)
ousbots commented 2 months ago

I ran into this issue as well, here is a simplified test case that can reproducibly toggle this bug with a line break.

> echo -en "abc" > test1.txt
> echo -en "abc\nabc" > test2.txt
> difft test1.txt test2.txt
thread 'main' panicked at /private/tmp/nix-build-difftastic-0.60.0.drv-0/cargo-vendor-dir/line-numbers-0.3.0/src/lib.rs:105:13:
Offset 8 is out of bounds for a string of length 7

> echo -en "\n" >> test2.txt
> difft test1.txt test2.txt
test2.txt --- Text
1 1 abc
. 2 abc
. 3
> difft --version
Difftastic 0.60.0 (built with rustc 1.80.1)
Wilfred commented 2 months ago

Thanks for the report, and especially for the small repro. The problem is here:

https://github.com/Wilfred/difftastic/blob/3c62ff37c04b1f8a6c625a0c8ac11a04d284d939/src/line_parser.rs#L113-L123

myers_diff::slice_unique_by_hash is free to return a different value if it's "equal", which is broken by the equality on StringIgnoringNewline, so we end up thinking that both lines are abc\n and try to index assuming the second line is also abc\n.

This is an annoying problem in general, where we want foo to be different to foo\n, but foo and foo\nbar should consider the first line to be the same.

Wilfred commented 1 month ago

I think the correct fix here is to unconditionally split on newlines, discarding them, but highlight whether or not the last line has a trailing newline (in text mode), following git's diff style.

cthompson-avb commented 1 month ago

I'm having a similar crash when diffing a YML file, using difft as the git difftool.

Version: Difftastic 0.60.0 (built with rustc 1.79.0) from homebrew

thread 'main' panicked at /Users/brew/Library/Caches/Homebrew/cargo_cache/registry/src/index.crates.io-6f17d22bba15001f/line-numbers-0.3.0/src/lib.rs:105:13:
Offset 17716 is out of bounds for a string of length 17715
stack backtrace:
   0: _rust_begin_unwind
   1: core::panicking::panic_fmt
   2: line_numbers::LinePositions::from_offset
   3: line_numbers::LinePositions::from_region
   4: difft::line_parser::change_positions
   5: difft::diff_file_content
   6: difft::diff_file
   7: difft::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
fatal: external diff died, stopping at schemas/api.yml