From 3c9d0414ed2db0167e6c828b547be8fc9f88fccc Mon Sep 17 00:00:00 2001 From: Greg Brockman Date: Tue, 20 Jul 2010 00:46:21 -0400 Subject: [PATCH 1/4] Check size of path buffer before writing into it This prevents a buffer overrun that could otherwise be triggered by creating a file called '.git' with contents gitdir: (something really long) Signed-off-by: Greg Brockman Signed-off-by: Junio C Hamano --- setup.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/setup.c b/setup.c index 0e4cfe603f..3bb046118c 100644 --- a/setup.c +++ b/setup.c @@ -170,6 +170,8 @@ static int is_git_directory(const char *suspect) char path[PATH_MAX]; size_t len = strlen(suspect); + if (PATH_MAX <= len + strlen("/objects")) + die("Too long path: %.*s", 60, suspect); strcpy(path, suspect); if (getenv(DB_ENVIRONMENT)) { if (access(getenv(DB_ENVIRONMENT), X_OK)) From 971ecbd1f8d1468951274d01103f80dd7d32d110 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Tue, 20 Jul 2010 12:17:12 -0500 Subject: [PATCH 2/4] t/README: clarify test_must_fail description Some have found the wording of the description to be somewhat ambiguous with respect to when it is desirable to use test_must_fail instead of "! ". Tweak the wording somewhat to hopefully clarify that it is _because_ test_must_fail can detect segmentation fault that it is desirable to use it instead of "! ". Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- t/README | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/t/README b/t/README index 0e4e8d8862..fecb76e599 100644 --- a/t/README +++ b/t/README @@ -275,6 +275,14 @@ library for your script to use. Merges the given rev using the given message. Like test_commit, creates a tag and calls test_tick before committing. + - test_must_fail + + Run a git command and ensure it fails in a controlled way. Use + this instead of "! ". When git-command dies due to a + segfault, test_must_fail diagnoses it as an error; "! " + treats it as just another expected failure, which would let such a + bug go unnoticed. + Tips for Writing Tests ---------------------- From 460d562eab00070dfeb54f6ecbf803ad14aed4a5 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Tue, 20 Jul 2010 10:24:47 -0500 Subject: [PATCH 3/4] t/t3700: convert two uses of negation operator '!' to use test_must_fail These two lines use the negation '!' operator to negate the result of a simple command. Since these commands do not contain any pipes or other complexities, the test_must_fail function can be used and is preferred since it will additionally detect termination due to a signal. This was noticed because the second use of '!' does not include a space between the '!' and the opening parens. Ksh interprets this as follows: !(pattern-list) Matches anything except one of the given patterns. Ksh performs a file glob using the pattern-list and then tries to execute the first file in the list. If a space is added between the '!' and the open parens, then Ksh will not interpret it as a pattern list, but in this case, it is preferred to use test_must_fail, so lets do so. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- t/t3700-add.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t3700-add.sh b/t/t3700-add.sh index 47fbf5362f..d03495dc7a 100755 --- a/t/t3700-add.sh +++ b/t/t3700-add.sh @@ -268,7 +268,7 @@ test_expect_success 'git add --dry-run of existing changed file' " test_expect_success 'git add --dry-run of non-existing file' " echo ignored-file >>.gitignore && - ! (git add --dry-run track-this ignored-file >actual 2>&1) && + test_must_fail git add --dry-run track-this ignored-file >actual 2>&1 && echo \"fatal: pathspec 'ignored-file' did not match any files\" | test_cmp - actual " @@ -281,7 +281,7 @@ add 'track-this' EOF test_expect_success 'git add --dry-run --ignore-missing of non-existing file' ' - !(git add --dry-run --ignore-missing track-this ignored-file >actual 2>&1) && + test_must_fail git add --dry-run --ignore-missing track-this ignored-file >actual 2>&1 && test_cmp expect actual ' From 77b5be2abab69a69618dd40a6ca8b754388b8216 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Tue, 20 Jul 2010 13:27:55 -0500 Subject: [PATCH 4/4] t/{t5541,lib-httpd}: replace problematic '!()' notation with test_must_fail The '!()' notation is interpreted as a pattern-list on Ksh. The Ksh man page describe it as follows: !(pattern-list) Matches anything except one of the given patterns. Ksh performs a file glob using the pattern-list and then tries to execute the first file in the list. If a space is added between the '!' and the open parens, then Ksh will not interpret it as a pattern list, but in this case, it is preferred to use test_must_fail, so lets do so. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- t/lib-httpd.sh | 2 +- t/t5541-http-push.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh index 71effc5bec..e733f6516f 100644 --- a/t/lib-httpd.sh +++ b/t/lib-httpd.sh @@ -145,7 +145,7 @@ test_http_push_nonff() { echo "changed" > path2 && git commit -a -m path2 --amend && - !(git push -v origin >output 2>&1) && + test_must_fail git push -v origin >output 2>&1 && (cd "$REMOTE_REPO" && test $HEAD = $(git rev-parse --verify HEAD)) ' diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh index 504884b9f8..b0c2a2c3ae 100755 --- a/t/t5541-http-push.sh +++ b/t/t5541-http-push.sh @@ -128,7 +128,7 @@ test_expect_success 'push fails for non-fast-forward refs unmatched by remote he # push master too; this ensures there is at least one '"'push'"' command to # the remote helper and triggers interaction with the helper. - !(git push -v origin +master master:retsam >output 2>&1) && + test_must_fail git push -v origin +master master:retsam >output 2>&1 && grep "^ + [a-f0-9]*\.\.\.[a-f0-9]* *master -> master (forced update)$" output && grep "^ ! \[rejected\] *master -> retsam (non-fast-forward)$" output &&