Merge branch 'siri/ct_telnet/div-improvements/OTP-15229' into maint

* siri/ct_telnet/div-improvements/OTP-15229:
  [ct] Add option {newline,string()} to ct_telnet:cmd and ct_telnet:send
  [ct] Improve error handling of bad regular expressions to ct_telnet

6 files changed, 240 insertions, 68 deletions

diff --git a/lib/common_test/doc/src/ct_telnet.xml b/lib/common_test/doc/src/ct_telnet.xml
index 9a12ce79ed..76f5305c46 100644
--- a/lib/common_test/doc/src/ct_telnet.xml
+++ b/lib/common_test/doc/src/ct_telnet.xml
@@ -239,18 +239,21 @@
         <v>Connection = connection()</v>
         <v>Cmd = string()</v>
         <v>Opts = [Opt]</v>
-        <v>Opt = {timeout, timeout()} | {newline, boolean()}</v>
+        <v>Opt = {timeout, timeout()} | {newline, boolean() | string()}</v>
         <v>Data = [string()]</v>
         <v>Reason = term()</v>
       </type>
       <desc><marker id="cmd-3"/>
         <p>Sends a command through Telnet and waits for prompt.</p>
 
-        <p>By default, this function adds a new line to the end of the
+        <p>By default, this function adds "\n" to the end of the
           specified command. If this is not desired, use option
           <c>{newline,false}</c>. This is necessary, for example, when
           sending Telnet command sequences prefixed with character
-          Interprete As Command (IAC).</p>
+          Interpret As Command (IAC). Option <c>{newline,string()}</c>
+          can also be used if a different line end than "\n" is
+          required, for instance <c>{newline,"\r\n"}</c>, to add both
+          carriage return and newline characters.</p>
 
         <p>Option <c>timeout</c> specifies how long the client must wait
           for prompt. If the time expires, the function returns
@@ -280,7 +283,7 @@
         <v>CmdFormat = string()</v>
         <v>Args = list()</v>
         <v>Opts = [Opt]</v>
-        <v>Opt = {timeout, timeout()} | {newline, boolean()}</v>
+        <v>Opt = {timeout, timeout()} | {newline, boolean() | string()}</v>
         <v>Data = [string()]</v>
         <v>Reason = term()</v>
       </type>
@@ -339,7 +342,7 @@
           subexpression number <c>N</c>. Subexpressions are denoted with
           <c>'(' ')'</c> in the regular expression.</p>
 
-        <p>If a <c>Tag</c> is speciifed, the returned <c>Match</c> also
+        <p>If a <c>Tag</c> is specified, the returned <c>Match</c> also
           includes the matched <c>Tag</c>. Otherwise, only <c>RxMatch</c>
           is returned.</p>
 
@@ -382,7 +385,7 @@
             can abort the operation of waiting for prompt.</p></item>
           <tag><c>repeat | repeat, N</c></tag>
           <item><p>The pattern(s) must be matched multiple times. If <c>N</c>
-            is speciified, the pattern(s) are matched <c>N</c> times, and
+            is specified, the pattern(s) are matched <c>N</c> times, and
             the function returns <c>HaltReason = done</c>. This option can be
             interrupted by one or more <c>HaltPatterns</c>. <c>MatchList</c>
             is always returned, that is, a list of <c>Match</c> instead of
@@ -547,17 +550,20 @@
         <v>Connection = connection()</v>
         <v>Cmd = string()</v>
         <v>Opts = [Opt]</v>
-        <v>Opt = {newline, boolean()}</v>
+        <v>Opt = {newline, boolean() | string()}</v>
         <v>Reason = term()</v>
       </type>
       <desc><marker id="send-3"/>
         <p>Sends a Telnet command and returns immediately.</p>
 
-        <p>By default, this function adds a newline to the end of the
+        <p>By default, this function adds "\n" to the end of the
           specified command. If this is not desired, option
           <c>{newline,false}</c> can be used. This is necessary, for example,
           when sending Telnet command sequences prefixed with character   
-          Interprete As Command (IAC).</p>
+          Interpret As Command (IAC). Option <c>{newline,string()}</c>
+          can also be used if a different line end than "\n" is
+          required, for instance <c>{newline,"\r\n"}</c>, to add both
+          carriage return and newline characters.</p>
 
         <p>The resulting output from the command can be read with
           <seealso marker="#get_data-1"><c>ct_telnet:get_data/2</c></seealso> or
@@ -584,12 +590,15 @@
         <v>CmdFormat = string()</v>
         <v>Args = list()</v>
         <v>Opts = [Opt]</v>
-        <v>Opt = {newline, boolean()}</v>
+        <v>Opt = {newline, boolean() | string()}</v>
         <v>Reason = term()</v>
       </type>
       <desc><marker id="sendf-4"/>
         <p>Sends a Telnet command and returns immediately (uses a format
           string and a list of arguments to build the command).</p>
+
+        <p>For details, see
+          <seealso marker="#send-3"><c>ct_telnet:send/3</c></seealso>.</p>
       </desc>
     </func>
   </funcs>
diff --git a/lib/common_test/src/ct_telnet.erl b/lib/common_test/src/ct_telnet.erl
index f9abecfd38..174008c790 100644
--- a/lib/common_test/src/ct_telnet.erl
+++ b/lib/common_test/src/ct_telnet.erl
@@ -194,6 +194,15 @@ send(Connection,Cmd,Opts) ->
 
 check_send_opts([{newline,Bool}|Opts]) when is_boolean(Bool) ->
     check_send_opts(Opts);
+check_send_opts([{newline,String}|Opts]) when is_list(String) ->
+    case lists:all(fun(I) when is_integer(I), I>=0, I=<127 -> true;
+                      (_) -> false
+                   end, String) of
+        true ->
+            check_send_opts(Opts);
+        false ->
+            {error,{invalid_option,{newline,String}}}
+    end;
 check_send_opts([Invalid|_]) ->
     {error,{invalid_option,Invalid}};
 check_send_opts([]) ->
@@ -211,10 +220,16 @@ expect(Connection,Patterns) ->
 
 expect(Connection,Patterns,Opts) ->
     case get_handle(Connection) of
-	{ok,Pid} ->
-	    call(Pid,{expect,Patterns,Opts});
-	Error ->
-	    Error
+        {ok,Pid} ->
+            case call(Pid,{expect,Patterns,Opts}) of
+                {error,Reason} when element(1,Reason)==bad_pattern ->
+                    %% Faulty user input - should fail the test case
+                    exit({Reason,{?MODULE,?FUNCTION_NAME,3}});
+                Other ->
+                    Other
+            end;
+        Error ->
+            Error
     end.
 
 %%%=================================================================
@@ -674,60 +689,68 @@ silent_teln_expect(Name,Pid,Data,Pattern,Prx,Opts) ->
 %% 3b) Repeat (sequence): 2) is repeated either N times or until a
 %% halt condition is fulfilled.
 teln_expect(Name,Pid,Data,Pattern0,Prx,Opts) ->
-    HaltPatterns =
+    HaltPatterns0 =
 	case get_ignore_prompt(Opts) of
 	    true ->
 		get_haltpatterns(Opts);
 	    false ->
 		[prompt | get_haltpatterns(Opts)]
 	end,
-
-    PromptCheck = get_prompt_check(Opts),
-
-    {WaitForPrompt,Pattern1,Opts1} = wait_for_prompt(Pattern0,Opts),
-
-    Seq = get_seq(Opts1),
-    Pattern2 = convert_pattern(Pattern1,Seq),
-    {IdleTimeout,TotalTimeout} = get_timeouts(Opts1),
-
-    EO = #eo{teln_pid=Pid,
-	     prx=Prx,
-	     idle_timeout=IdleTimeout,
-	     total_timeout=TotalTimeout,
-	     seq=Seq,
-	     haltpatterns=HaltPatterns,
-	     prompt_check=PromptCheck},
+    case convert_pattern(HaltPatterns0,false) of
+        {ok,HaltPatterns} ->
+            {WaitForPrompt,Pattern1,Opts1} = wait_for_prompt(Pattern0,Opts),
+            Seq = get_seq(Opts1),
+            case convert_pattern(Pattern1,Seq) of
+                {ok,Pattern2} ->
+                    {IdleTimeout,TotalTimeout} = get_timeouts(Opts1),
+                    PromptCheck = get_prompt_check(Opts1),
+
+                    EO = #eo{teln_pid=Pid,
+                             prx=Prx,
+                             idle_timeout=IdleTimeout,
+                             total_timeout=TotalTimeout,
+                             seq=Seq,
+                             haltpatterns=HaltPatterns,
+                             prompt_check=PromptCheck},
     
-    case get_repeat(Opts1) of
-	false ->
-	    case teln_expect1(Name,Pid,Data,Pattern2,[],EO) of
-		{ok,Matched,Rest} when WaitForPrompt ->
-		    case lists:reverse(Matched) of
-			[{prompt,_},Matched1] ->
-			    {ok,Matched1,Rest};
-			[{prompt,_}|Matched1] ->
-			    {ok,lists:reverse(Matched1),Rest}
-		    end;
-		{ok,Matched,Rest} ->
-		    {ok,Matched,Rest};
-		{halt,Why,Rest} ->
-		    {error,Why,Rest};
-		{error,Reason} ->
-		    {error,Reason}
-	    end;
-	N ->
-	    EO1 = EO#eo{repeat=N},
-	    repeat_expect(Name,Pid,Data,Pattern2,[],EO1)
+                    case get_repeat(Opts1) of
+                        false ->
+                            case teln_expect1(Name,Pid,Data,Pattern2,[],EO) of
+                                {ok,Matched,Rest} when WaitForPrompt ->
+                                    case lists:reverse(Matched) of
+                                        [{prompt,_},Matched1] ->
+                                            {ok,Matched1,Rest};
+                                        [{prompt,_}|Matched1] ->
+                                            {ok,lists:reverse(Matched1),Rest}
+                                    end;
+                                {ok,Matched,Rest} ->
+                                    {ok,Matched,Rest};
+                                {halt,Why,Rest} ->
+                                    {error,Why,Rest};
+                                {error,Reason} ->
+                                    {error,Reason}
+                            end;
+                        N ->
+                            EO1 = EO#eo{repeat=N},
+                            repeat_expect(Name,Pid,Data,Pattern2,[],EO1)
+                    end;
+               Error ->
+                    Error
+            end;
+        Error ->
+            Error
     end.
 
-convert_pattern(Pattern,Seq) 
-  when is_list(Pattern) and not is_integer(hd(Pattern)) ->
-    case Seq of
-	true -> Pattern;
-	false -> rm_dupl(Pattern,[])
-    end;
+convert_pattern(Pattern0,Seq)
+  when Pattern0==[] orelse (is_list(Pattern0) and not is_integer(hd(Pattern0))) ->
+    Pattern =
+        case Seq of
+            true -> Pattern0;
+            false -> rm_dupl(Pattern0,[])
+        end,
+    compile_pattern(Pattern,[]);
 convert_pattern(Pattern,_Seq) ->
-    [Pattern].
+    compile_pattern([Pattern],[]).
 
 rm_dupl([P|Ps],Acc) ->
     case lists:member(P,Acc) of
@@ -739,6 +762,25 @@ rm_dupl([P|Ps],Acc) ->
 rm_dupl([],Acc) ->
     lists:reverse(Acc).
 
+compile_pattern([prompt|Patterns],Acc) ->
+    compile_pattern(Patterns,[prompt|Acc]);
+compile_pattern([{prompt,_}=P|Patterns],Acc) ->
+    compile_pattern(Patterns,[P|Acc]);
+compile_pattern([{Tag,Pattern}|Patterns],Acc) ->
+    try re:compile(Pattern,[unicode]) of
+        {ok,MP} -> compile_pattern(Patterns,[{Tag,MP}|Acc]);
+        {error,Error} -> {error,{bad_pattern,{Tag,Pattern},Error}}
+    catch error:badarg -> {error,{bad_pattern,{Tag,Pattern}}}
+    end;
+compile_pattern([Pattern|Patterns],Acc) ->
+    try re:compile(Pattern,[unicode]) of
+        {ok,MP} -> compile_pattern(Patterns,[MP|Acc]);
+        {error,Error} -> {error,{bad_pattern,Pattern,Error}}
+    catch error:badarg -> {error,{bad_pattern,Pattern}}
+    end;
+compile_pattern([],Acc) ->
+    {ok,lists:reverse(Acc)}.
+
 get_timeouts(Opts) ->
     {case lists:keysearch(idle_timeout,1,Opts) of
 	 {value,{_,T}} ->
@@ -772,7 +814,7 @@ get_seq(Opts) ->
 get_haltpatterns(Opts) ->
     case lists:keysearch(halt,1,Opts) of
 	{value,{halt,HaltPatterns}} ->
-	    convert_pattern(HaltPatterns,false);
+	    HaltPatterns;
 	false ->
 	    []
     end.
@@ -1068,7 +1110,7 @@ match_line(Name,Pid,Line,[{prompt,PromptType}|Patterns],FoundPrompt,Term,
   when PromptType=/=FoundPrompt ->
     match_line(Name,Pid,Line,Patterns,FoundPrompt,Term,EO,RetTag);
 match_line(Name,Pid,Line,[{Tag,Pattern}|Patterns],FoundPrompt,Term,EO,RetTag) ->
-    case re:run(Line,Pattern,[{capture,all,list},unicode]) of
+    case re:run(Line,Pattern,[{capture,all,list}]) of
 	nomatch ->
 	    match_line(Name,Pid,Line,Patterns,FoundPrompt,Term,EO,RetTag);
 	{match,Match} ->
@@ -1076,7 +1118,7 @@ match_line(Name,Pid,Line,[{Tag,Pattern}|Patterns],FoundPrompt,Term,EO,RetTag) ->
 	    {RetTag,{Tag,Match}}
     end;
 match_line(Name,Pid,Line,[Pattern|Patterns],FoundPrompt,Term,EO,RetTag) ->
-    case re:run(Line,Pattern,[{capture,all,list},unicode]) of
+    case re:run(Line,Pattern,[{capture,all,list}]) of
 	nomatch ->
 	    match_line(Name,Pid,Line,Patterns,FoundPrompt,Term,EO,RetTag);
 	{match,Match} ->
diff --git a/lib/common_test/src/ct_telnet_client.erl b/lib/common_test/src/ct_telnet_client.erl
index 76e4b9ea70..007477c855 100644
--- a/lib/common_test/src/ct_telnet_client.erl
+++ b/lib/common_test/src/ct_telnet_client.erl
@@ -101,9 +101,11 @@ close(Pid) ->
     end.	    
 
 send_data(Pid, Data) ->
-    send_data(Pid, Data, true).
+    send_data(Pid, Data, "\n").
 send_data(Pid, Data, true) ->
-    send_data(Pid, Data++"\n", false);
+    send_data(Pid, Data, "\n");
+send_data(Pid, Data, Newline) when is_list(Newline) ->
+    send_data(Pid, Data++Newline, false);
 send_data(Pid, Data, false) ->
     Pid ! {send_data, Data},
     ok.
diff --git a/lib/common_test/test/ct_telnet_SUITE.erl b/lib/common_test/test/ct_telnet_SUITE.erl
index a0089c9bc9..f71b7c370f 100644
--- a/lib/common_test/test/ct_telnet_SUITE.erl
+++ b/lib/common_test/test/ct_telnet_SUITE.erl
@@ -50,10 +50,10 @@
 suite() -> [{ct_hooks,[ts_install_cth]}].
 
 groups() ->
-    [{legacy, [], [unix_telnet,own_server,timetrap]},
-     {raw,    [], [unix_telnet,own_server,timetrap]},
-     {html,   [], [unix_telnet,own_server]},
-     {silent, [], [unix_telnet,own_server]}].
+    [{legacy, [], [unix_telnet,own_server,faulty_regexp,timetrap]},
+     {raw,    [], [unix_telnet,own_server,faulty_regexp,timetrap]},
+     {html,   [], [unix_telnet,own_server,faulty_regexp]},
+     {silent, [], [unix_telnet,own_server,faulty_regexp]}].
 
 all() ->
     [
@@ -119,6 +119,12 @@ own_server(Config) ->
     all_tests_in_suite(own_server,"ct_telnet_own_server_SUITE",
 		       CfgFile,Config).
 
+faulty_regexp(Config) ->
+    CfgFile = "telnet.faulty_regexp." ++
+	atom_to_list(groupname(Config)) ++ ".cfg",
+    all_tests_in_suite(faulty_regexp,"ct_telnet_faulty_regexp_SUITE",
+		       CfgFile,Config).
+
 timetrap(Config) ->
     CfgFile = "telnet.timetrap." ++
 	atom_to_list(groupname(Config)) ++ ".cfg",
@@ -225,6 +231,31 @@ events_to_check(unix_telnet,Config) ->
     all_cases(ct_telnet_basic_SUITE,Config);
 events_to_check(own_server,Config) ->
     all_cases(ct_telnet_own_server_SUITE,Config);
+events_to_check(faulty_regexp,_Config) ->
+    [{?eh,start_logging,{'DEF','RUNDIR'}},
+     {?eh,tc_done,
+      {ct_telnet_faulty_regexp_SUITE,expect_pattern,
+       {failed,
+        {error,{{bad_pattern,"invalid(pattern",{"missing )",15}},
+                {ct_telnet,expect,3}}}}}},
+     {?eh,tc_done,
+      {ct_telnet_faulty_regexp_SUITE,expect_pattern_no_string,
+       {failed,
+        {error,{{bad_pattern,invalid_pattern},
+                {ct_telnet,expect,3}}}}}},
+     {?eh,tc_done,
+      {ct_telnet_faulty_regexp_SUITE,expect_tag_pattern,
+       {failed,
+        {error,{{bad_pattern,{tag,"invalid(pattern"},{"missing )",15}},
+                {ct_telnet,expect,3}}}}}},
+     {?eh,tc_done,
+      {ct_telnet_faulty_regexp_SUITE,expect_tag_pattern_no_string,
+       {failed,
+        {error,{{bad_pattern,{tag,invalid_pattern}},
+                {ct_telnet,expect,3}}}}}},
+     {?eh,tc_done,{ct_telnet_faulty_regexp_SUITE,expect_pattern_unicode,ok}},
+     {?eh,tc_done,{ct_telnet_faulty_regexp_SUITE,expect_tag_pattern_unicode,ok}},
+     {?eh,stop_logging,[]}];
 events_to_check(timetrap,_Config) ->
     [{?eh,start_logging,{'DEF','RUNDIR'}},
      {?eh,tc_done,{ct_telnet_timetrap_SUITE,expect_timetrap,
diff --git a/lib/common_test/test/ct_telnet_SUITE_data/ct_telnet_faulty_regexp_SUITE.erl b/lib/common_test/test/ct_telnet_SUITE_data/ct_telnet_faulty_regexp_SUITE.erl
new file mode 100644
index 0000000000..a5c9451a9c
--- /dev/null
+++ b/lib/common_test/test/ct_telnet_SUITE_data/ct_telnet_faulty_regexp_SUITE.erl
@@ -0,0 +1,79 @@
+-module(ct_telnet_faulty_regexp_SUITE).
+
+-compile(export_all).
+
+-include_lib("common_test/include/ct.hrl").
+
+-define(name, telnet_server_conn1).
+
+%%--------------------------------------------------------------------
+%% TEST SERVER CALLBACK FUNCTIONS
+%%--------------------------------------------------------------------
+
+init_per_suite(Config) ->
+    Config.
+
+end_per_suite(_Config) ->
+    ok.
+
+suite() -> [{require,?name,{unix,[telnet]}},
+	    {require,ct_conn_log},
+ 	    {ct_hooks, [{cth_conn_log,[]}]}].
+
+all() ->
+    [expect_pattern,
+     expect_pattern_no_string,
+     expect_tag_pattern,
+     expect_tag_pattern_no_string,
+     expect_pattern_unicode,
+     expect_tag_pattern_unicode].
+
+groups() ->
+    [].
+
+init_per_group(_GroupName, Config) ->
+    Config.
+
+end_per_group(_GroupName, Config) ->
+    Config.
+
+init_per_testcase(_,Config) ->
+    ct:log("init_per_testcase: opening telnet connection...",[]),
+    {ok,_} = ct_telnet:open(?name),
+    ct:log("...done",[]),
+    Config.
+
+end_per_testcase(_,_Config) ->
+    ct:log("end_per_testcase: closing telnet connection...",[]),
+    _ = ct_telnet:close(?name),
+    ct:log("...done",[]),
+    ok.
+
+expect_pattern(_) ->
+    ok = ct_telnet:send(?name, "echo ayt"),
+    ok = ct_telnet:expect(?name, "invalid(pattern").
+
+expect_pattern_no_string(_) ->
+    ok = ct_telnet:send(?name, "echo ayt"),
+    ok = ct_telnet:expect(?name, invalid_pattern).
+
+expect_tag_pattern(_) ->
+    ok = ct_telnet:send(?name, "echo ayt"),
+    ok = ct_telnet:expect(?name, {tag,"invalid(pattern"}).
+
+expect_tag_pattern_no_string(_) ->
+    ok = ct_telnet:send(?name, "echo ayt"),
+    ok = ct_telnet:expect(?name, {tag,invalid_pattern}).
+
+%% Test that a unicode pattern can be given without the testcase
+%% failing.  Do however notice that there is no real unicode support
+%% in ct_telnet yet, that is, the telnet binary mode is not supported.
+expect_pattern_unicode(_) ->
+    ok = ct_telnet:send(?name, "echo ayt"),
+    {error,{prompt,_}} = ct_telnet:expect(?name, "pattern_with_unicode_αβ"),
+    ok.
+
+expect_tag_pattern_unicode(_) ->
+    ok = ct_telnet:send(?name, "echo ayt"),
+    {error,{prompt,_}} = ct_telnet:expect(?name, "pattern_with_unicode_αβ"),
+    ok.
diff --git a/lib/common_test/test/ct_telnet_SUITE_data/ct_telnet_own_server_SUITE.erl b/lib/common_test/test/ct_telnet_SUITE_data/ct_telnet_own_server_SUITE.erl
index 985fa40ad2..34df57027e 100644
--- a/lib/common_test/test/ct_telnet_SUITE_data/ct_telnet_own_server_SUITE.erl
+++ b/lib/common_test/test/ct_telnet_SUITE_data/ct_telnet_own_server_SUITE.erl
@@ -58,7 +58,8 @@ all() ->
      server_speaks,
      server_disconnects,
      newline_ayt,
-     newline_break
+     newline_break,
+     newline_string
     ].
 
 groups() ->
@@ -393,3 +394,11 @@ newline_break(_) ->
     "> " = lists:flatten(R),
     ok = ct_telnet:close(Handle),
     ok.
+
+%% Test option {newline,String} to specify an own newline, e.g. "\r\n"
+newline_string(_) ->
+    {ok, Handle} = ct_telnet:open(telnet_server_conn1),
+    ok = ct_telnet:send(Handle, "echo hello-", [{newline,"own_nl\n"}]),
+    {ok,["hello-own_nl"]} = ct_telnet:expect(Handle, ["hello-own_nl"]),
+    ok = ct_telnet:close(Handle),
+    ok.