patch_parse: ensure valid patch output with EOFNL

Author: Erik Aigner

src/apply.c

@@ -199,14 +199,21 @@ static int apply_hunk(
 
 	for (i = 0; i < hunk->line_count; i++) {
 		size_t linenum = hunk->line_start + i;
-		git_diff_line *line = git_array_get(patch->lines, linenum);
+		git_diff_line *line = git_array_get(patch->lines, linenum), *prev;
 
 		if (!line) {
 			error = apply_err("preimage does not contain line %"PRIuZ, linenum);
 			goto done;
 		}
 
 		switch (line->origin) {
+			case GIT_DIFF_LINE_CONTEXT_EOFNL:
+			case GIT_DIFF_LINE_DEL_EOFNL:
+			case GIT_DIFF_LINE_ADD_EOFNL:
+				prev = i ? git_array_get(patch->lines, i - 1) : NULL;
+				if (prev && prev->content[prev->content_len - 1] == '\n')
+					prev->content_len -= 1;
+				break;
 			case GIT_DIFF_LINE_CONTEXT:
 				if ((error = git_vector_insert(&preimage.lines, line)) < 0 ||
 				    (error = git_vector_insert(&postimage.lines, line)) < 0)

src/patch_generate.c

@@ -836,7 +836,7 @@ static int patch_generated_line_cb(
 {
 	git_patch_generated  *patch = payload;
 	git_patch_hunk *hunk;
-	git_diff_line   *line;
+	git_diff_line *line;
 
 	GIT_UNUSED(delta);
 	GIT_UNUSED(hunk_);

src/patch_parse.c

@@ -524,6 +524,14 @@ static int parse_hunk_header(
 	return -1;
 }
 
+static int eof_for_origin(int origin) {
+	if (origin == GIT_DIFF_LINE_ADDITION)
+		return GIT_DIFF_LINE_ADD_EOFNL;
+	if (origin == GIT_DIFF_LINE_DELETION)
+		return GIT_DIFF_LINE_DEL_EOFNL;
+	return GIT_DIFF_LINE_CONTEXT_EOFNL;
+}
+
 static int parse_hunk_body(
 	git_patch_parsed *patch,
 	git_patch_hunk *hunk,
@@ -534,6 +542,7 @@ static int parse_hunk_body(
 
 	int oldlines = hunk->hunk.old_lines;
 	int newlines = hunk->hunk.new_lines;
+	int last_origin = 0;
 
 	for (;
 		ctx->parse_ctx.remain_len > 1 &&
@@ -584,8 +593,13 @@ static int parse_hunk_body(
 			 * the "\ No newline at end of file" marker. Do not
 			 * verify its format, as it may be localized.
 			 */
-			if (!oldlines)
-				continue;
+			if (!oldlines) {
+				prefix = 0;
+				origin = eof_for_origin(last_origin);
+				old_lineno = -1;
+				new_lineno = -1;
+				break;
+			}
 			/* fall through */
 
 		default:
@@ -607,6 +621,8 @@ static int parse_hunk_body(
 		line->new_lineno = new_lineno;
 
 		hunk->line_count++;
+
+		last_origin = origin;
 	}
 
 	if (oldlines || newlines) {
@@ -617,7 +633,7 @@ static int parse_hunk_body(
 	}
 
 	/*
-	 * Handle "\ No newline at end of file".  Only expect the leading
+	 * Handle "\ No newline at end of file". Only expect the leading
 	 * backslash, though, because the rest of the string could be
 	 * localized.  Because `diff` optimizes for the case where you
 	 * want to apply the patch by hand.
@@ -628,11 +644,24 @@ static int parse_hunk_body(
 		line = git_array_get(patch->base.lines, git_array_size(patch->base.lines) - 1);
 
 		if (line->content_len < 1) {
-			error = git_parse_err("cannot trim trailing newline of empty line");
+			error = git_parse_err("last line has no trailing newline");
 			goto done;
 		}
 
-		line->content_len--;
+		line = git_array_alloc(patch->base.lines);
+		GIT_ERROR_CHECK_ALLOC(line);
+
+		memset(line, 0x0, sizeof(git_diff_line));
+
+		line->content = ctx->parse_ctx.line;
+		line->content_len = ctx->parse_ctx.line_len;
+		line->content_offset = ctx->parse_ctx.content_len - ctx->parse_ctx.remain_len;
+		line->origin = eof_for_origin(last_origin);
+		line->num_lines = 1;
+		line->old_lineno = -1;
+		line->new_lineno = -1;
+
+		hunk->line_count++;
 
 		git_parse_advance_line(&ctx->parse_ctx);
 	}

tests/patch/parse.c

@@ -27,6 +27,18 @@ static void ensure_patch_validity(git_patch *patch)
 	cl_assert_equal_i(0, delta->new_file.size);
 }
 
+static void ensure_identical_patch_inout(const char *content) {
+	git_buf buf = GIT_BUF_INIT;
+	git_patch *patch;
+
+	cl_git_pass(git_patch_from_buffer(&patch, content, strlen(content), NULL));
+	cl_git_pass(git_patch_to_buf(&buf, patch));
+	cl_assert_equal_strn(git_buf_cstr(&buf), content, strlen(content));
+
+	git_patch_free(patch);
+	git_buf_dispose(&buf);
+}
+
 void test_patch_parse__original_to_change_middle(void)
 {
 	git_patch *patch;
@@ -104,21 +116,15 @@ void test_patch_parse__invalid_patches_fails(void)
 
 void test_patch_parse__no_newline_at_end_of_new_file(void)
 {
-	git_patch *patch;
-	cl_git_pass(git_patch_from_buffer(&patch, PATCH_APPEND_NO_NL, strlen(PATCH_APPEND_NO_NL), NULL));
-	git_patch_free(patch);
+	ensure_identical_patch_inout(PATCH_APPEND_NO_NL);
 }
 
 void test_patch_parse__no_newline_at_end_of_old_file(void)
 {
-	git_patch *patch;
-	cl_git_pass(git_patch_from_buffer(&patch, PATCH_APPEND_NO_NL_IN_OLD_FILE, strlen(PATCH_APPEND_NO_NL_IN_OLD_FILE), NULL));
-	git_patch_free(patch);
+	ensure_identical_patch_inout(PATCH_APPEND_NO_NL_IN_OLD_FILE);
 }
 
 void test_patch_parse__files_with_whitespaces_succeeds(void)
 {
-	git_patch *patch;
-	cl_git_pass(git_patch_from_buffer(&patch, PATCH_NAME_WHITESPACE, strlen(PATCH_NAME_WHITESPACE), NULL));
-	git_patch_free(patch);
+	ensure_identical_patch_inout(PATCH_NAME_WHITESPACE);
 }