Incorrect error message from binary_to_term/2 with safe

[nathanl]

Describe the bug
If binary_to_term/2 is called with the safe option and the binary contains a non-existent atom, the error message indicates that the safe option itself is invalid.

To Reproduce

binary_to_term(<<131,100,0,10,97,95,110,101,119,95,97,116,111,109>>, [safe]).

The bad argument error says argument 2: invalid option in list. This is not correct; safe is a valid option.

If we then call the function without safe, it will allocate the atom a_new_atom.

binary_to_term(<<131,100,0,10,97,95,110,101,119,95,97,116,111,109>>, []).
a_new_atom

Now that the atom has been allocated, the safe option works just fine.

binary_to_term(<<131,100,0,10,97,95,110,101,119,95,97,116,111,109>>, [safe]).
a_new_atom

Expected behavior

The error message should indicate that the binary is unsafe.

Affected versions

This bug is present on master as of the latest commit, b2f0302 on 2021-07-23.

[vkatsuba]

Not sure but looks like this is expected behavior, please see #binary_to_term-2:

1 > binary_to_term(<<131,100,0,10,97,95,110,101,119,95,97,116,111,109>>, [safe]). 
** exception error: bad argument
...
1> a_new_atom.
a_new_atom
2> binary_to_term(<<131,100,0,10,97,95,110,101,119,95,97,116,111,109>>, [safe]). 
a_new_atom

(C):

Use this option when receiving binaries from an untrusted source.

When enabled, it prevents decoding data that can be used to attack the Erlang runtime. In the event of receiving unsafe data, decoding fails with a badarg error.
....

[nathanl]

I agree that failing with a badarg error is expected. That's not the bug.

The bug is that the badarg error indicates the wrong argument as being bad. It says that [safe] is an invalid option, which is not true.

What it should say is that <<131,100,0,10,97,95,110,101,119,95,97,116,111,109>>, which would cause a new atom to be allocated, is a bad argument.

[vkatsuba]

Then this sounds like improvement, not a bug. If yes, maybe make sense to change the label.

[nathanl]

If the error message were unclear, like "one of the arguments is bad", clarifying it would be an improvement. But it clearly indicates the wrong information - it says the second argument is bad when in fact the first one is bad. I would call that a bug.

But since this does not actually cause incorrect execution, but only makes debugging difficult, maybe you wouldn't call it a bug. It's definitely a small bug, at most.

I'm happy to leave classification to the maintainers.

[bjorng]

I consider it a bug.