Re: [PATCH] More test cases for sanitized path names

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> I haven't looked at the code, but I suspect that "git add" and
> anything that uses the same logic as "ls-files --error-unmatch"
> would still not work with the setup patch.

Ok, I looked at the code.

I already had a fix for "ls-files --error-unmatch" in the
weatherbaloon patch.  The logic to complain and fail nonsense
paths was needed for "add".

The attached is on top of the previous one and your test case
updates.  We may want to also add tests for --error-unmatch.

It is very tempting to enhance pathspec API in such a way that
it is not just a NULL terminated array of (char*) pointers, but
a pointer to a richer structure that knows the number of
elements and which strings need to be freed (and we would invent
a "free_pathspec()" function to free them) and such, but that is
an independent surgery that is outside of the scope of flying
weatherbaloons.

---

 builtin-add.c    |   12 ++++++++++++
 t/t7010-setup.sh |   20 ++++++++++++++------
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 4a91e3e..820110e 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -228,6 +228,18 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		goto finish;
 	}
 
+	if (*argv) {
+		/* Was there an invalid path? */
+		if (pathspec) {
+			int num;
+			for (num = 0; pathspec[num]; num++)
+				; /* just counting */
+			if (argc != num)
+				exit(1); /* error message already given */
+		} else
+			exit(1); /* error message already given */
+	}
+
 	fill_directory(&dir, pathspec, ignored_too);
 
 	if (show_only) {
diff --git a/t/t7010-setup.sh b/t/t7010-setup.sh
index 60c4a46..e809e0e 100755
--- a/t/t7010-setup.sh
+++ b/t/t7010-setup.sh
@@ -135,20 +135,28 @@ test_expect_success 'blame using absolute path names' '
 	diff -u f1.txt f2.txt
 '
 
-test_expect_failure 'add a directory outside the work tree' '
+test_expect_success 'setup deeper work tree' '
+	test_create_repo tester
+'
+
+test_expect_success 'add a directory outside the work tree' '(
+	cd tester &&
 	d1="$(cd .. ; pwd)" &&
 	git add "$d1"
-	echo $?
-'
+)'
 
-test_expect_failure 'add a file outside the work tree, nasty case 1' '(
+test_expect_success 'add a file outside the work tree, nasty case 1' '(
+	cd tester &&
 	f="$(pwd)x" &&
+	echo "$f" &&
 	touch "$f" &&
 	git add "$f"
 )'
 
-test_expect_failure 'add a file outside the work tree, nasty case 2' '(
-	f="$(pwd|sed "s/.$//")x" &&
+test_expect_success 'add a file outside the work tree, nasty case 2' '(
+	cd tester &&
+	f="$(pwd | sed "s/.$//")x" &&
+	echo "$f" &&
 	touch "$f" &&
 	git add "$f"
 )'
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux