erlang / otp

Erlang/OTP
http://erlang.org
Apache License 2.0
11.35k stars 2.95k forks source link

ERL-1257: Types of crypto.erl #4085

Closed OTP-Maintainer closed 3 years ago

OTP-Maintainer commented 4 years ago

Original reporter: kostis Affected version: OTP-23.0 Fixed in version: OTP-23.1 Component: crypto Migrated from: https://bugs.erlang.org/browse/ERL-1257


Some spec declarations in the {{crypto}} module show serious confusion.  For example, the code of "The New API" reads as follows:
{code:erlang}
-spec crypto_init(Cipher, Key, FlagOrOptions) -> State | descriptive_error()
                                                   when Cipher :: cipher_no_iv(),
                                                        Key :: iodata(),
                                                        FlagOrOptions :: crypto_opts() | boolean(),
                                                        State :: crypto_state() .
crypto_init(Cipher, Key, FlagOrOptions) ->
    crypto_init(Cipher, Key, <<>>, FlagOrOptions).

-spec crypto_init(Cipher, Key, IV, FlagOrOptions) -> State | descriptive_error()
                                                       when Cipher :: cipher_iv(),
 ...
{code}
[Apologies for the ugly format, but it's the one used in the file.]

It cannot possibly be that {{Cipher}} is declared as {{cipher_no_iv()}} in the first function which then calls the second function which expects a {{cipher_iv()}} where these two types have nothing in common.

The same problem exists in the specs of {{crypto_one_time}}.

Also, in various places in the specs, {{FlagOrOptions}} reads {{crypto_opts() | boolean()}}, i.e., as above, but then again the code defines:
{code:erlang}
-type crypto_opts() :: boolean() 
                     | [ crypto_opt() ] .
{code}
Obviously, {{boolean()}} should appear in either the type declaration or in the {{|}}, not in both.

Finally, I recommend taking out {{| descriptive_error()}} (an alias for {{no_return()}}) from the return type of these functions.  They will never really return this.
OTP-Maintainer commented 4 years ago

hans said:

The no_return() is possible, it can be thrown from the nif C-function:
{code:java}
1> crypto:crypto_init(aes_128_ctr, <<>>, <<>>, true).
** exception error: {badarg,{"api_ng.c",143},"Bad key size"}
     in function  crypto:ng_crypto_init_nif/5
        called as crypto:ng_crypto_init_nif(aes_128_ctr,<<>>,<<>>,true,
                                            undefined)
2> 
{code}
OTP-Maintainer commented 4 years ago

hans said:

Thanks for the report!  Will be released in OTP-23.1
OTP-Maintainer commented 4 years ago

kostis said:

`no_return()` does not mean what you think it means.

`no_return()` means that the function does not return any value ever, not that it throws some exception.

For example, the `lists:nth/2` function, or for that matter pretty much any function in Erlang, also throws an exception when called with some combination of arguments that the function does not handle:
{code:java}
1> lists:nth(2, []).
** exception error: no function clause matching lists:nth(2,[]) (lists.erl, line 170)
{code}
but it's (correctly) spec-ed as returning `term()`, not `term() | no_return()`.