[PATCH 04/11] blame: fix -L bounds checking bug

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jul 31, 2013, 08:15 UTC
Message-ID: <1375258545-42240-5-git-send-email-sunshine@sunshineco.com>
In-Reply-To: <1375258545-42240-1-git-send-email-sunshine@sunshineco.com>

Since inception, -LX,Y has correctly reported an out-of-range error when Y is beyond end of file, however, X was not checked, and an out-of-range X would cause a crash. 92f9e273 (blame: prevent a segv when -L given start > EOF; 2010-02-08) attempted to rectify this shortcoming but has its own off-by-one error which allows X to extend one line past end of file. For example, given a file with 5 lines:

  git blame -L5 foo  # OK, blames line 5
  git blame -L6 foo  # accepted, no error, no output, huh?
  git blame -L7 foo  # error "fatal: file foo has only 5 lines"

Fix this bug.

In order to avoid regressing "blame foo" when foo is an empty file, the fix is slightly more complicated than changing '<' to '<='.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 builtin/blame.c     | 4 ++--
 t/annotate-tests.sh | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 079dcd3..e70b089 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2495,13 +2495,13 @@ parse_done:
 	bottom = top = 0;
 	if (bottomtop)
 		prepare_blame_range(&sb, bottomtop, lno, &bottom, &top);
+	if (lno < top || ((lno || bottom) && lno < bottom))
+		die("file %s has only %lu lines", path, lno);
 	if (bottom < 1)
 		bottom = 1;
 	if (top < 1)
 		top = lno;
 	bottom--;
-	if (lno < top || lno < bottom)
-		die("file %s has only %lu lines", path, lno);
 
 	ent = xcalloc(1, sizeof(*ent));
 	ent->lno = bottom;
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index f67332c..f117ef0 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -232,7 +232,7 @@ test_expect_success 'blame -L X (X == nlines)' '
 	check_count -L$n C 1
 '
 
-test_expect_failure 'blame -L X (X == nlines + 1)' '
+test_expect_success 'blame -L X (X == nlines + 1)' '
 	n=$(expr $(wc -l <file) + 2) &&
 	test_must_fail $PROG -L$n file
 '
@@ -321,7 +321,7 @@ test_expect_success 'blame -L 0 empty (undocumented)' '
 	check_count -h HEAD^^ -f incremental -L0
 '
 
-test_expect_failure 'blame -L 1 empty' '
+test_expect_success 'blame -L 1 empty' '
 	test_must_fail $PROG -L1 incremental HEAD^^
 '
 
@@ -341,7 +341,7 @@ test_expect_success 'blame -L 1 half' '
 	check_count -h HEAD^ -f incremental -L1 I 1
 '
 
-test_expect_failure 'blame -L 2 half' '
+test_expect_success 'blame -L 2 half' '
 	test_must_fail $PROG -L2 incremental HEAD^
 '
 
@@ -361,7 +361,7 @@ test_expect_success 'blame -L 1 full' '
 	check_count -f incremental -L1 I 1
 '
 
-test_expect_failure 'blame -L 2 full' '
+test_expect_success 'blame -L 2 full' '
 	test_must_fail $PROG -L2 incremental
 '

-- 
1.8.3.4.1120.gc240c48