[ct_netconfc] Fix XML parsing when multiple messages in package

If a ssh package contained more than one netconf end tag, then the
second end tag was never detected in ct_netconfc:handle_data. Instead
it was included in the XML data given to the xmerl parser, which then
failed with reason "\"]]>\" is not allowed in content".

This problem was introduced by OTP-13007.

3 files changed, 233 insertions, 28 deletions

diff --git a/lib/common_test/src/ct_netconfc.erl b/lib/common_test/src/ct_netconfc.erl
index 6e3d1ab1d8..3da1115c76 100644
--- a/lib/common_test/src/ct_netconfc.erl
+++ b/lib/common_test/src/ct_netconfc.erl
@@ -264,6 +264,7 @@
 		session_id,
 		msg_id = 1,
 		hello_status,
+		no_end_tag_buff = <<>>,
 		buff = <<>>,
 		pending = [],    % [#pending]
 		event_receiver}).% pid
@@ -1170,7 +1171,7 @@ handle_msg({Ref,timeout},#state{pending=Pending} = State) ->
 	end,
     %% Halfhearted try to get in correct state, this matches
     %% the implementation before this patch
-    {R,State#state{pending=Pending1, buff= <<>>}}.
+    {R,State#state{pending=Pending1, no_end_tag_buff= <<>>, buff= <<>>}}.
 
 %% @private
 %% Called by ct_util_server to close registered connections before terminate.
@@ -1205,7 +1206,7 @@ call(Client, Msg, Timeout, WaitStop) ->
 		    {error,no_such_client};
 		{error,{process_down,Pid,normal}} when WaitStop ->
 		    %% This will happen when server closes connection
-		    %% before clien received rpc-reply on
+		    %% before client received rpc-reply on
 		    %% close-session.
 		    ok;
 		{error,{process_down,Pid,normal}} ->
@@ -1372,24 +1373,37 @@ to_xml_doc(Simple) ->
 
 %%%-----------------------------------------------------------------
 %%% Parse and handle received XML data
-handle_data(NewData,#state{connection=Connection,buff=Buff0} = State0) ->
+%%% Two buffers are used:
+%%%   * 'no_end_tag_buff' contains data that is checked and does not
+%%%     contain any (part of an) end tag.
+%%%   * 'buff' contains all other saved data - it may or may not
+%%%     include (a part of) an end tag.
+%%% The reason for this is to avoid running binary:split/3 multiple
+%%% times on the same data when it does not contain an end tag. This
+%%% can be a considerable optimation in the case when a lot of data is
+%%% received (e.g. when fetching all data from a node) and the data is
+%%% sent in multiple ssh packages.
+handle_data(NewData,#state{connection=Connection} = State0) ->
     log(Connection,recv,NewData),
-    {Start,AddSz} =
-	case byte_size(Buff0) of
-	    BSz when BSz<5 -> {0,BSz};
-	    BSz -> {BSz-5,5}
-	end,
-    Length = byte_size(NewData) + AddSz,
+    NoEndTag0 = State0#state.no_end_tag_buff,
+    Buff0 = State0#state.buff,
     Data = <<Buff0/binary, NewData/binary>>,
-    case binary:split(Data,?END_TAG,[{scope,{Start,Length}}]) of
+    case binary:split(Data,?END_TAG,[]) of
 	[_NoEndTagFound] ->
-	    {noreply, State0#state{buff=Data}};
+	    NoEndTagSize = case byte_size(Data) of
+			       Sz when Sz<5 -> 0;
+			       Sz -> Sz-5
+			   end,
+	    <<NoEndTag1:NoEndTagSize/binary,Buff/binary>> = Data,
+	    NoEndTag = <<NoEndTag0/binary,NoEndTag1/binary>>,
+	    {noreply, State0#state{no_end_tag_buff=NoEndTag, buff=Buff}};
 	[FirstMsg0,Buff1] ->
-	    FirstMsg = remove_initial_nl(FirstMsg0),
+	    FirstMsg = remove_initial_nl(<<NoEndTag0/binary,FirstMsg0/binary>>),
 	    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
+		    case decode(Simple, State0#state{no_end_tag_buff= <<>>,
+						     buff=Buff1}) of
 			{noreply, #state{buff=Buff} = State} when Buff =/= <<>> ->
 			    %% Recurse if we have more data in buffer
 			    handle_data(<<>>, State);
@@ -1401,10 +1415,12 @@ handle_data(NewData,#state{connection=Connection,buff=Buff0} = State0) ->
 			   [{parse_error,Reason},
 			    {buffer, Buff0},
 			    {new_data,NewData}]),
-		    handle_error(Reason, State0#state{buff= <<>>})
+		    handle_error(Reason, State0#state{no_end_tag_buff= <<>>,
+						      buff= <<>>})
 	    end
     end.
 
+
 %% xml does not accept a leading nl and some netconf server add a nl after
 %% each ?END_TAG, ignore them
 remove_initial_nl(<<"\n", Data/binary>>) ->
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 ea49e36608..49b02d2bba 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
@@ -99,7 +99,10 @@ all() ->
 	     connection_crash,
 	     get_event_streams,
 	     create_subscription,
-	     receive_event
+	     receive_one_event,
+	     receive_multiple_events,
+	     receive_event_and_rpc,
+	     receive_event_and_rpc_in_chunks
 	    ]
     end.
 
@@ -354,7 +357,7 @@ get(Config) ->
 get_a_lot(Config) ->
     DataDir = ?config(data_dir,Config),
     {ok,Client} = open_success(DataDir),
-    Descr = lists:append(lists:duplicate(100,"Description of myserver! ")),
+    Descr = lists:append(lists:duplicate(1000,"Description of myserver! ")),
     Server = {server,[{xmlns,"myns"}],[{name,[],["myserver"]},
 				       {description,[],[Descr]}]},
     Data = lists:duplicate(100,Server),
@@ -964,16 +967,16 @@ create_subscription(Config) ->
 
     ok.
 
-receive_event(Config) ->
+receive_one_event(Config) ->
     DataDir = ?config(data_dir,Config),
     {ok,Client} = open_success(DataDir),
     ?NS:expect_reply({'create-subscription',[stream]},ok),
     ?ok = ct_netconfc:create_subscription(Client),
 
-    ?NS:hupp(send_event),
+    ?NS:hupp({send_events,1}),
 
     receive
-	%% Matching ?NS:make_msg(event)
+	%% Matching ?NS:make_msg({event,_})
 	{notification,?NETCONF_NOTIF_NAMESPACE_ATTR,
 	 [{eventTime,[],[_Time]},
 	  {event,[{xmlns,"http://my.namespaces.com/event"}],
@@ -991,6 +994,187 @@ receive_event(Config) ->
 
     ok.
 
+receive_multiple_events(Config) ->
+    DataDir = ?config(data_dir,Config),
+    {ok,Client} = open_success(DataDir),
+    ?NS:expect_reply({'create-subscription',[stream]},ok),
+    ?ok = ct_netconfc:create_subscription(Client),
+
+    ?NS:hupp({send_events,3}),
+
+    receive
+	%% Matching ?NS:make_msg({event,_})
+	{notification,_,_} ->
+	    ok;
+	Other1 ->
+	    ct:fail({got_unexpected_while_waiting_for_event, Other1})
+    after 3000 ->
+	    ct:fail(timeout_waiting_for_event)
+    end,
+    receive
+	%% Matching ?NS:make_msg({event,_})
+	{notification,_,_} ->
+	    ok;
+	Other2 ->
+	    ct:fail({got_unexpected_while_waiting_for_event, Other2})
+    after 3000 ->
+	    ct:fail(timeout_waiting_for_event)
+    end,
+    receive
+	%% Matching ?NS:make_msg({event,_})
+	{notification,_,_} ->
+	    ok;
+	Other3 ->
+	    ct:fail({got_unexpected_while_waiting_for_event, Other3})
+    after 3000 ->
+	    ct:fail(timeout_waiting_for_event)
+    end,
+
+    ?NS:expect_do_reply('close-session',close,ok),
+    ?ok = ct_netconfc:close_session(Client),
+
+    ok.
+
+receive_event_and_rpc(Config) ->
+    DataDir = ?config(data_dir,Config),
+    {ok,Client} = open_success(DataDir),
+
+    ?NS:expect_reply({'create-subscription',[stream]},ok),
+    ?ok = ct_netconfc:create_subscription(Client),
+
+    %% Construct the data to return from netconf server - one
+    %% rpc-reply and one notification - to be sent in the same ssh
+    %% package.
+    Data = [{servers,[{xmlns,"myns"}],[{server,[],[{name,[],["myserver"]}]}]}],
+    Rpc = {'rpc-reply',?NETCONF_NAMESPACE_ATTR ++ [{'message-id',"2"}],
+	   [{data,Data}]},
+    RpcXml = list_to_binary(xmerl:export_simple_element(Rpc,xmerl_xml)),
+
+    Notification =
+	{notification,?NETCONF_NOTIF_NAMESPACE_ATTR,
+	 [{eventTime,["2012-06-14T14:50:54+02:00"]},
+	  {event,[{xmlns,"http://my.namespaces.com/event"}],
+	   [{severity,["major"]},
+	    {description,["Something terrible happened"]}]}]},
+    NotifXml =
+	list_to_binary(xmerl:export_simple_element(Notification,xmerl_xml)),
+
+    ?NS:expect_reply('get',[RpcXml,NotifXml]),
+    {ok,Data} = ct_netconfc:get(Client,{server,[{xmlns,"myns"}],[]}),
+
+    receive
+	{notification,_,_} ->
+	    ok;
+	Other1 ->
+	    ct:fail({got_unexpected_while_waiting_for_event, Other1})
+    after 3000 ->
+	    ct:fail(timeout_waiting_for_event)
+    end,
+
+
+    %% Then do the same again, but now send notification first then
+    %% the rpc-reply.
+    Rpc2 = {'rpc-reply',?NETCONF_NAMESPACE_ATTR ++ [{'message-id',"3"}],
+	   [{data,Data}]},
+    RpcXml2 = list_to_binary(xmerl:export_simple_element(Rpc2,xmerl_xml)),
+    ?NS:expect_reply('get',[NotifXml,RpcXml2]),
+    {ok,Data} = ct_netconfc:get(Client,{server,[{xmlns,"myns"}],[]}),
+
+    receive
+	{notification,_,_} ->
+	    ok;
+	Other2 ->
+	    ct:fail({got_unexpected_while_waiting_for_event, Other2})
+    after 3000 ->
+	    ct:fail(timeout_waiting_for_event)
+    end,
+
+    ?NS:expect_do_reply('close-session',close,ok),
+    ?ok = ct_netconfc:close_session(Client),
+
+    ok.
+
+
+receive_event_and_rpc_in_chunks(Config) ->
+    DataDir = ?config(data_dir,Config),
+    {ok,Client} = open_success(DataDir),
+
+    ?NS:expect_reply({'create-subscription',[stream]},ok),
+    ?ok = ct_netconfc:create_subscription(Client),
+
+    %% Construct the data to return from netconf server
+    Data = [{servers,[{xmlns,"myns"}],
+	     [{server,[],[{name,[],["server0"]}]},
+	      {server,[],[{name,[],["server1"]}]},
+	      {server,[],[{name,[],["server2"]}]},
+	      {server,[],[{name,[],["server3"]}]},
+	      {server,[],[{name,[],["server4"]}]},
+	      {server,[],[{name,[],["server5"]}]},
+	      {server,[],[{name,[],["server6"]}]},
+	      {server,[],[{name,[],["server7"]}]},
+	      {server,[],[{name,[],["server8"]}]},
+	      {server,[],[{name,[],["server9"]}]}]
+	    }],
+    Rpc = {'rpc-reply',?NETCONF_NAMESPACE_ATTR ++ [{'message-id',"2"}],
+	   [{data,Data}]},
+    RpcXml = list_to_binary(xmerl:export_simple_element(Rpc,xmerl_xml)),
+
+    Notification =
+	{notification,?NETCONF_NOTIF_NAMESPACE_ATTR,
+	 [{eventTime,["2012-06-14T14:50:54+02:00"]},
+	  {event,[{xmlns,"http://my.namespaces.com/event"}],
+	   [{severity,["major"]},
+	    {description,["Something terrible happened"]}]}]},
+    NotifXml =
+	list_to_binary(xmerl:export_simple_element(Notification,xmerl_xml)),
+
+
+    %% First part contains a notif, but only parts of the end tag
+    Part1 =
+	<<"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n",
+	  NotifXml/binary,"\n]]">>,
+
+    %% Second part contains rest of end tag, full rpc-reply and full
+    %% notif except end tag
+    Part2 =
+	<<">]]>\n",RpcXml/binary,"\n",?END_TAG/binary,NotifXml/binary>>,
+
+    %% Third part contains last end tag
+    Part3 = <<"\n",?END_TAG/binary,"\n">>,
+
+    %% Spawn a process which will wait a bit for the client to send
+    %% the request (below), then order the server to the chunks of the
+    %% rpc-reply one by one.
+    spawn(fun() -> ct:sleep(500),?NS:hupp(send,Part1),
+		   ct:sleep(100),?NS:hupp(send,Part2),
+		   ct:sleep(100),?NS:hupp(send,Part3)
+	  end),
+
+    %% Order server to expect a get - then the process above will make
+    %% sure the rpc-reply is sent.
+    ?NS:expect('get'),
+    {ok,Data} = ct_netconfc:get(Client,{server,[{xmlns,"myns"}],[]}),
+
+    receive
+	{notification,_,_} ->
+	    ok;
+	Other1 ->
+	    ct:fail({got_unexpected_while_waiting_for_event, Other1})
+    after 3000 ->
+	    ct:fail(timeout_waiting_for_event)
+    end,
+    receive
+	{notification,_,_} ->
+	    ok;
+	Other2 ->
+	    ct:fail({got_unexpected_while_waiting_for_event, Other2})
+    after 3000 ->
+	    ct:fail(timeout_waiting_for_event)
+    end,
+    ?NS:expect_do_reply('close-session',close,ok),
+    ?ok = ct_netconfc:close_session(Client),
+    ok.
+
 %%%-----------------------------------------------------------------
 
 break(_Config) ->
diff --git a/lib/common_test/test/ct_netconfc_SUITE_data/ns.erl b/lib/common_test/test/ct_netconfc_SUITE_data/ns.erl
index 07893faabc..67827a053f 100644
--- a/lib/common_test/test/ct_netconfc_SUITE_data/ns.erl
+++ b/lib/common_test/test/ct_netconfc_SUITE_data/ns.erl
@@ -144,8 +144,8 @@ expect_do_reply(SessionId,Expect,Do,Reply) ->
 %% Hupp the server - i.e. tell it to do something -
 %% e.g. hupp(send_event) will cause send_event(State) to be called on
 %% the session channel process.
-hupp(send_event) ->
-    hupp(send,[make_msg(event)]);
+hupp({send_events,N}) ->
+    hupp(send,[make_msg({event,N})]);
 hupp(kill) ->
     hupp(1,fun hupp_kill/1,[]).
 
@@ -446,9 +446,12 @@ reply(ConnRef,Reply) ->
 from_simple(Simple) ->
     unicode_c2b(xmerl:export_simple_element(Simple,xmerl_xml)).
 
-xml(Content) ->
-    <<"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n",
-      Content/binary,"\n",?END_TAG/binary>>.
+xml(Content) when is_binary(Content) ->
+    xml([Content]);
+xml(Content) when is_list(Content) ->
+    Msgs = [<<Msg/binary,"\n",?END_TAG/binary>> || Msg <- Content],
+    MsgsBin = list_to_binary(Msgs),
+    <<"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n", MsgsBin/binary>>.
 
 rpc_reply(Content) when is_binary(Content) ->
     MsgId = case erase(msg_id) of
@@ -571,15 +574,17 @@ make_msg({ok,Data}) ->
 make_msg({data,Data}) ->
     xml(rpc_reply(from_simple({data,Data})));
 
-make_msg(event) ->
-    xml(<<"<notification xmlns=\"",?NETCONF_NOTIF_NAMESPACE,"\">"
+make_msg({event,N}) ->
+    Notification = <<"<notification xmlns=\"",?NETCONF_NOTIF_NAMESPACE,"\">"
 	  "<eventTime>2012-06-14T14:50:54+02:00</eventTime>"
 	  "<event xmlns=\"http://my.namespaces.com/event\">"
 	  "<severity>major</severity>"
 	  "<description>Something terrible happened</description>"
 	  "</event>"
-	  "</notification>">>);
-make_msg(Xml) when is_binary(Xml) ->
+	  "</notification>">>,
+    xml(lists:duplicate(N,Notification));
+make_msg(Xml) when is_binary(Xml) orelse
+		   (is_list(Xml) andalso is_binary(hd(Xml))) ->
     xml(Xml);
 make_msg(Simple) when is_tuple(Simple) ->
     xml(from_simple(Simple)).