freestrings / jsonpath

JsonPath engine written in Rust. Webassembly and Javascript support too
MIT License
122 stars 37 forks source link

panic: index out of bounds in `SelectorMut::replace_with` #86

Open martinohmann opened 2 years ago

martinohmann commented 2 years ago

Hi,

I discovered the following panic while using SelectorMut::remove:

thread 'main' panicked at 'index out of bounds: the len is 1 but the index is 2', /home/mohmann/.cargo/registry/src/github.com-1ecc6299db9ec823/jsonpath_lib-0.3.0/src/select/mod.rs:814:56
stack backtrace:
   0: rust_begin_unwind
             at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35/library/std/src/panicking.rs:517:5
   1: core::panicking::panic_fmt
             at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35/library/core/src/panicking.rs:101:14
   2: core::panicking::panic_bounds_check
             at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35/library/core/src/panicking.rs:77:5
   3: <usize as core::slice::index::SliceIndex<[T]>>::index_mut
             at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35/library/core/src/slice/index.rs:190:14
   4: core::slice::index::<impl core::ops::index::IndexMut<I> for [T]>::index_mut
             at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35/library/core/src/slice/index.rs:26:9
   5: <alloc::vec::Vec<T,A> as core::ops::index::IndexMut<I>>::index_mut
             at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35/library/alloc/src/vec/mod.rs:2477:9
   6: jsonpath_lib::select::replace_value
             at /home/mohmann/.cargo/registry/src/github.com-1ecc6299db9ec823/jsonpath_lib-0.3.0/src/select/mod.rs:814:56
   7: jsonpath_lib::select::SelectorMut::replace_with
             at /home/mohmann/.cargo/registry/src/github.com-1ecc6299db9ec823/jsonpath_lib-0.3.0/src/select/mod.rs:958:17
   8: jsonpath_lib::select::SelectorMut::remove
             at /home/mohmann/.cargo/registry/src/github.com-1ecc6299db9ec823/jsonpath_lib-0.3.0/src/select/mod.rs:929:9
   9: jsonlib_crash::main
             at ./src/main.rs:8:19
  10: core::ops::function::FnOnce::call_once
             at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

The jsonpath_lib version is 0.3.0. Here is a minimal reproducer:

use jsonpath_lib::SelectorMut;
use serde_json::json;
use std::error::Error;

fn main() -> Result<(), Box<dyn Error>> {
    let value = json!([1, 2, 3]);

    let mutated = SelectorMut::new()
        .str_path("$[*]")?
        .value(value)
        .remove()?
        .take();

    println!("{:?}", mutated);

    Ok(())
}

I expected mutated to contain an empty array, but got this panic instead. Please note that when using .delete() instead of .remove() this code does not panic but produces the expected result of an array which contains only nulls.

I suspect that the array length isn't recalculated correctly after removing elements.

martinohmann commented 2 years ago

@freestrings I saw that this was already fixed in https://github.com/freestrings/jsonpath/commit/c3b973437cddbd05fc41796e7a2ddb7b998150a2, sorry for the duplication.

Is it possible to publish a patch release for this?

freestrings commented 2 years ago

@martinohmann The next release is being delayed. I will do a minor release first as soon as possible. Thanks for reporting the issue.

stefanpieck commented 2 years ago

I ran into the same issue. After seeing the last comment I tried out the current master version (https://github.com/freestrings/jsonpath#1a84c5af9445e617a6deed14dadfaee13af0d95f). There the panic did not occur any more, but when deleting values in the replace_with() method, values which are located directly after a deleted value are ignored.

here a test to reproduce:

#[test]
fn delete_in_replace() {
    let input = json!({
        "myList": [
            "BLUE",
            "ORANGE",
            "GREEN",
            "RED"
        ]
    });

    let output = jsonpath_lib::replace_with(input, "$.myList[*]", &mut |_| None)
        .expect("replace to succeed");

    assert_eq!(json!({"myList": []}), output);
}

which results in:

thread 'tests::delete_in_replace' panicked at 'assertion failed: `(left == right)`
  left: `Object({"myList": Array([])})`,
 right: `Object({"myList": Array([String("ORANGE"), String("RED")])})`'