test: fix mishandling of "test '(' = '('" and similar

function                                             old     new   delta
test_main                                            246     350    +104

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

2 files changed, 55 insertions, 34 deletions

diff --git a/coreutils/test.c b/coreutils/test.c
index 4df505a..88cc550 100644
--- a/coreutils/test.c
+++ b/coreutils/test.c
@@ -826,7 +826,6 @@ int test_main(int argc, char **argv)
 {
 	int res;
 	const char *arg0;
-//	bool negate = 0;
 
 	arg0 = bb_basename(argv[0]);
 	if (arg0[0] == '[') {
@@ -844,6 +843,7 @@ int test_main(int argc, char **argv)
 		}
 		argv[argc] = NULL;
 	}
+	/* argc is unused after this point */
 
 	/* We must do DEINIT_S() prior to returning */
 	INIT_S();
@@ -862,43 +862,45 @@ int test_main(int argc, char **argv)
 	 */
 	/*ngroups = 0; - done by INIT_S() */
 
-	//argc--;
 	argv++;
+	args = argv;
 
-	/* Implement special cases from POSIX.2, section 4.62.4 */
-	if (!argv[0]) { /* "test" */
-		res = 1;
-		goto ret;
-	}
-#if 0
-// Now it's fixed in the parser and should not be needed
-	if (LONE_CHAR(argv[0], '!') && argv[1]) {
-		negate = 1;
-		//argc--;
-		argv++;
-	}
-	if (!argv[1]) { /* "test [!] arg" */
-		res = (*argv[0] == '\0');
-		goto ret;
-	}
-	if (argv[2] && !argv[3]) {
-		check_operator(argv[1]);
-		if (last_operator->op_type == BINOP) {
-			/* "test [!] arg1 <binary_op> arg2" */
-			args = argv;
-			res = (binop() == 0);
-			goto ret;
+	/* Implement special cases from POSIX.2, section 4.62.4.
+	 * Testcase: "test '(' = '('"
+	 * The general parser would misinterpret '(' as group start.
+	 */
+	if (1) {
+		int negate = 0;
+ again:
+		if (!argv[0]) {
+			/* "test" */
+			res = 1;
+			goto ret_special;
+		}
+		if (!argv[1]) {
+			/* "test [!] arg" */
+			res = (argv[0][0] == '\0');
+			goto ret_special;
+		}
+		if (argv[2] && !argv[3]) {
+			check_operator(argv[1]);
+			if (last_operator->op_type == BINOP) {
+				/* "test [!] arg1 <binary_op> arg2" */
+				args = argv;
+				res = (binop() == 0);
+ ret_special:
+				/* If there was leading "!" op... */
+				res ^= negate;
+				goto ret;
+			}
+		}
+		if (LONE_CHAR(argv[0], '!')) {
+			argv++;
+			negate ^= 1;
+			goto again;
 		}
 	}
 
-	/* Some complex expression. Undo '!' removal */
-	if (negate) {
-		negate = 0;
-		//argc++;
-		argv--;
-	}
-#endif
-	args = argv;
 	res = !oexpr(check_operator(*args));
 
 	if (*args != NULL && *++args != NULL) {
@@ -911,6 +913,5 @@ int test_main(int argc, char **argv)
 	}
  ret:
 	DEINIT_S();
-//	return negate ? !res : res;
 	return res;
 }
diff --git a/testsuite/test.tests b/testsuite/test.tests
index 2c92e34..1c2edaf 100755
--- a/testsuite/test.tests
+++ b/testsuite/test.tests
@@ -76,4 +76,24 @@ testing "test ! a = b -a ! c = d: should be true (0)" \
 	"0\n" \
 	"" ""
 
+testing "test '!' = '!': should be true (0)" \
+	"busybox test '!' = '!'; echo \$?" \
+	"0\n" \
+	"" ""
+
+testing "test '(' = '(': should be true (0)" \
+	"busybox test '(' = '('; echo \$?" \
+	"0\n" \
+	"" ""
+
+testing "test '!' '!' = '!': should be false (1)" \
+	"busybox test '!' '!' = '!'; echo \$?" \
+	"1\n" \
+	"" ""
+
+testing "test '!' '(' = '(': should be false (1)" \
+	"busybox test '!' '(' = '('; echo \$?" \
+	"1\n" \
+	"" ""
+
 exit $FAILCOUNT