From 1baa54479df7224985b0889562ecd5701b0805bc Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Tue, 28 Apr 2015 14:16:08 +0200 Subject: common_test: Recurse when there is more data in netconf When several packets where receive in one packet ct_netconf failed to deliver them to the user. For example several subscritiption message could be in the buffer but only the first was sent to the user. Error handling could be improved, maybe the connection should be closed when unparseable packet arrives or timeout occurs. --- lib/common_test/src/ct_netconfc.erl | 120 ++++++++------------- .../ct_netconfc_SUITE_data/netconfc1_SUITE.erl | 17 +-- 2 files changed, 57 insertions(+), 80 deletions(-) (limited to 'lib/common_test') diff --git a/lib/common_test/src/ct_netconfc.erl b/lib/common_test/src/ct_netconfc.erl index 85fb1ea8d2..8650bef89c 100644 --- a/lib/common_test/src/ct_netconfc.erl +++ b/lib/common_test/src/ct_netconfc.erl @@ -1118,7 +1118,9 @@ handle_msg({Ref,timeout},#state{pending=Pending} = State) -> close_session -> stop; _ -> noreply end, - {R,State#state{pending=Pending1}}. + %% Halfhearted try to get in correct state, this matches + %% the implementation before this patch + {R,State#state{pending=Pending1, buff= <<>>}}. %% @private %% Called by ct_util_server to close registered connections before terminate. @@ -1308,72 +1310,54 @@ to_xml_doc(Simple) -> %%%----------------------------------------------------------------- %%% Parse and handle received XML data -handle_data(NewData,#state{connection=Connection,buff=Buff} = State) -> +handle_data(NewData,#state{connection=Connection,buff=Buff0} = State0) -> log(Connection,recv,NewData), - Data = <>, - case xmerl_sax_parser:stream(<<>>, - [{continuation_fun,fun sax_cont/1}, - {continuation_state,{Data,Connection,false}}, - {event_fun,fun sax_event/3}, - {event_state,[]}]) of - {ok, Simple, Rest} -> - decode(Simple,State#state{buff=Rest}); - {fatal_error,_Loc,Reason,_EndTags,_EventState} -> - ?error(Connection#connection.name,[{parse_error,Reason}, - {buffer,Buff}, - {new_data,NewData}]), - case Reason of - {could_not_fetch_data,Msg} -> - handle_msg(Msg,State#state{buff = <<>>}); - _Other -> - Pending1 = - case State#state.pending of - [] -> - []; - Pending -> - %% Assuming the first request gets the - %% first answer - P=#pending{tref=TRef,caller=Caller} = - lists:last(Pending), - _ = timer:cancel(TRef), - Reason1 = {failed_to_parse_received_data,Reason}, - ct_gen_conn:return(Caller,{error,Reason1}), - lists:delete(P,Pending) - end, - {noreply,State#state{pending=Pending1,buff = <<>>}} - end - end. - -%%%----------------------------------------------------------------- -%%% Parsing of XML data -%% Contiuation function for the sax parser -sax_cont(done) -> - {<<>>,done}; -sax_cont({Data,Connection,false}) -> + Data = append_wo_initial_nl(Buff0,NewData), case binary:split(Data,[?END_TAG],[]) of - [All] -> - %% No end tag found. Remove what could be a part - %% of an end tag from the data and save for next - %% iteration - SafeSize = size(All)-5, - <> = All, - {New,{Save,Connection,true}}; - [_Msg,_Rest]=Msgs -> - %% We have at least one full message. Any excess data will - %% be returned from xmerl_sax_parser:stream/2 in the Rest - %% parameter. - {list_to_binary(Msgs),done} - end; -sax_cont({Data,Connection,true}) -> - case ssh_receive_data() of - {ok,Bin} -> - log(Connection,recv,Bin), - sax_cont({<>,Connection,false}); - {error,Reason} -> - throw({could_not_fetch_data,Reason}) + [_NoEndTagFound] -> + {noreply, State0#state{buff=Data}}; + [FirstMsg,Buff1] -> + SaxArgs = [{event_fun,fun sax_event/3}, {event_state,[]}], + case xmerl_sax_parser:stream(FirstMsg, SaxArgs) of + {ok, Simple, _Thrash} -> + case decode(Simple, State0#state{buff=Buff1}) of + {noreply, #state{buff=Buff} = State} when Buff =/= <<>> -> + %% Recurse if we have more data in buffer + handle_data(<<>>, State); + Other -> + Other + end; + {fatal_error,_Loc,Reason,_EndTags,_EventState} -> + ?error(Connection#connection.name, + [{parse_error,Reason}, + {buffer, Buff0}, + {new_data,NewData}]), + handle_error(Reason, State0#state{buff= <<>>}) + end end. - +%% xml does not accept a leading nl and some netconf server add a nl after +%% each ?END_TAG, ignore them +append_wo_initial_nl(<<>>,NewData) -> NewData; +append_wo_initial_nl(<<"\n", Data/binary>>, NewData) -> + append_wo_initial_nl(Data, NewData); +append_wo_initial_nl(Data, NewData) -> + <>. + +handle_error(Reason, State) -> + Pending1 = case State#state.pending of + [] -> []; + Pending -> + %% Assuming the first request gets the + %% first answer + P=#pending{tref=TRef,caller=Caller} = + lists:last(Pending), + _ = timer:cancel(TRef), + Reason1 = {failed_to_parse_received_data,Reason}, + ct_gen_conn:return(Caller,{error,Reason1}), + lists:delete(P,Pending) + end, + {noreply, State#state{pending=Pending1}}. %% Event function for the sax parser. It builds a simple XML structure. %% Care is taken to keep namespace attributes and prefixes as in the original XML. @@ -1836,16 +1820,6 @@ get_tag([]) -> %%%----------------------------------------------------------------- %%% SSH stuff -ssh_receive_data() -> - receive - {ssh_cm, CM, {data, Ch, _Type, Data}} -> - ssh_connection:adjust_window(CM,Ch,size(Data)), - {ok, Data}; - {ssh_cm, _CM, {Closed, _Ch}} = X when Closed == closed; Closed == eof -> - {error,X}; - {_Ref,timeout} = X -> - {error,X} - end. ssh_open(#options{host=Host,timeout=Timeout,port=Port,ssh=SshOpts,name=Name}) -> case ssh:connect(Host, Port, diff --git a/lib/common_test/test/ct_netconfc_SUITE_data/netconfc1_SUITE.erl b/lib/common_test/test/ct_netconfc_SUITE_data/netconfc1_SUITE.erl index a145d85b1d..099deeb980 100644 --- a/lib/common_test/test/ct_netconfc_SUITE_data/netconfc1_SUITE.erl +++ b/lib/common_test/test/ct_netconfc_SUITE_data/netconfc1_SUITE.erl @@ -490,13 +490,16 @@ action(Config) -> Data = [{myactionreturn,[{xmlns,"myns"}],["value"]}], %% test either to receive {data,Data} or {ok,Data}, %% both need to be handled - {Reply,RetVal} = case element(3, now()) rem 2 of - 0 -> {{data,Data},{ok,Data}}; - 1 -> {{ok,Data},ok} - end, - ct:log("Client will receive {~w,Data}", [element(1,Reply)]), - ?NS:expect_reply(action,Reply), - RetVal = ct_netconfc:action(Client,{myaction,[{xmlns,"myns"}],[]}), + ct:log("Client will receive {~w,~p}", [data,Data]), + ct:log("Expecting ~p", [{ok, Data}]), + ?NS:expect_reply(action,{data, Data}), + {ok, Data} = ct_netconfc:action(Client,{myaction,[{xmlns,"myns"}],[]}), + + ct:log("Client will receive {~w,~p}", [ok,Data]), + ct:log("Expecting ~p", [ok]), + ?NS:expect_reply(action,{ok, Data}), + ok = ct_netconfc:action(Client,{myaction,[{xmlns,"myns"}],[]}), + ?NS:expect_do_reply('close-session',close,ok), ?ok = ct_netconfc:close_session(Client), ok. -- cgit v1.2.3