[PATCH] blame: prevent a segv when -L given start > EOF

blame would segv if given -L <lineno> with <lineno> past the end of the file.
While we're fixing the bug, add test cases for an invalid <start> when called
as -L <start>,<end> or -L<start>.

Signed-off-by: Jay Soffian <redacted>
---
bottom is start and top is end, which seems backwards to me, but alas, it is
what it is. :-)

 builtin-blame.c  |    2 +-
 t/t8003-blame.sh |    8 ++++++++
 2 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/builtin-blame.c b/builtin-blame.c
index 6408ec8..10f7eac 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c

@@ -2433,7 +2433,7 @@ parse_done:
 	if (top < 1)
 		top = lno;
 	bottom--;
-	if (lno < top)
+	if (lno < top || lno < bottom)
 		die("file %s has only %lu lines", path, lno);
 
 	ent = xcalloc(1, sizeof(*ent));

diff --git a/t/t8003-blame.sh b/t/t8003-blame.sh
index ad834f2..4a8db74 100755
--- a/t/t8003-blame.sh
+++ b/t/t8003-blame.sh

@@ -157,4 +157,12 @@ EOF
   git --no-pager blame $COMMIT -- uno >/dev/null
 '
 
+test_expect_success 'blame -L with invalid start' '
+	test_must_fail git blame -L5 tres 2>&1 | grep "has only 2 lines"
+'
+
+test_expect_success 'blame -L with invalid end' '
+	git blame -L1,5 tres 2>&1 | grep "has only 2 lines"
+'
+
 test_done

-- 
1.7.0.rc1.200.g9c1f9

On Mon, Feb 8, 2010 at 10:48 PM, Jay Soffian [off-list ref] wrote:

+test_expect_success 'blame -L with invalid start' '
+       test_must_fail git blame -L5 tres 2>&1 | grep "has only 2 lines"
+'

Sorry, please remove test_must_fail after applying. Otherwise correct.

j.

Jay Soffian schrieb:

+test_expect_success 'blame -L with invalid start' '
+	test_must_fail git blame -L5 tres 2>&1 | grep "has only 2 lines"

Please write this as

	test_must_fail git blame -L5 tres >output 2>&1 &&
	grep "has only 2 lines" output

+'
+
+test_expect_success 'blame -L with invalid end' '
+	git blame -L1,5 tres 2>&1 | grep "has only 2 lines"

	test_must_fail git blame -L1,5 tres >output 2>&1 &&
	grep "has only 2 lines" output

because shells look only at the exit code of the last command in a pipeline.

-- Hannes

On Tue, Feb 9, 2010 at 2:43 AM, Johannes Sixt [off-list ref] wrote:

Jay Soffian schrieb:

quoted

+test_expect_success 'blame -L with invalid start' '
+     test_must_fail git blame -L5 tres 2>&1 | grep "has only 2 lines"

Please write this as

       test_must_fail git blame -L5 tres >output 2>&1 &&
       grep "has only 2 lines" output

quoted

+'
+
+test_expect_success 'blame -L with invalid end' '
+     git blame -L1,5 tres 2>&1 | grep "has only 2 lines"

       test_must_fail git blame -L1,5 tres >output 2>&1 &&
       grep "has only 2 lines" output

because shells look only at the exit code of the last command in a pipeline.

Thanks, I knew that. I'd left in test_must_fail accidentally because
initially I wasn't bothering to grep the output. I then added the grep
and forgot to remove test_must_fail. Isn't this an adequate test:

  test_expect_success 'blame -L with invalid start' '
     git blame -L5 tres 2>&1 | grep "has only 2 lines"

As it seems unlikely git would crash and still output the message
correctly in this case.

?

j.

blame would segv if given -L <lineno> with <lineno> past the end of the file.
While we're fixing the bug, add test cases for an invalid <start> when called
as -L <start>,<end> or -L<start>.

Signed-off-by: Jay Soffian <redacted>
---
Modified the tests per Hannes recommendations.

 builtin-blame.c  |    2 +-
 t/t8003-blame.sh |   10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/builtin-blame.c b/builtin-blame.c
index 6408ec8..10f7eac 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c

@@ -2433,7 +2433,7 @@ parse_done:
 	if (top < 1)
 		top = lno;
 	bottom--;
-	if (lno < top)
+	if (lno < top || lno < bottom)
 		die("file %s has only %lu lines", path, lno);
 
 	ent = xcalloc(1, sizeof(*ent));

diff --git a/t/t8003-blame.sh b/t/t8003-blame.sh
index ad834f2..be648b2 100755
--- a/t/t8003-blame.sh
+++ b/t/t8003-blame.sh

@@ -157,4 +157,14 @@ EOF
   git --no-pager blame $COMMIT -- uno >/dev/null
 '
 
+test_expect_success 'blame -L with invalid start' '
+	test_must_fail git blame -L5 tres >output 2>&1 &&
+	grep "has only 2 lines" output
+'
+
+test_expect_success 'blame -L with invalid end' '
+	test_must_fail git blame -L1,5 tres >output 2>&1 &&
+	grep "has only 2 lines" output
+'
+
 test_done

-- 
1.7.0.rc1.200.g9c1f9