From 70caf52f1a7e09569c776bb1f99b0bf4737c63c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Thu, 10 Jan 2013 12:15:43 +0100 Subject: Correct error handling for the NIF functions Also make sure that the error handling is contained within the asn1rt_nif module and does not leak out to generated code. --- lib/asn1/c_src/asn1_erl_nif.c | 47 ++++++++++++++++++++++++++++++-------- lib/asn1/src/asn1rt_nif.erl | 25 ++++++++++++++++++-- lib/asn1/src/asn1rtt_ber.erl | 32 +------------------------- lib/asn1/src/asn1rtt_per.erl | 12 +--------- lib/asn1/test/ber_decode_error.erl | 43 ++++++++++++++++++---------------- 5 files changed, 85 insertions(+), 74 deletions(-) diff --git a/lib/asn1/c_src/asn1_erl_nif.c b/lib/asn1/c_src/asn1_erl_nif.c index dbff14f9b3..73323534d2 100644 --- a/lib/asn1/c_src/asn1_erl_nif.c +++ b/lib/asn1/c_src/asn1_erl_nif.c @@ -27,7 +27,6 @@ #define ASN1_OK 0 #define ASN1_ERROR -1 #define ASN1_COMPL_ERROR 1 -#define ASN1_MEMORY_ERROR 0 #define ASN1_DECODE_ERROR 2 #define ASN1_TAG_ERROR -3 #define ASN1_LEN_ERROR -4 @@ -1221,7 +1220,33 @@ static ERL_NIF_TERM encode_per_complete(ErlNifEnv* env, int argc, return enif_make_binary(env, &out_binary); } -static ERL_NIF_TERM decode_ber_tlv(ErlNifEnv* env, int argc, +static ERL_NIF_TERM +make_ber_error_term(ErlNifEnv* env, unsigned int return_code, + unsigned int err_pos) +{ + ERL_NIF_TERM reason; + ERL_NIF_TERM t; + + switch (return_code) { + case ASN1_TAG_ERROR: + reason = enif_make_atom(env, "invalid_tag"); + break; + case ASN1_LEN_ERROR: + case ASN1_INDEF_LEN_ERROR: + reason = enif_make_atom(env, "invalid_length"); + break; + case ASN1_VALUE_ERROR: + reason = enif_make_atom(env, "invalid_value"); + break; + default: + reason = enif_make_atom(env, "unknown"); + break; + } + t = enif_make_tuple2(env, reason, enif_make_int(env, err_pos)); + return enif_make_tuple2(env, enif_make_atom(env, "error"), t); +} + +static ERL_NIF_TERM decode_ber_tlv_raw(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]) { ErlNifBinary in_binary; ERL_NIF_TERM return_term; @@ -1230,11 +1255,11 @@ static ERL_NIF_TERM decode_ber_tlv(ErlNifEnv* env, int argc, if (!enif_inspect_iolist_as_binary(env, argv[0], &in_binary)) return enif_make_badarg(env); - if ((return_code = ber_decode_begin(env, &return_term, in_binary.data, - in_binary.size, &err_pos)) != ASN1_OK - ) - return enif_make_tuple2(env, enif_make_atom(env,"error"), enif_make_tuple2(env, - enif_make_int(env, return_code),enif_make_int(env, err_pos))); + return_code = ber_decode_begin(env, &return_term, in_binary.data, + in_binary.size, &err_pos); + if (return_code != ASN1_OK) { + return make_ber_error_term(env, return_code, err_pos); + } return return_term; } @@ -1297,8 +1322,10 @@ static void unload(ErlNifEnv* env, void* priv_data) { } -static ErlNifFunc nif_funcs[] = { { "encode_per_complete", 1, - encode_per_complete }, { "decode_ber_tlv", 1, decode_ber_tlv }, { - "encode_ber_tlv", 1, encode_ber_tlv } }; +static ErlNifFunc nif_funcs[] = { + { "encode_per_complete", 1, encode_per_complete }, + { "decode_ber_tlv_raw", 1, decode_ber_tlv_raw }, + { "encode_ber_tlv", 1, encode_ber_tlv }, +}; ERL_NIF_INIT(asn1rt_nif, nif_funcs, load, NULL, upgrade, unload) diff --git a/lib/asn1/src/asn1rt_nif.erl b/lib/asn1/src/asn1rt_nif.erl index de1fb94816..0b2e5a62a5 100644 --- a/lib/asn1/src/asn1rt_nif.erl +++ b/lib/asn1/src/asn1rt_nif.erl @@ -77,10 +77,31 @@ load_nif() -> Status end. -encode_per_complete(_TagValueList) -> +decode_ber_tlv(Binary) -> + case decode_ber_tlv_raw(Binary) of + {error,Reason} -> + exit({error,{asn1,Reason}}); + Other -> + Other + end. + +encode_per_complete(TagValueList) -> + case encode_per_complete_raw(TagValueList) of + {error,Reason} -> handle_error(Reason, TagValueList); + Other when is_binary(Other) -> Other + end. + +handle_error([], _)-> + exit({error,{asn1,enomem}}); +handle_error($1, L) -> % error in complete in driver + exit({error,{asn1,L}}); +handle_error(ErrL, L) -> + exit({error,{asn1,ErrL,L}}). + +encode_per_complete_raw(_TagValueList) -> erlang:nif_error({nif_not_loaded,module,?MODULE,line,?LINE}). -decode_ber_tlv(_Binary) -> +decode_ber_tlv_raw(_Binary) -> erlang:nif_error({nif_not_loaded,module,?MODULE,line,?LINE}). encode_ber_tlv(_TagValueList) -> diff --git a/lib/asn1/src/asn1rtt_ber.erl b/lib/asn1/src/asn1rtt_ber.erl index b374191f37..f3f6875eb3 100644 --- a/lib/asn1/src/asn1rtt_ber.erl +++ b/lib/asn1/src/asn1rtt_ber.erl @@ -123,43 +123,13 @@ ber_encode(Tlv) -> asn1rt_nif:encode_ber_tlv(Tlv). ber_decode_nif(B) -> - case asn1rt_nif:decode_ber_tlv(B) of - {error, Reason} -> handle_error(Reason, B); - Else -> Else - end. + asn1rt_nif:decode_ber_tlv(B). ber_decode_erlang(B) when is_binary(B) -> decode_primitive(B); ber_decode_erlang(Tlv) -> {Tlv,<<>>}. -handle_error([],_)-> - exit({error,{asn1,{"memory allocation problem"}}}); -handle_error({$1,_},L) -> % error in nif - exit({error,{asn1,L}}); -handle_error({$2,T},L) -> % error in nif due to wrong tag - exit({error,{asn1,{"bad tag after byte:",error_pos(T),L}}}); -handle_error({$3,T},L) -> % error in driver due to length error - exit({error,{asn1,{"bad length field after byte:", - error_pos(T),L}}}); -handle_error({$4,T},L) -> % error in driver due to indefinite length error - exit({error,{asn1, - {"indefinite length without end bytes after byte:", - error_pos(T),L}}}); -handle_error({$5,T},L) -> % error in driver due to indefinite length error - exit({error,{asn1,{"bad encoded value after byte:", - error_pos(T),L}}}); -handle_error(ErrL,L) -> - exit({error,{asn1,ErrL,L}}). - -error_pos([]) -> - "unknown position"; -error_pos([B])-> - B; -error_pos([B|Bs]) -> - BS = 8 * length(Bs), - B bsl BS + error_pos(Bs). - decode_primitive(Bin) -> {Form,TagNo,V,Rest} = decode_tag_and_length(Bin), case Form of diff --git a/lib/asn1/src/asn1rtt_per.erl b/lib/asn1/src/asn1rtt_per.erl index d545c8a854..8234121934 100644 --- a/lib/asn1/src/asn1rtt_per.erl +++ b/lib/asn1/src/asn1rtt_per.erl @@ -1311,17 +1311,7 @@ get_constraint(C,Key) -> %% complete(L) -> - case asn1rt_nif:encode_per_complete(L) of - {error, Reason} -> handle_error(Reason, L); - Else when is_binary(Else) -> Else - end. - -handle_error([],_)-> - exit({error,{asn1,{"memory allocation problem in driver"}}}); -handle_error($1,L) -> % error in complete in driver - exit({error,{asn1,L}}); -handle_error(ErrL,L) -> - exit({error,{asn1,ErrL,L}}). + asn1rt_nif:encode_per_complete(L). octets_to_complete(Len,Val) when Len < 256 -> [20,Len,Val]; diff --git a/lib/asn1/test/ber_decode_error.erl b/lib/asn1/test/ber_decode_error.erl index ff6e386a88..af0105bf26 100644 --- a/lib/asn1/test/ber_decode_error.erl +++ b/lib/asn1/test/ber_decode_error.erl @@ -21,31 +21,34 @@ -export([run/1]). --include_lib("test_server/include/test_server.hrl"). - run([]) -> - ?line {ok,B} = asn1_wrapper:encode('Constructed','S3',{'S3',17}), - ?line [T,L|V] = lists:flatten(B), - ?line Bytes = [T,L+3|V] ++ [2,1,3], - ?line case asn1_wrapper:decode('Constructed','S3',Bytes) of - {error,{asn1,{unexpected,_}}} -> ok - end, - %% Unexpected bytes must be accepted if there is an extensionmark - ?line {ok,{'S3ext',17}} = asn1_wrapper:decode('Constructed','S3ext',Bytes), - ok; -run([driver]) -> - %% test of OTP-4797, bad indata to driver does not cause an EXIT - ?line {error,_Reason} = asn1rt:decode('Constructed','S3',[3,5]), - ok; -run([nif]) -> - %% test of OTP-4797, bad indata to driver does not cause an EXIT - ?line {error,_Reason} = asn1rt:decode('Constructed','S3',[3,5]), - ok. - + {ok,B} = asn1_wrapper:encode('Constructed','S3',{'S3',17}), + [T,L|V] = lists:flatten(B), + Bytes = [T,L+3|V] ++ [2,1,3], + case asn1_wrapper:decode('Constructed','S3',Bytes) of + {error,{asn1,{unexpected,_}}} -> ok + end, + %% Unexpected bytes must be accepted if there is an extensionmark + {ok,{'S3ext',17}} = asn1_wrapper:decode('Constructed','S3ext',Bytes), + %% Truncated tag. + {error,{asn1,{invalid_tag,_}}} = + (catch 'Constructed':decode('I', <<31,255,255>>)), + %% Overlong tag. + {error,{asn1,{invalid_tag,_}}} = + (catch 'Constructed':decode('I', <<31,255,255,255,127>>)), + %% Invalid length. + {error,{asn1,{invalid_length,_}}} = + (catch 'Constructed':decode('I', <<8,255>>)), + %% Other errors. + {error,{asn1,{invalid_value,_}}} = + (catch 'Constructed':decode('I', <<>>)), + {error,{asn1,{invalid_value,_}}} = + (catch 'Constructed':decode('I', <<8,7>>)), + ok. -- cgit v1.2.3