Skip to content

Commit de7659c

Browse files
committed
patch_parse: use paths from "---"/"+++" lines for binary patches
For some patches, it is not possible to derive the old and new file paths from the patch header's first line, most importantly when they contain spaces. In such a case, we derive both paths from the "---" and "+++" lines, which allow for non-ambiguous parsing. We fail to use these paths when parsing binary patches without data, though, as we always expect the header paths to be filled in. Fix this by using the "---"/"+++" paths by default and only fall back to header paths if they aren't set. If neither of those paths are set, we just return an error. Add two tests to verify this behaviour, one of which would have previously caused a segfault.
1 parent 01ea911 commit de7659c

File tree

3 files changed

+37
-5
lines changed

3 files changed

+37
-5
lines changed

src/patch_parse.c

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -878,12 +878,18 @@ static int parse_patch_binary_nodata(
878878
git_patch_parsed *patch,
879879
git_patch_parse_ctx *ctx)
880880
{
881+
const char *old = patch->old_path ? patch->old_path : patch->header_old_path;
882+
const char *new = patch->new_path ? patch->new_path : patch->header_new_path;
883+
884+
if (!old || !new)
885+
return git_parse_err("corrupt binary data without paths at line %"PRIuZ, ctx->parse_ctx.line_num);
886+
881887
if (git_parse_advance_expected_str(&ctx->parse_ctx, "Binary files ") < 0 ||
882-
git_parse_advance_expected_str(&ctx->parse_ctx, patch->header_old_path) < 0 ||
883-
git_parse_advance_expected_str(&ctx->parse_ctx, " and ") < 0 ||
884-
git_parse_advance_expected_str(&ctx->parse_ctx, patch->header_new_path) < 0 ||
885-
git_parse_advance_expected_str(&ctx->parse_ctx, " differ") < 0 ||
886-
git_parse_advance_nl(&ctx->parse_ctx) < 0)
888+
git_parse_advance_expected_str(&ctx->parse_ctx, old) < 0 ||
889+
git_parse_advance_expected_str(&ctx->parse_ctx, " and ") < 0 ||
890+
git_parse_advance_expected_str(&ctx->parse_ctx, new) < 0 ||
891+
git_parse_advance_expected_str(&ctx->parse_ctx, " differ") < 0 ||
892+
git_parse_advance_nl(&ctx->parse_ctx) < 0)
887893
return git_parse_err("corrupt git binary header at line %"PRIuZ, ctx->parse_ctx.line_num);
888894

889895
patch->base.binary.contains_data = 0;

tests/patch/parse.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,20 @@ void test_patch_parse__binary_file_with_empty_quoted_paths(void)
170170
strlen(PATCH_BINARY_FILE_WITH_QUOTED_EMPTY_PATHS), NULL));
171171
}
172172

173+
void test_patch_parse__binary_file_path_with_spaces(void)
174+
{
175+
git_patch *patch;
176+
cl_git_fail(git_patch_from_buffer(&patch, PATCH_BINARY_FILE_PATH_WITH_SPACES,
177+
strlen(PATCH_BINARY_FILE_PATH_WITH_SPACES), NULL));
178+
}
179+
180+
void test_patch_parse__binary_file_path_without_body_paths(void)
181+
{
182+
git_patch *patch;
183+
cl_git_fail(git_patch_from_buffer(&patch, PATCH_BINARY_FILE_PATH_WITHOUT_BODY_PATHS,
184+
strlen(PATCH_BINARY_FILE_PATH_WITHOUT_BODY_PATHS), NULL));
185+
}
186+
173187
void test_patch_parse__memory_leak_on_multiple_paths(void)
174188
{
175189
git_patch *patch;

tests/patch/patch_common.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -924,6 +924,18 @@
924924
"+++ \"\"\n" \
925925
"Binary files "
926926

927+
#define PATCH_BINARY_FILE_PATH_WITH_SPACES \
928+
"diff --git a b c d e f\n" \
929+
"--- a b c\n" \
930+
"+++ d e f\n" \
931+
"Binary files a b c and d e f differ"
932+
933+
#define PATCH_BINARY_FILE_PATH_WITHOUT_BODY_PATHS \
934+
"diff --git a b c d e f\n" \
935+
"--- \n" \
936+
"+++ \n" \
937+
"Binary files a b c and d e f differ"
938+
927939
#define PATCH_MULTIPLE_OLD_PATHS \
928940
"diff --git \n" \
929941
"--- \n" \

0 commit comments

Comments
 (0)