rusterlium / rustler

Safe Rust bridge for creating Erlang NIF functions
https://docs.rs/crate/rustler
Apache License 2.0
4.31k stars 225 forks source link

`ListIterator` panics while iterating over an improper list #338

Open koba-e964 opened 3 years ago

koba-e964 commented 3 years ago

If ListIterator tries to iterate over an improper list, it panics while retrieving its elements.

Is it possible for ListIterator to fail in a modest way like throwing errors, rather than panic!-ing? Besides, because the functions in rustler::types::list are hidden, there are no documented ways to decouple improper lists (e.g. rustler::types::lists::list_get_cell). It would be very helpful if they were documented.

Here is the output:

$ iex -S mix
Erlang/OTP 23 [erts-11.1.1] [source] [64-bit] [smp:4:4] [ds:4:4:10] [async-threads:1] [hipe] [dtrace]

Compiling NIF crate :example (native/example)...
   Compiling example v0.1.0 (/Users/koba_mac/srcview/phx_rust_21/native/example)
warning: unused variable: `list`
  --> src/lib.rs:31:9
   |
31 |     let list: Vec<Term<'a>> = args[0].decode()?;
   |         ^^^^ help: if this is intentional, prefix it with an underscore: `_list`
   |
   = note: `#[warn(unused_variables)]` on by default

warning: 1 warning emitted

    Finished dev [unoptimized + debuginfo] target(s) in 4.98s
Interactive Elixir (1.11.2) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> NifExample.inplace_sort([1 | 2]) 
thread '<unnamed>' panicked at 'list iterator found improper list', /Users/koba_mac/.cargo/registry/src/github.com-1ecc6299db9ec823/rustler-0.21.1/src/types/list.rs:73:21
             note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
                                                                                          ** (ErlangError) Erlang error: :nif_panic
    (phx_rust_21 0.1.0) NifExample.inplace_sort([1 | 2])
iex(1)> 

(Code used to reproduce this panic!-ing: https://github.com/koba-e964/phx_rust_21/compare/bug/improper-lists)

hansihe commented 3 years ago

This isn't really a bug, this is intended behavior. We could add another iterator type which returns an enum for either an element or an improper tail.

filmor commented 3 years ago

Also, the direct functions are internal only, but Term has a list_get_cell method that you can use instead. With this, you should be able to build your own iterator.

This is documented: https://docs.rs/rustler/0.22.0-rc.0/rustler/struct.Term.html#method.list_get_cell

Last edit: We could probably add a line to the docs of ListIterator that it can only be used with proper lists, and refer to Term::list_get_cell for improper lists.

koba-e964 commented 3 years ago

Also, the direct functions are internal only, but Term has a list_get_cell method that you can use instead. With this, you should be able to build your own iterator. This is documented: https://docs.rs/rustler/0.22.0-rc.0/rustler/struct.Term.html#method.list_get_cell

Uh, I overlooked that. Thank you for pointing this out.

Last edit: We could probably add a line to the docs of ListIterator that it can only be used with proper lists, and refer to Term::list_get_cell for improper lists.

It would be great if we had these explanations in the doc.