Commit a5f9b5f8d8c2dbcd1bec92065e5df7bae76d3c7c

Russell Belfer 2013-07-05T16:59:38

Diff hunk context off by one on long lines The diff hunk context string that is returned to xdiff need not be NUL terminated because the xdiff code just copies the number of bytes that you report directly into the output. There was an off by one in the diff driver code when the header context was longer than the output buffer size, the output buffer length included the NUL byte which was copied into the hunk header. Fixes #1710

diff --git a/src/diff_driver.c b/src/diff_driver.c
index 469be0d..77b0e9f 100644
--- a/src/diff_driver.c
+++ b/src/diff_driver.c
@@ -373,10 +373,11 @@ static long diff_context_find(
 		!ctxt->match_line(ctxt->driver, ctxt->line.ptr, ctxt->line.size))
 		return -1;
 
-	git_buf_truncate(&ctxt->line, (size_t)out_size);
-	git_buf_copy_cstr(out, (size_t)out_size, &ctxt->line);
+	if (out_size > (long)ctxt->line.size)
+		out_size = ctxt->line.size;
+	memcpy(out, ctxt->line.ptr, (size_t)out_size);
 
-	return (long)ctxt->line.size;
+	return out_size;
 }
 
 void git_diff_find_context_init(
diff --git a/tests-clar/diff/diff_helpers.c b/tests-clar/diff/diff_helpers.c
index 4e23792..a5c322b 100644
--- a/tests-clar/diff/diff_helpers.c
+++ b/tests-clar/diff/diff_helpers.c
@@ -70,8 +70,9 @@ int diff_hunk_cb(
 	diff_expects *e = payload;
 
 	GIT_UNUSED(delta);
-	GIT_UNUSED(header);
-	GIT_UNUSED(header_len);
+
+	/* confirm no NUL bytes in header text */
+	while (header_len--) cl_assert('\0' != *header++);
 
 	e->hunks++;
 	e->hunk_old_lines += range->old_lines;
diff --git a/tests-clar/diff/drivers.c b/tests-clar/diff/drivers.c
index 06ab2ff..e02dd5c 100644
--- a/tests-clar/diff/drivers.c
+++ b/tests-clar/diff/drivers.c
@@ -123,3 +123,34 @@ void test_diff_drivers__patterns(void)
 	git_tree_free(one);
 }
 
+void test_diff_drivers__long_lines(void)
+{
+	const char *base = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed non nisi ligula. Ut viverra enim sed lobortis suscipit.\nPhasellus eget erat odio. Praesent at est iaculis, ultricies augue vel, dignissim risus. Suspendisse at nisi quis turpis fringilla rutrum id sit amet nulla.\nNam eget dolor fermentum, aliquet nisl at, convallis tellus. Pellentesque rhoncus erat enim, id porttitor elit euismod quis.\nMauris sollicitudin magna odio, non egestas libero vehicula ut. Etiam et quam velit. Fusce eget libero rhoncus, ultricies felis sit amet, egestas purus.\nAliquam in semper tellus. Pellentesque adipiscing rutrum velit, quis malesuada lacus consequat eget.\n";
+	git_index *idx;
+	git_diff_list *diff;
+	git_diff_patch *patch;
+	char *actual;
+	const char *expected = "diff --git a/longlines.txt b/longlines.txt\nindex c1ce6ef..0134431 100644\n--- a/longlines.txt\n+++ b/longlines.txt\n@@ -3,3 +3,5 @@ Phasellus eget erat odio. Praesent at est iaculis, ultricies augue vel, dignissi\n Nam eget dolor fermentum, aliquet nisl at, convallis tellus. Pellentesque rhoncus erat enim, id porttitor elit euismod quis.\n Mauris sollicitudin magna odio, non egestas libero vehicula ut. Etiam et quam velit. Fusce eget libero rhoncus, ultricies felis sit amet, egestas purus.\n Aliquam in semper tellus. Pellentesque adipiscing rutrum velit, quis malesuada lacus consequat eget.\n+newline\n+newline\n";
+
+	g_repo = cl_git_sandbox_init("empty_standard_repo");
+
+	cl_git_mkfile("empty_standard_repo/longlines.txt", base);
+	cl_git_pass(git_repository_index(&idx, g_repo));
+	cl_git_pass(git_index_add_bypath(idx, "longlines.txt"));
+	cl_git_pass(git_index_write(idx));
+	git_index_free(idx);
+
+	cl_git_append2file("empty_standard_repo/longlines.txt", "newline\nnewline\n");
+
+	cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, NULL));
+	cl_assert_equal_sz(1, git_diff_num_deltas(diff));
+	cl_git_pass(git_diff_get_patch(&patch, NULL, diff, 0));
+	cl_git_pass(git_diff_patch_to_str(&actual, patch));
+
+	cl_assert_equal_s(expected, actual);
+
+	free(actual);
+	git_diff_patch_free(patch);
+	git_diff_list_free(diff);
+}
+