4857 xargs(1) -n and -I combine to do potentially the wrong thing

Reviewed by: Robert Mustacchi <rm@joyent.com>
Approved by: Dan McDonald <danmcd@omniti.com>

6 files changed, 344 insertions, 16 deletions

diff --git a/usr/src/cmd/xargs/xargs.c b/usr/src/cmd/xargs/xargs.c
index e50c7d6d45..2e568d54ba 100644
--- a/usr/src/cmd/xargs/xargs.c
+++ b/usr/src/cmd/xargs/xargs.c
@@ -19,7 +19,8 @@
  * CDDL HEADER END
  */
 /*
- * Copyright 2012 DEY Storage Systems, Inc.  All rights reserved.
+ * Copyright 2014 Garrett D'Amore <garrett@damore.org>
+ * Copyright 2012 DEY Storage Systems, Inc.
  *
  * Portions of this file developed by DEY Storage Systems, Inc. are licensed
  * under the terms of the Common Development and Distribution License (CDDL)
@@ -115,6 +116,8 @@ static int	N_lines = 0;
 static int	DASHX = FALSE;
 static int	MORE = TRUE;
 static int	PER_LINE = FALSE;
+static int	LINE_CONT = FALSE;
+static int	EAT_LEAD = FALSE;
 static int	ERR = FALSE;
 static int	OK = TRUE;
 static int	LEGAL = FALSE;
@@ -211,7 +214,8 @@ main(int argc, char **argv)
 
 		case 'I':
 			/* -I replstr: Insert mode. replstr *is* required. */
-			INSERT = PER_LINE = LEGAL = TRUE;
+			INSERT = PER_LINE = LEGAL = EAT_LEAD = TRUE;
+			LINE_CONT = FALSE;
 			N_ARGS = 0;
 			INSPAT = optarg;
 			if (*optarg == '\0') {
@@ -232,7 +236,8 @@ main(int argc, char **argv)
 			 * parse this by hand:
 			 */
 
-			INSERT = PER_LINE = LEGAL = TRUE;
+			INSERT = PER_LINE = LEGAL = EAT_LEAD = TRUE;
+			LINE_CONT = FALSE;
 			N_ARGS = 0;
 			if ((optarg != NULL) && (*optarg != '\0')) {
 				INSPAT = optarg;
@@ -254,9 +259,9 @@ main(int argc, char **argv)
 			 * -L number: # of times cmd is executed
 			 * number *is* required here:
 			 */
-			PER_LINE = TRUE;
+			PER_LINE = LINE_CONT = TRUE;
 			N_ARGS = 0;
-			INSERT = FALSE;
+			INSERT = EAT_LEAD = FALSE;
 			if ((PER_LINE = atoi(optarg)) <= 0) {
 				ermsg(_("#lines must be positive int: %s\n"),
 				    optarg);
@@ -272,9 +277,9 @@ main(int argc, char **argv)
 			 * parseargs handles the optional arg processing.
 			 */
 
-			PER_LINE = LEGAL = TRUE;  /* initialization	*/
+			PER_LINE = LINE_CONT = LEGAL = TRUE;
 			N_ARGS = 0;
-			INSERT = FALSE;
+			INSERT = EAT_LEAD = FALSE;
 
 			if ((optarg != NULL) && (*optarg != '\0')) {
 				if ((PER_LINE = atoi(optarg)) <= 0)
@@ -292,7 +297,7 @@ main(int argc, char **argv)
 				    optarg);
 			} else {
 				LEGAL = DASHX || N_ARGS == 1;
-				INSERT = PER_LINE = FALSE;
+				INSERT = PER_LINE = LINE_CONT = FALSE;
 			}
 			break;
 
@@ -421,8 +426,8 @@ main(int argc, char **argv)
 
 			N_args++;
 
-			if ((PER_LINE && N_lines >= PER_LINE) ||
-			    (N_ARGS && (N_args) >= N_ARGS)) {
+			if ((PER_LINE && (N_lines >= PER_LINE)) ||
+			    (N_ARGS && (N_args >= N_ARGS))) {
 				break;
 			}
 
@@ -520,17 +525,19 @@ static char *
 getarg(char *arg)
 {
 	char	*xarg = arg;
-	wchar_t	c;
+	wchar_t	c = 0;
 	char	mbc[MB_LEN_MAX];
 	size_t	len;
 	int	escape = 0;
 	int	inquote = 0;
+	int	last = 0;
 
 	arg[0] = '\0';
 
 	while (MORE) {
 
 		len = 0;
+		last = c;
 		c = getwchr(mbc, &len);
 
 		if (((arg - xarg) + len) > BUFLIM) {
@@ -546,6 +553,16 @@ getarg(char *arg)
 				store_str(&arg, mbc, len);
 				continue;
 			}
+			/*
+			 * NB: Some other versions rip off all of the trailing
+			 * blanks.  The spec only claims that this should
+			 * be done for a single blank.  We follow the spec.
+			 */
+			if (LINE_CONT && iswctype(last, blank)) {
+				len = 0;
+				*arg = 0;
+				continue;
+			}
 			/* FALLTHRU */
 
 		case '\0':
@@ -617,7 +634,16 @@ getarg(char *arg)
 				store_str(&arg, mbc, len);
 				continue;
 			}
-			/* unquoted blank */
+			if (EAT_LEAD && last == 0) {
+				c = 0;		/* Roll it back */
+				continue;
+			}
+			if (PER_LINE) {
+				store_str(&arg, mbc, len);
+				continue;
+			}
+
+			/* unquoted blank without special handling */
 			break;
 		}
 
@@ -729,7 +755,7 @@ insert(char *pattern, char *subst)
 	bufend = &buffer[MAXSBUF];
 
 	while (*++pat) {
-		if (strncmp(pat, INSPAT, ipatlen) == 0) {
+		if (strncmp(pat, INSPAT, ipatlen + 1) == 0) {
 			if (pbuf + len >= bufend) {
 				break;
 			} else {
@@ -904,8 +930,8 @@ usage()
  *	-Estr	-> "-E "str"
  *	-i	-> "-i "{}"
  *	-irep	-> "-i "rep"
- *	-l	-> "-i "1"
- *	-l10	-> "-i "10"
+ *	-l	-> "-l "1"
+ *	-l10	-> "-l "10"
  *
  *	since the -e, -i and -l flags all take optional subarguments,
  */
diff --git a/usr/src/pkg/manifests/system-test-utiltest.mf b/usr/src/pkg/manifests/system-test-utiltest.mf
index 54ec7dcc29..5ccc2647d1 100644
--- a/usr/src/pkg/manifests/system-test-utiltest.mf
+++ b/usr/src/pkg/manifests/system-test-utiltest.mf
@@ -28,5 +28,6 @@ file path=opt/util-tests/README mode=0444
 file path=opt/util-tests/bin/utiltest mode=0555
 file path=opt/util-tests/runfiles/default.run mode=0444
 file path=opt/util-tests/tests/printf_test mode=0555
+file path=opt/util-tests/tests/xargs_test mode=0555
 license lic_CDDL license=lic_CDDL
 depend fmri=system/test/testrunner type=require
diff --git a/usr/src/test/util-tests/runfiles/default.run b/usr/src/test/util-tests/runfiles/default.run
index 638ba93ce1..9450ab7508 100644
--- a/usr/src/test/util-tests/runfiles/default.run
+++ b/usr/src/test/util-tests/runfiles/default.run
@@ -24,3 +24,4 @@ outputdir = /var/tmp/test_results
 
 [/opt/util-tests/tests/printf_test]
 
+[/opt/util-tests/tests/xargs_test]
diff --git a/usr/src/test/util-tests/tests/Makefile b/usr/src/test/util-tests/tests/Makefile
index 41307ba8e4..503f5791e4 100644
--- a/usr/src/test/util-tests/tests/Makefile
+++ b/usr/src/test/util-tests/tests/Makefile
@@ -14,6 +14,6 @@
 # Copyright 2014 Garrett D'Amore <garrett@damore.org>
 #
 
-SUBDIRS = printf
+SUBDIRS = printf xargs
 
 include $(SRC)/test/Makefile.com
diff --git a/usr/src/test/util-tests/tests/xargs/Makefile b/usr/src/test/util-tests/tests/xargs/Makefile
new file mode 100644
index 0000000000..cf89660f75
--- /dev/null
+++ b/usr/src/test/util-tests/tests/xargs/Makefile
@@ -0,0 +1,38 @@
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# http://www.illumos.org/license/CDDL.
+#
+
+#
+# Copyright (c) 2012 by Delphix. All rights reserved.
+# Copyright 2014 Garrett D'Amore <garrett@damore.org>
+#
+
+include $(SRC)/cmd/Makefile.cmd
+include $(SRC)/test/Makefile.com
+
+PROG = xargs_test
+
+ROOTOPTPKG = $(ROOT)/opt/util-tests
+TESTDIR = $(ROOTOPTPKG)/tests
+
+CMDS = $(PROG:%=$(TESTDIR)/%)
+$(CMDS) := FILEMODE = 0555
+
+all lint clean clobber:
+
+install: all $(CMDS)
+
+$(CMDS): $(TESTDIR) $(PROG).ksh
+
+$(TESTDIR):
+	$(INS.dir)
+
+$(TESTDIR)/%: %.ksh
+	$(INS.rename)
diff --git a/usr/src/test/util-tests/tests/xargs/xargs_test.ksh b/usr/src/test/util-tests/tests/xargs/xargs_test.ksh
new file mode 100644
index 0000000000..2d6f76ce10
--- /dev/null
+++ b/usr/src/test/util-tests/tests/xargs/xargs_test.ksh
@@ -0,0 +1,262 @@
+#! /usr/bin/ksh
+#
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# http://www.illumos.org/license/CDDL.
+#
+
+#
+# Copyright 2014 Garrett D'Amore <garrett@damore.org>
+#
+
+XARGS=${XARGS:=/usr/bin/xargs}
+
+test_start() {
+	print "TEST STARTING ${1}: ${2}"
+}
+
+test_pass() {
+	print "TEST PASS: ${1}"
+}
+
+test_fail() {
+	print "TEST FAIL: ${1}: ${2}"
+	exit -1
+}
+
+checkrv() {
+	if [[ $? -ne 0 ]]; then
+		test_fail $1 "exit failure"
+	fi
+}
+
+compare() {
+	if [[ "$2" != "$3" ]]; then
+		test_fail $1 "compare mismatch, got [$2] expected [$3]"
+	fi
+}
+
+test1() {
+	t=test1
+	test_start $t "-I handling"
+	comp=$(echo foo bar baz other | $XARGS -I THING echo '** THING **')
+	checkrv $t
+	good='** foo bar baz other **'
+	compare $t "$comp" "$good"
+	test_pass $t
+}
+
+test2() {
+	t=test2
+	test_start $t "-n 1 handling"
+	comp=$(echo foo bar baz other | $XARGS -n 1 echo '***')
+	checkrv $t
+	good='*** foo
+*** bar
+*** baz
+*** other'
+	compare $t "$comp" "$good"
+	test_pass $t
+}
+
+test3() {
+	t=test3
+	test_start $t "-I before -n 1"
+	comp=$(echo foo bar baz other | $XARGS -I THING -n1 echo '** THING **')
+	checkrv $t
+	good='** THING ** foo
+** THING ** bar
+** THING ** baz
+** THING ** other'
+	compare $t "$comp" "$good"
+	test_pass $t
+}
+
+test4() {
+	t=test4
+	test_start $t "-n 1  before -I"
+	comp=$(echo foo bar baz other | $XARGS -n 1 -I THING echo '** THING **')
+	checkrv $t
+	good='** foo bar baz other **'
+	compare $t "$comp" "$good"
+	test_pass $t
+}
+
+test5() {
+	t=test5
+	test_start $t "-i multiple lines handling"
+	comp=$(printf "abc def\nxyz\n123" | $XARGS -n1 -i echo '[{}]')
+	checkrv $t
+	good='[abc def]
+[xyz]
+[123]'
+	compare $t "$comp" "$good"
+	test_pass $t
+}
+
+test6() {
+	t=test6
+	test_start $t "-E handling"
+	comp=$(printf "abc def xyx\n_\n123\nnope" | $XARGS -edef echo)
+	checkrv $t
+	good='abc'
+	compare $t "$comp" "$good"
+	test_pass $t
+}
+
+test7() {
+	t=test7
+	test_start $t "newlines in arguments"
+	comp=$(printf "abc def\nxyz\n\n123 456\n789" | $XARGS echo)
+	checkrv $t
+	good='abc def xyz 123 456 789'
+	compare $t "$comp" "$good"
+	test_pass $t
+}
+
+test8() {
+	t=test8
+	test_start $t "limited counts via -n3"
+	comp=$(printf "abc def ghi jkl mno 123 456 789" | $XARGS -n 3 echo '**' )
+	checkrv $t
+	good='** abc def ghi
+** jkl mno 123
+** 456 789'
+	compare $t "$comp" "$good"
+	test_pass $t
+}
+
+test9() {
+	t=test9
+	test_start $t "multiple lines via -L2"
+	comp=$(printf "abc def\n123 456\npeterpiper" | $XARGS -L2 echo '**')
+	checkrv $t
+	good='** abc def 123 456
+** peterpiper'
+	compare $t "$comp" "$good"
+	test_pass $t
+}
+
+test10() {
+	t=test10
+	test_start $t "argument sizes"
+	comp=$(printf "abc def 123 456 peter alpha\n" | $XARGS -s15 echo)
+	checkrv $t
+	good='abc def
+123 456
+peter
+alpha'
+	compare $t "$comp" "$good"
+	test_pass $t
+}
+
+test11() {
+	t=test11
+	test_start $t "bare -e"
+	comp=$(printf "abc def _ end of string" | $XARGS -e echo '**')
+	checkrv $t
+	good='** abc def _ end of string'
+	compare $t "$comp" "$good"
+	test_pass $t
+}
+
+test12() {
+	t=test12
+	test_start $t "-E ''"
+	comp=$(printf "abc def _ end of string" | $XARGS -E '' echo '**')
+	checkrv $t
+	good='** abc def _ end of string'
+	compare $t "$comp" "$good"
+	test_pass $t
+}
+
+test13() {
+	t=test13
+	test_start $t "end of string (no -E or -e)"
+	comp=$(printf "abc def _ end of string" | $XARGS echo '**')
+	checkrv $t
+	good='** abc def'
+	compare $t "$comp" "$good"
+	test_pass $t
+}
+
+test14() {
+	t=test14
+	test_start $t "trailing blank with -L"
+	comp=$(printf "abc def \n123 456\npeter\nbogus" | $XARGS -L2 echo '**')
+	checkrv $t
+	good='** abc def 123 456 peter
+** bogus'
+	compare $t "$comp" "$good"
+	test_pass $t
+}
+
+test15() {
+	t=test15
+	test_start $t "leading and embedded blanks with -i"
+	comp=$(printf "abc def\n  xyz  bogus\nnext" | $XARGS -i echo '** {}')
+	checkrv $t
+	good='** abc def
+** xyz  bogus
+** next'
+	compare $t "$comp" "$good"
+	test_pass $t
+}
+
+test16() {
+	t=test16
+	test_start $t "single character replstring"
+	comp=$(echo foo bar baz other | $XARGS -I X echo '** X **')
+	checkrv $t
+	good='** foo bar baz other **'
+	compare $t "$comp" "$good"
+	test_pass $t
+}
+
+test17() {
+	t=test17
+	test_start $t "null byte separators"
+	comp=$(print 'foo bar baz\000more data' | $XARGS -n1 -0 echo '**')
+	checkrv $t
+	good='** foo bar baz
+** more data'
+	compare $t "$comp" "$good"
+	test_pass $t
+}
+
+test18() {
+	t=test18
+	test_start $t "escape characters"
+	comp=$(printf 'foo\\ bar second" "arg third' | $XARGS -n1 echo '**')
+	checkrv $t
+	good='** foo bar
+** second arg
+** third'
+	compare $t "$comp" "$good"
+	test_pass $t
+}
+
+test1
+test2
+test3
+test4
+test5
+test6
+test7
+test8
+test9
+test10
+test11
+test12
+test13
+test14
+test15
+test16
+test17
+test18