From 37cb3d954a0c5b60336a2b1544d060185800fc82 Mon Sep 17 00:00:00 2001 From: Micael Karlberg Date: Wed, 18 Nov 2020 12:27:43 +0100 Subject: [megaco] Empty statistics descriptor incorrectly not allowed In version 3 of the standard its *allowed* to have an 'empty' statistics descriptor. ABNF for version 3: statisticsDescriptor = StatsToken [LBRKT statisticsParameter *(COMMA statisticsParameter) RBRKT] This differ from version 1 where the ABNF looks like this: statisticsDescriptor = StatsToken LBRKT statisticsParameter *(COMMA statisticsParameter ) RBRKT Note that there is some conflict with, for instance, auditReturnItem, which required "some tweaking". OTP-17012 (ERL-1405) --- lib/megaco/src/text/megaco_text_gen_v3.hrl | 14 +++++++++--- lib/megaco/src/text/megaco_text_parser_v3.hrl | 32 ++++++++++++++++++++++++++- lib/megaco/src/text/megaco_text_parser_v3.yrl | 23 ++++++++++++------- 3 files changed, 57 insertions(+), 12 deletions(-) diff --git a/lib/megaco/src/text/megaco_text_gen_v3.hrl b/lib/megaco/src/text/megaco_text_gen_v3.hrl index e440ec6651..8da58fff9a 100644 --- a/lib/megaco/src/text/megaco_text_gen_v3.hrl +++ b/lib/megaco/src/text/megaco_text_gen_v3.hrl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2005-2019. All Rights Reserved. +%% Copyright Ericsson AB 2005-2020. All Rights Reserved. %% %% Licensed under the Apache License, Version 2.0 (the "License"); %% you may not use this file except in compliance with the License. @@ -1152,7 +1152,9 @@ enc_ammDescriptor({Tag, Desc}, State) -> statisticsDescriptor -> enc_StatisticsDescriptor(Desc, State); _ -> error({invalid_ammDescriptor_tag, Tag}) - end. + end; +enc_ammDescriptor(Tag, State) when is_atom(Tag) -> + enc_ammDescriptor({Tag, []}, State). enc_AmmsReply(#'AmmsReply'{terminationID = TIDs, terminationAudit = asn1_NOVALUE}, State) -> @@ -3208,14 +3210,20 @@ enc_PackagesItem(Val, State) enc_StatisticsDescriptor({'StatisticsDescriptor',Val}, State) -> enc_StatisticsDescriptor(Val, State); -enc_StatisticsDescriptor(List, State) when is_list(List) -> +enc_StatisticsDescriptor(List, State) + when is_list(List) andalso (List =/= []) -> [ ?StatsToken, ?LBRKT_INDENT(State), enc_list([{List, fun enc_StatisticsParameter/2}], ?INC_INDENT(State)), ?RBRKT_INDENT(State) + ]; +enc_StatisticsDescriptor([], State) -> + [ + ?StatsToken ]. + enc_StatisticsParameter(Val, State) when is_record(Val, 'StatisticsParameter') -> PkgdName = ?META_ENC(statistics, Val#'StatisticsParameter'.statName), diff --git a/lib/megaco/src/text/megaco_text_parser_v3.hrl b/lib/megaco/src/text/megaco_text_parser_v3.hrl index 7b8fff2080..9d0afaa11b 100644 --- a/lib/megaco/src/text/megaco_text_parser_v3.hrl +++ b/lib/megaco/src/text/megaco_text_parser_v3.hrl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2005-2019. All Rights Reserved. +%% Copyright Ericsson AB 2005-2020. All Rights Reserved. %% %% Licensed under the Apache License, Version 2.0 (the "License"); %% you may not use this file except in compliance with the License. @@ -1740,6 +1740,36 @@ do_merge_control_streamParms([{SubTag, SD} | T] = All, LCD) -> do_merge_control_streamParms([], LCD) -> LCD. + +%% This is to get around the two defs: +%% +%% statisticsDescriptor = StatsToken [LBRKT statisticsParameter *(COMMA +%% statisticsParameter) RBRKT] +%% +%% and +%% +%% auditReturnParameter = (mediaDescriptor / modemDescriptor / muxDescriptor / +%% eventsDescriptor / signalsDescriptor / +%% digitMapDescriptor / observedEventsDescriptor / +%% eventBufferDescriptor / statisticsDescriptor / +%% packagesDescriptor / errorDescriptor / +%% auditReturnItem) +%% auditReturnItem = (MuxToken / ModemToken / MediaToken / +%% DigitMapToken / StatsToken / ObservedEventsToken / +%% PackagesToken) +%% +%% In both statisticsDescriptor and auditReturnItem the StatsToken +%% can occure on its owm => conflict + +-ifdef(megaco_parser_inline). +-compile({inline,[{ensure_arp_statisticsDescriptor,1}]}). +-endif. +ensure_arp_statisticsDescriptor([]) -> + {auditReturnItem, statsToken}; +ensure_arp_statisticsDescriptor(SPs) -> + {statisticsDescriptor, SPs}. + + -ifdef(megaco_parser_inline). -compile({inline,[{merge_terminationStateDescriptor,1}]}). -endif. diff --git a/lib/megaco/src/text/megaco_text_parser_v3.yrl b/lib/megaco/src/text/megaco_text_parser_v3.yrl index 54739af82e..f2d9d93d39 100644 --- a/lib/megaco/src/text/megaco_text_parser_v3.yrl +++ b/lib/megaco/src/text/megaco_text_parser_v3.yrl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2005-2016. All Rights Reserved. +%% Copyright Ericsson AB 2005-2020. All Rights Reserved. %% %% Licensed under the Apache License, Version 2.0 (the "License"); %% you may not use this file except in compliance with the License. @@ -760,12 +760,13 @@ auditReturnParameter -> signalsDescriptor : {signalsDescriptor, '$1'} . auditReturnParameter -> digitMapDescriptor : {digitMapDescriptor, '$1'} . auditReturnParameter -> observedEventsDescriptor : {observedEventsDescriptor, '$1'} . auditReturnParameter -> eventBufferDescriptor : {eventBufferDescriptor, '$1'} . -auditReturnParameter -> statisticsDescriptor : {statisticsDescriptor, '$1'} . +%% Conflict with 'empty' statisticsDescriptor and auditReturnItem (see below) +auditReturnParameter -> statisticsDescriptor : ensure_arp_statisticsDescriptor('$1') . auditReturnParameter -> packagesDescriptor : {packagesDescriptor, '$1'} . auditReturnParameter -> errorDescriptor : {errorDescriptor, '$1'} . auditReturnParameter -> auditReturnItem : {auditReturnItem, '$1'} . -auditDescriptor -> 'AuditToken' 'LBRKT' auditDescriptorBody 'RBRKT' : +auditDescriptor -> 'AuditToken' 'LBRKT' auditDescriptorBody 'RBRKT' : merge_auditDescriptor('$3') . auditDescriptorBody -> auditItem auditItemList : ['$1' | '$2']. @@ -780,13 +781,16 @@ auditReturnItem -> 'MuxToken' : muxToken . auditReturnItem -> 'ModemToken' : modemToken . auditReturnItem -> 'MediaToken' : mediaToken . auditReturnItem -> 'DigitMapToken' : digitMapToken . -auditReturnItem -> 'StatsToken' : statsToken . +%% Conflict with 'empty' statisticsDescriptor: +%% auditReturnItem -> 'StatsToken' : statsToken . auditReturnItem -> 'ObservedEventsToken' : observedEventsToken . auditReturnItem -> 'PackagesToken' : packagesToken . %% at-most-once, and DigitMapToken and PackagesToken are not allowed %% in AuditCapabilities command auditItem -> auditReturnItem : '$1' . +%% Moved from ari above (conflict with 'empty' statisticsDescriptor) +auditItem -> 'StatsToken' : statsToken. auditItem -> 'SignalsToken' : signalsToken. auditItem -> 'EventBufferToken' : eventBufferToken. auditItem -> 'EventsToken' : eventsToken . @@ -1095,11 +1099,13 @@ localParmList -> '$empty': [] . terminationStateDescriptor -> 'TerminationStateToken' 'LBRKT' terminationStateParm - terminationStateParms 'RBRKT' - : merge_terminationStateDescriptor(['$3' | '$4']) . + terminationStateParms 'RBRKT' : + merge_terminationStateDescriptor(['$3' | '$4']) . -terminationStateParms -> 'COMMA' terminationStateParm terminationStateParms : ['$2' | '$3'] . -terminationStateParms -> '$empty' : [] . +terminationStateParms -> 'COMMA' terminationStateParm terminationStateParms : + ['$2' | '$3'] . +terminationStateParms -> '$empty' : + [] . %% at-most-once per item except for propertyParm localParm -> 'ReservedGroupToken' 'EQUAL' onOrOff : {group, '$3'} . @@ -1504,6 +1510,7 @@ statisticsDescriptor -> 'StatsToken' 'LBRKT' statisticsParameter statisticsParameters 'RBRKT' : ['$3' | '$4'] . +statisticsDescriptor -> 'StatsToken' : [] . statisticsParameters -> 'COMMA' statisticsParameter statisticsParameters : ['$2' | '$3'] . statisticsParameters -> '$empty' : [] . -- cgit v1.2.1 From 03dc5cca250d8ee8f4f91d9a054291419c7b1ad6 Mon Sep 17 00:00:00 2001 From: Micael Karlberg Date: Wed, 18 Nov 2020 12:33:38 +0100 Subject: [megaco|test] Add test cases for empty statistics descriptor Add test cases both for compact and pretty. OTP-17012 (ERL-1405) --- lib/megaco/test/megaco_codec_v3_SUITE.erl | 133 ++++++++++++++++++++++++----- lib/megaco/test/megaco_test_lib.erl | 2 +- lib/megaco/test/megaco_test_msg_v3_lib.erl | 33 +++++-- 3 files changed, 139 insertions(+), 29 deletions(-) diff --git a/lib/megaco/test/megaco_codec_v3_SUITE.erl b/lib/megaco/test/megaco_codec_v3_SUITE.erl index 5f4951acce..01cd94c952 100644 --- a/lib/megaco/test/megaco_codec_v3_SUITE.erl +++ b/lib/megaco/test/megaco_codec_v3_SUITE.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2006-2019. All Rights Reserved. +%% Copyright Ericsson AB 2006-2020. All Rights Reserved. %% %% Licensed under the Apache License, Version 2.0 (the "License"); %% you may not use this file except in compliance with the License. @@ -113,6 +113,8 @@ compact_otp6017_msg01/1, compact_otp6017_msg02/1, compact_otp6017_msg03/1, + compact_erl1405_msg01/1, + compact_erl1405_msg02/1, flex_compact_otp4299_msg1/1, flex_compact_otp7431_msg01/1, @@ -169,6 +171,8 @@ pretty_otp7671_msg04/1, pretty_otp7671_msg05/1, pretty_otp8114_msg01/1, + pretty_erl1405_msg01/1, + pretty_erl1405_msg02/1, flex_pretty_otp5042_msg1/1, flex_pretty_otp5085_msg1/1, @@ -380,7 +384,9 @@ compact_tickets_cases() -> compact_otp5993_msg03, compact_otp6017_msg01, compact_otp6017_msg02, - compact_otp6017_msg03 + compact_otp6017_msg03, + compact_erl1405_msg01, + compact_erl1405_msg02 ]. flex_compact_tickets_cases() -> @@ -442,7 +448,9 @@ pretty_tickets_cases() -> pretty_otp7671_msg03, pretty_otp7671_msg04, pretty_otp7671_msg05, - pretty_otp8114_msg01 + pretty_otp8114_msg01, + pretty_erl1405_msg01, + pretty_erl1405_msg02 ]. flex_pretty_tickets_cases() -> @@ -594,13 +602,13 @@ pretty_test_msgs(suite) -> pretty_test_msgs(Config) when is_list(Config) -> ?ACQUIRE_NODES(1, Config), Msgs = - msgs1a(text) ++ - msgs1b(text) ++ - msgs3525(text) ++ - msgs5(text) ++ - msgs6(text) ++ - msgs7(text) ++ - msgs8(text), + msgs1a(text) ++ + msgs1b(text) ++ + msgs3525(text) ++ + msgs5(text) ++ + msgs6(text) ++ + msgs7(text) ++ + msgs8(text), %% Msgs = msgs1a(text), %% Msgs = msgs1b(text), %% Msgs = msgs35525(text), @@ -644,13 +652,13 @@ compact_test_msgs(suite) -> compact_test_msgs(Config) when is_list(Config) -> ?ACQUIRE_NODES(1, Config), Msgs = - msgs1a(text) ++ - msgs1b(text) ++ - msgs3525(text) ++ - msgs5(text) ++ - msgs6(text) ++ - msgs7(text) ++ - msgs8(text), + msgs1a(text) ++ + msgs1b(text) ++ + msgs3525(text) ++ + msgs5(text) ++ + msgs6(text) ++ + msgs7(text) ++ + msgs8(text), %% Msgs = msgs7(text), DynamicDecode = false, test_msgs(megaco_compact_text_encoder, DynamicDecode, ?EC, Msgs). @@ -2172,6 +2180,62 @@ compact_otp6017_msg(CID) when is_integer(CID) -> "{SC=root{SV{MT=RS,RE=901}}}}". +%% -------------------------------------------------------------- + +compact_erl1405_msg01(suite) -> + []; +compact_erl1405_msg01(Config) when is_list(Config) -> + put(severity,trc), + put(dbg,true), + d("compact_erl1405_msg01 -> entry", []), + ?ACQUIRE_NODES(1, Config), + ok = compact_erl1405(statisticsDescriptor), + erase(severity), + erase(dbg), + ok. + +compact_erl1405_msg02(suite) -> + []; +compact_erl1405_msg02(Config) when is_list(Config) -> + put(severity,trc), + put(dbg,true), + d("compact_erl1405_msg02 -> entry", []), + ?ACQUIRE_NODES(1, Config), + ok = compact_erl1405({statisticsDescriptor, []}), + erase(severity), + erase(dbg), + ok. + +compact_erl1405(Descriptor) -> + ticket_compact_encode_decode_ok( erl1405_msg(Descriptor) ). + +erl1405_msg(Descriptor) -> + #'MegacoMessage'{ + mess = #'Message'{ + version = ?VERSION, + mId = {deviceName, "test"}, + messageBody = {transactions, [ + {transactionRequest, #'TransactionRequest'{ + transactionId = 1, + actions = [ + #'ActionRequest'{ + contextId = ?megaco_choose_context_id, + commandRequests = [ + #'CommandRequest'{ + command = {addReq, #'AmmRequest'{ + terminationID = [#megaco_term_id{id = ["test"]}], + descriptors = [Descriptor] + }} + } + ] + } + ] + }} + ]} + } + }. + + %% ============================================================== %% %% F l e x C o m p a c t T e s t c a s e s @@ -3971,7 +4035,6 @@ cmp_otp7671_msg05(#'MegacoMessage'{authHeader = asn1_NOVALUE, %% -------------------------------------------------------------- %% - pretty_otp8114_msg01(suite) -> []; pretty_otp8114_msg01(Config) when is_list(Config) -> @@ -4056,6 +4119,38 @@ otp8114(InitialMessage, Codec, Conf) -> megaco_codec_test_lib:expect_exec(Instructions, InitialData). + +%% -------------------------------------------------------------- +%% + +pretty_erl1405_msg01(suite) -> + []; +pretty_erl1405_msg01(Config) when is_list(Config) -> + put(severity,trc), + put(dbg,true), + d("pretty_erl1405_msg01 -> entry", []), + ?ACQUIRE_NODES(1, Config), + ok = pretty_erl1405(statisticsDescriptor), + erase(severity), + erase(dbg), + ok. + +pretty_erl1405_msg02(suite) -> + []; +pretty_erl1405_msg02(Config) when is_list(Config) -> + put(severity,trc), + put(dbg,true), + d("pretty_erl1405_msg02 -> entry", []), + ?ACQUIRE_NODES(1, Config), + ok = pretty_erl1405({statisticsDescriptor, []}), + erase(severity), + erase(dbg), + ok. + +pretty_erl1405(Descriptor) -> + ticket_pretty_encode_decode_ok( erl1405_msg(Descriptor) ). + + %% ============================================================== %% %% F l e x P r e t t y T e s t c a s e s @@ -4816,7 +4911,7 @@ msgs7(Encoding) -> {msg78a07, msg78a07(), Plain, [{dbg,false}],[text,binary,erlang]}, {msg78a08, msg78a08(), Plain, [{dbg,false}],[text,binary,erlang]}, {msg78a09, msg78a09(), Plain, [{dbg,false}],[text,binary,erlang]}, - {msg79a01, msg79a01(), Plain, [{dbg,false}],[text,binary,erlang]} + {msg79a01, msg79a01(), Plain, [{dbg,true}],[text,binary,erlang]} ], [{N,M,F,C}||{N,M,F,C,E} <- Msgs,lists:member(Encoding,E)]. diff --git a/lib/megaco/test/megaco_test_lib.erl b/lib/megaco/test/megaco_test_lib.erl index 1e4b7841ee..4fc8af67b1 100644 --- a/lib/megaco/test/megaco_test_lib.erl +++ b/lib/megaco/test/megaco_test_lib.erl @@ -495,7 +495,7 @@ init_per_suite(Config) -> end; (_) -> false - end, + end, COND = [ {unix, [{linux, LinuxVersionVerify}, {darwin, DarwinVersionVerify}]}%% , diff --git a/lib/megaco/test/megaco_test_msg_v3_lib.erl b/lib/megaco/test/megaco_test_msg_v3_lib.erl index 5264791370..8bb821b475 100644 --- a/lib/megaco/test/megaco_test_msg_v3_lib.erl +++ b/lib/megaco/test/megaco_test_msg_v3_lib.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2006-2016. All Rights Reserved. +%% Copyright Ericsson AB 2006-2020. All Rights Reserved. %% %% Licensed under the Apache License, Version 2.0 (the "License"); %% you may not use this file except in compliance with the License. @@ -3775,8 +3775,16 @@ is_AmmRequest_descriptors(Descs) -> is_AmmRequest_descriptors([], _) -> true; -is_AmmRequest_descriptors([{Tag, _} = Desc|Descs], FoundDescs) -> - d("is_AmmRequest_descriptors -> entry with" +is_AmmRequest_descriptors([Desc|Descs], FoundDescs) -> + FoundDescs2 = is_AmmRequest_descriptor(Desc, FoundDescs), + is_AmmRequest_descriptors(Descs, FoundDescs2); +is_AmmRequest_descriptors(Descs, _) -> + d("is_AmmRequest_descriptors -> entry with WRONG TYPE" + "~n Descs: ~p", [Descs]), + wrong_type('AmmRequest_descriptors', Descs). + +is_AmmRequest_descriptor({Tag, _} = Desc, FoundDescs) when is_atom(Tag) -> + d("is_AmmRequest_descriptor -> entry with" "~n Tag: ~p" "~n FoundDescs: ~p", [Tag, FoundDescs]), case lists:member(Tag, FoundDescs) of @@ -3785,15 +3793,13 @@ is_AmmRequest_descriptors([{Tag, _} = Desc|Descs], FoundDescs) -> false -> case is_AmmDescriptor(Desc) of true -> - is_AmmRequest_descriptors(Descs, [Tag|FoundDescs]); + [Tag|FoundDescs]; false -> wrong_type('AmmRequest_descriptors', Desc) end end; -is_AmmRequest_descriptors(Descs, _) -> - d("is_AmmRequest_descriptors -> entry with WRONG TYPE" - "~n Descs: ~p", [Descs]), - wrong_type('AmmRequest_descriptors', Descs). +is_AmmRequest_descriptor(Tag, FoundDescs) when is_atom(Tag) -> + is_AmmRequest_descriptor({Tag, []}, FoundDescs). chk_AmmRequest(R, R) when is_record(R, 'AmmRequest') -> @@ -3837,7 +3843,11 @@ chk_AmmRequest_descriptors([H|T1], [H|T2]) -> wrong_type('AmmRequest_descriptors_val', H) end; chk_AmmRequest_descriptors([H1|T1], [H2|T2]) -> - d("chk_AmmRequest_descriptors -> entry when not equal"), + d("chk_AmmRequest_descriptors -> entry when not equal: " + "~n H1: ~p" + "~n T1: ~p" + "~n H2: ~p" + "~n T2: ~p", [H1, T1, H2, T2]), validate(fun() -> chk_AmmDescriptor(H1, H2) end, 'AmmRequest_descriptors_val'), chk_AmmRequest_descriptors(T1, T2); @@ -3885,6 +3895,11 @@ is_AmmDescriptor_val(statisticsDescriptor, D) -> chk_AmmDescriptor(D, D) -> chk_type(fun is_AmmDescriptor_tag/1, 'AmmDescriptor', D); +%% There are two ways of spec an empty statisticsDescriptor: +%% * statisticsDescriptor +%% * {statisticsDescriptor, []} +chk_AmmDescriptor(Tag, {Tag, []}) when (Tag =:= statisticsDescriptor) -> + ok; chk_AmmDescriptor({Tag, Val1} = Cmd1, {Tag, Val2} = Cmd2) -> case (is_AmmDescriptor_tag(Tag) andalso is_AmmDescriptor_val(Tag, Val1) andalso -- cgit v1.2.1