Merge branch 'lukas/erts/fix-erlexec-argument-check/OTP-17314/OTP-17315'

* lukas/erts/fix-erlexec-argument-check/OTP-17314/OTP-17315:
  kernel: Warn when invalid -(s)name is given
  otp: Fix make test PATH handling
  erlexec: Add checks that next argument is valid

4 files changed, 126 insertions, 75 deletions

diff --git a/erts/etc/common/erlexec.c b/erts/etc/common/erlexec.c
index 2a0def7f48..13a79aaf17 100644
--- a/erts/etc/common/erlexec.c
+++ b/erts/etc/common/erlexec.c
@@ -404,6 +404,13 @@ static void add_boot_config(void)
 # define ADD_BOOT_CONFIG
 #endif
 
+#define NEXT_ARG_CHECK_NAMED(Option) \
+    do {                                                                \
+        if (i+1 >= argc || strncmp(argv[i+1], "--", 3) == 0)            \
+            usage(Option);                                              \
+    } while(0)
+
+#define NEXT_ARG_CHECK() NEXT_ARG_CHECK_NAMED(argv[i])
 
 #ifdef __WIN32__
 __declspec(dllexport) int win_erlexec(int argc, char **argv, HANDLE module, int windowed)
@@ -513,15 +520,11 @@ int main(int argc, char **argv)
 	    } else if (strcmp(argv[i], "-extra") == 0) {
 		break;
 	    } else if (strcmp(argv[i], "-emu_type") == 0) {
-		if (i + 1 >= argc) {
-                    usage(argv[i]);
-                }
+                NEXT_ARG_CHECK();
                 emu_type = argv[i+1];
                 i++;
 	    } else if (strcmp(argv[i], "-emu_flavor") == 0) {
-		if (i + 1 >= argc) {
-                    usage(argv[i]);
-                }
+                NEXT_ARG_CHECK();
                 emu_flavor = argv[i+1];
                 i++;
 	    }
@@ -610,8 +613,7 @@ int main(int argc, char **argv)
 			    error("Conflicting -boot options");
                         if (got_start_erl)
                             error("Conflicting -start_erl and -boot options");
-			if (i+1 >= argc)
-			    usage("-boot");
+                        NEXT_ARG_CHECK();
 			boot_script = strsave(argv[i+1]);
 			i++;
 		    }
@@ -635,8 +637,7 @@ int main(int argc, char **argv)
 		    else if (strcmp(argv[i], "-config") == 0){
 			if (got_start_erl)
 			    error("Conflicting -start_erl and -config options");
-			if (i+1 >= argc)
-			    usage("-config");
+                        NEXT_ARG_CHECK();
                         do {
                             config_script_cnt++;
                             config_scripts = erealloc(config_scripts,
@@ -647,8 +648,7 @@ int main(int argc, char **argv)
 		    }
 #endif
                     else if (strcmp(argv[i], "-configfd") == 0) {
-                        if (i+1 >= argc)
- 			    usage("-configfd");
+                        NEXT_ARG_CHECK();
                         if ( strcmp(argv[i+1], "0") != 0 ) {
 			    add_arg(argv[i]);
                         } else {
@@ -685,13 +685,13 @@ int main(int argc, char **argv)
 		    } else if (strcmp(argv[i], "-emu_qouted_cmd_exit") == 0) {
 			print_qouted_cmd_exit = 1;
 		    } else if (strcmp(argv[i], "-env") == 0) { /* -env VARNAME VARVALUE */
-			if (i+2 >= argc)
-			    usage("-env");
-			set_env(argv[i+1], argv[i+2]);
-			i += 2;
+                        NEXT_ARG_CHECK();
+                        i += 1;
+                        NEXT_ARG_CHECK_NAMED("-env");
+			set_env(argv[i], argv[i+1]);
+			i += 1;
 		    } else if (strcmp(argv[i], "-epmd") == 0) {
-			if (i+1 >= argc)
-			    usage("-epmd");
+                        NEXT_ARG_CHECK();
 			epmd_prog = argv[i+1];
 			++i;
 		    } else {
@@ -731,8 +731,7 @@ int main(int argc, char **argv)
 
 		  case 'n':
 		    if (strcmp(argv[i], "-name") == 0) { /* -name NAME */
-			if (i+1 >= argc)
-			    usage("-name");
+                        NEXT_ARG_CHECK();
 
 			/*
 			 * Note: Cannot use add_args() here, due to non-defined
@@ -758,8 +757,7 @@ int main(int argc, char **argv)
 
 		  case 's':	/* -sname NAME */
 		    if (strcmp(argv[i], "-sname") == 0) {
-			if (i+1 >= argc)
-			    usage("-sname");
+                        NEXT_ARG_CHECK();
 			add_arg(argv[i]);
 			add_arg(argv[i+1]);
 			isdistributed = 1;
@@ -767,6 +765,7 @@ int main(int argc, char **argv)
 		    }
 #ifdef __WIN32__
 		    else if (strcmp(argv[i], "-service_event") == 0) {
+                        NEXT_ARG_CHECK();
 			add_arg(argv[i]);
 			add_arg(argv[i+1]);
 			i++;
@@ -780,8 +779,7 @@ int main(int argc, char **argv)
 		    }
 #endif
 		    else if (strcmp(argv[i], "-start_epmd") == 0) {
-			if (i+1 >= argc)
-			    usage("-start_epmd");
+                        NEXT_ARG_CHECK();
 
 			if (strcmp(argv[i+1], "true") == 0) {
 			    /* The default */
@@ -833,8 +831,7 @@ int main(int argc, char **argv)
 		  case 'K':
 		      if (argv[i][2] != '\0')
 			  goto the_default;
-		      if (i+1 >= argc)
-			  usage(argv[i]);
+                      NEXT_ARG_CHECK();
 		      argv[i][0] = '-';
 		      add_Eargs(argv[i]);
 		      add_Eargs(argv[i+1]);
@@ -844,6 +841,7 @@ int main(int argc, char **argv)
                       if (argv[i][2] == 'O' && (argv[i][3] == 't' || argv[i][3] == 'p')) {
                           if (argv[i][4] != '\0')
                               goto the_default;
+                          NEXT_ARG_CHECK();
                           argv[i][0] = '-';
                           add_Eargs(argv[i]);
                           add_Eargs(argv[i+1]);
@@ -854,6 +852,7 @@ int main(int argc, char **argv)
                           (argv[i][4] == 't' || argv[i][4] == 'p')) {
                           if (argv[i][5] != '\0')
                               goto the_default;
+                          NEXT_ARG_CHECK();
                           argv[i][0] = '-';
                           add_Eargs(argv[i]);
                           add_Eargs(argv[i+1]);
@@ -863,9 +862,7 @@ int main(int argc, char **argv)
                       usage(argv[i]);
                       break;
                   case 'J':
-                      if (i + 1 >= argc) {
-                          usage(argv[i]);
-                      }
+                      NEXT_ARG_CHECK();
                       argv[i][0] = '-';
                       add_Eargs(argv[i]);
                       add_Eargs(argv[i+1]);
@@ -889,8 +886,7 @@ int main(int argc, char **argv)
 		      }
 		      else if (argv[i][2] != '\0')
 			  goto the_default;
-		      if (i+1 >= argc)
-			  usage(argv[i]);
+                      NEXT_ARG_CHECK();
 		      argv[i][0] = '-';
 		      add_Eargs(argv[i]);
 		      add_Eargs(argv[i+1]);
@@ -909,17 +905,17 @@ int main(int argc, char **argv)
 			}
 		      }
 		      if (i+1 < argc) {
-			if ((argv[i+1][0] != '-') &&
-			    (argv[i+1][0] != '+')) {
-			  if (argv[i+1][0] == 'i') {
-			    add_Eargs(argv[i]);
-			    add_Eargs(argv[i+1]);
-			    i++;
-			    break;
-			  } else {
-			    usage(argv[i]);
+                          if (argv[i+1][1] == '\0') {
+                              if ((argv[i+1][0] == 'i') ||
+                                  (argv[i+1][0] == 'c') ||
+                                  (argv[i+1][0] == 'd')
+                                  ) {
+                                  add_Eargs(argv[i]);
+                                  add_Eargs(argv[i+1]);
+                                  i++;
+                                  break;
+                              }
 			  }
-			}
 		      }
 		      add_Eargs(argv[i]);
 		      break;
@@ -946,10 +942,7 @@ int main(int argc, char **argv)
 						plusM_au_alloc_switches))
 			  || is_one_of_strings(&argv[i][2],
 					       plusM_other_switches)) {
-			  if (i+1 >= argc
-			      || argv[i+1][0] == '-'
-			      || argv[i+1][0] == '+')
-			      usage(argv[i]);
+                          NEXT_ARG_CHECK();
 			  argv[i][0] = '-';
 			  add_Eargs(argv[i]);
 			  add_Eargs(argv[i+1]);
@@ -963,10 +956,7 @@ int main(int argc, char **argv)
 		      if (!is_one_of_strings(&argv[i][2], plush_val_switches)) {
 			  goto the_default;
 		      } else {
-			  if (i+1 >= argc
-			      || argv[i+1][0] == '-'
-			      || argv[i+1][0] == '+')
-			      usage(argv[i]);
+                          NEXT_ARG_CHECK();
 			  argv[i][0] = '-';
 			  add_Eargs(argv[i]);
 			  add_Eargs(argv[i+1]);
@@ -978,10 +968,7 @@ int main(int argc, char **argv)
 					     plusr_val_switches))
 			  goto the_default;
 		      else {
-			  if (i+1 >= argc
-			      || argv[i+1][0] == '-'
-			      || argv[i+1][0] == '+')
-			      usage(argv[i]);
+                          NEXT_ARG_CHECK();
 			  argv[i][0] = '-';
 			  add_Eargs(argv[i]);
 			  add_Eargs(argv[i+1]);
@@ -993,10 +980,7 @@ int main(int argc, char **argv)
 					     pluss_val_switches))
 			  goto the_default;
 		      else {
-			  if (i+1 >= argc
-			      || argv[i+1][0] == '-'
-			      || argv[i+1][0] == '+')
-			      usage(argv[i]);
+                          NEXT_ARG_CHECK();
 			  argv[i][0] = '-';
 			  add_Eargs(argv[i]);
 			  add_Eargs(argv[i+1]);
@@ -1006,8 +990,7 @@ int main(int argc, char **argv)
 		  case 'p':
 		      if (argv[i][2] != 'c' || argv[i][3] != '\0')
 			  goto the_default;
-		      if (i+1 >= argc)
-			  usage(argv[i]);
+                      NEXT_ARG_CHECK();
 		      argv[i][0] = '-';
 		      add_Eargs(argv[i]);
 		      add_Eargs(argv[i+1]);
@@ -1017,10 +1000,7 @@ int main(int argc, char **argv)
 		      if (!is_one_of_strings(&argv[i][2], plusz_val_switches)) {
 			  goto the_default;
 		      } else {
-			  if (i+1 >= argc
-			      || argv[i+1][0] == '-'
-			      || argv[i+1][0] == '+')
-			      usage(argv[i]);
+                          NEXT_ARG_CHECK();
 			  argv[i][0] = '-';
 			  add_Eargs(argv[i]);
 			  add_Eargs(argv[i+1]);
diff --git a/erts/test/erlexec_SUITE.erl b/erts/test/erlexec_SUITE.erl
index 5794f7e309..113f6e1ceb 100644
--- a/erts/test/erlexec_SUITE.erl
+++ b/erts/test/erlexec_SUITE.erl
@@ -27,14 +27,30 @@
 %%%-------------------------------------------------------------------
 -module(erlexec_SUITE).
 
--export([all/0, suite/0, init_per_testcase/2, end_per_testcase/2]).
+-export([all/0, suite/0, init_per_suite/1, end_per_suite/1,
+         init_per_testcase/2, end_per_testcase/2]).
 
 -export([args_file/1, evil_args_file/1, missing_args_file/1, env/1, args_file_env/1,
-         otp_7461/1, otp_7461_remote/1, argument_separation/1,
+         otp_7461/1, otp_7461_remote/1, argument_separation/1, argument_with_option/1,
          zdbbl_dist_buf_busy_limit/1]).
 
 -include_lib("common_test/include/ct.hrl").
 
+suite() ->
+    [{ct_hooks,[ts_install_cth]},
+     {timetrap, {minutes, 1}}].
+
+all() -> 
+    [args_file, evil_args_file, missing_args_file, env, args_file_env,
+     otp_7461, argument_separation, argument_with_option, zdbbl_dist_buf_busy_limit].
+
+init_per_suite(Config) ->
+    [{suite_erl_flags, save_env()} | Config].
+
+end_per_suite(Config) ->
+    SavedEnv = proplists:get_value(suite_erl_flags, Config),
+    restore_env(SavedEnv).
+
 init_per_testcase(Case, Config) ->
     SavedEnv = save_env(),
     [{testcase, Case},{erl_flags_env, SavedEnv}|Config].
@@ -45,14 +61,6 @@ end_per_testcase(_Case, Config) ->
     cleanup_nodes(),
     ok.
 
-suite() ->
-    [{ct_hooks,[ts_install_cth]},
-     {timetrap, {minutes, 1}}].
-
-all() -> 
-    [args_file, evil_args_file, missing_args_file, env, args_file_env,
-     otp_7461, argument_separation, zdbbl_dist_buf_busy_limit].
-
 %% Test that plain first argument does not
 %% destroy -home switch [OTP-8209] or interact with environments
 argument_separation(Config) when is_list(Config) ->
@@ -114,6 +122,66 @@ flush() ->
             ok
     end.
 
+%% Test that giving an invalid argument to an option that needs
+%% an argument will always fail.
+argument_with_option(Config) when is_list(Config) ->
+    ErlSingle = ["emu_flavor", "emu_type", "configfd", "epmd",
+                 "name", "sname", "start_epmd"] ++
+        case os:type() of
+            {win32,_} -> ["boot","config","service_event"];
+            _ -> []
+        end,
+
+    MissingCheck =
+        fun(CmdLine, Prefix, Postfix) ->
+                InvalidArg = emu_args("-s init stop " ++ Prefix ++ CmdLine ++ Postfix,
+                                      [return_output]),
+                case re:run(InvalidArg, "Missing argument\\(s\\) for '\\" ++ Prefix ++ CmdLine ++ "'.") of
+                    nomatch ->
+                        ct:log("Output: ~ts",[InvalidArg]),
+                        ct:fail("Failed to fail on ~ts",[Prefix ++ CmdLine ++ Postfix]);
+                    {match,_} ->
+                        ok
+                end
+        end,
+    
+    [begin
+         MissingCheck(CmdLine,"-",""),
+
+         %% We test what happens when ERL_FLAGS is set
+         os:putenv("ERL_FLAGS","-test"),
+         MissingCheck(CmdLine,"-", ""),
+         os:unsetenv("ERL_FLAGS")
+     end || CmdLine <- ErlSingle],
+
+    EmuSingle = ["a", "A", "C", "e", "i", "n", "P", "Q", "t",
+                 "T", "R", "W", "K", "IOt", "IOp", "IOPt", "IOPp",
+                 "J", "SP", "SDcpu", "SDPcpu", "SDio",
+                 "hms", "rg", "sbt", "zdbbl"],
+
+    [begin
+         MissingCheck(CmdLine,"+",""),
+
+         %% We test what happens when ERL_FLAGS is set
+         os:putenv("ERL_FLAGS","-test"),
+         MissingCheck(CmdLine,"+", ""),
+         os:unsetenv("ERL_FLAGS")
+     end || CmdLine <- EmuSingle],
+
+    ErlDouble = ["env"],
+    
+    [begin
+         MissingCheck(CmdLine,"-",""),
+         MissingCheck(CmdLine,"-"," a"),
+
+         %% We test what happens when ERL_FLAGS is set
+         os:putenv("ERL_FLAGS","-test"),
+         MissingCheck(CmdLine,"-", ""),
+         MissingCheck(CmdLine,"-", " a"),
+         os:unsetenv("ERL_FLAGS")
+     end || CmdLine <- ErlDouble],
+    ok.
+
 args_file(Config) when is_list(Config) ->
     AFN1 = privfile("1", Config),
     AFN2 = privfile("2", Config),
@@ -445,7 +513,7 @@ emu_args(CmdLineArgs, Opts) ->
     io:format("CmdLineArgs = ~ts, Opts = ~p~n", [CmdLineArgs, Opts]),
     {ok,[[Erl]]} = init:get_argument(progname),
     EmuCL = os:cmd(Erl ++ " -emu_qouted_cmd_exit " ++ CmdLineArgs),
-    ct:pal("EmuCL = ~ts", [EmuCL]),
+%    ct:pal("EmuCL = ~ts", [EmuCL]),
     case lists:member(return_output, Opts) of
         true ->
             EmuCL;
diff --git a/lib/kernel/src/erl_distribution.erl b/lib/kernel/src/erl_distribution.erl
index a0d55d62f6..38f493b7a6 100644
--- a/lib/kernel/src/erl_distribution.erl
+++ b/lib/kernel/src/erl_distribution.erl
@@ -109,6 +109,10 @@ do_start_link([{Arg,Flag}|T]) ->
             ?LOG_WARNING("Multiple -~p given to erl, using the first, ~p",
                          [Arg, Name]),
 	    start_link([list_to_atom(Name),Flag|ticktime()], true, net_sup);
+        {ok,[Invalid|_]} ->
+            ?LOG_ERROR("Invalid -~p given to erl, ~ts",
+                       [Arg, lists:join(" ",Invalid)]),
+	    do_start_link(T);
 	_ ->
 	    do_start_link(T)
     end;
diff --git a/make/test_target_script.sh b/make/test_target_script.sh
index f605efa120..cc18f06797 100755
--- a/make/test_target_script.sh
+++ b/make/test_target_script.sh
@@ -231,7 +231,6 @@ $CT_RUN -logdir $MAKE_TEST_CT_LOGS\
         -pa "$ERL_TOP/lib/common_test/test_server"\
         ${ARGS}\
         -erl_args\
-        -env "$PATH"\
         -env ERL_CRASH_DUMP "$MAKE_TEST_DIR/${APPLICATION}_erl_crash.dump"\
         -boot start_sasl\
         -sasl errlog_type error\