Skip to content

Commit

Permalink
apply: don't segfault on binary files with missing data
Browse files Browse the repository at this point in the history
Usually when applying a binary diff generated without
--binary, it will be rejected early, as we don't even have
the full sha1 of the pre- and post-images.

However, if the diff is generated with --full-index (but not
--binary), then we will actually try to apply it. If we have
the postimage blob, then we can take a shortcut and never
even look at the binary diff at all (e.g., this can happen
when rebasing changes within a repository).

If we don't have the postimage blob, though, we try to look
at the actual fragments, of which there are none, and get a
segfault. This patch checks explicitly for that case and
complains to the user instead of segfaulting. We need to
keep the check at a low level so that the "shortcut" case
above continues to work.

We also add a test that demonstrates the segfault. While
we're at it, let's also explicitly test the shortcut case.

Reported-by: Rafaël Carré <[email protected]>
Signed-off-by: Jeff King <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
  • Loading branch information
peff authored and gitster committed Oct 29, 2010
1 parent 87b5054 commit 24305cd
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 1 deletion.
6 changes: 6 additions & 0 deletions builtin/apply.c
Original file line number Diff line number Diff line change
Expand Up @@ -2645,6 +2645,12 @@ static int apply_binary_fragment(struct image *img, struct patch *patch)
unsigned long len;
void *dst;

if (!fragment)
return error("missing binary patch data for '%s'",
patch->new_name ?
patch->new_name :
patch->old_name);

/* Binary patch is irreversible without the optional second hunk */
if (apply_in_reverse) {
if (!fragment->next)
Expand Down
27 changes: 26 additions & 1 deletion t/t4103-apply-binary.sh
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,16 @@ test_expect_success 'setup' "
git diff-tree -p -C master binary >C.diff &&
git diff-tree -p --binary master binary >BF.diff &&
git diff-tree -p --binary -C master binary >CF.diff
git diff-tree -p --binary -C master binary >CF.diff &&
git diff-tree -p --full-index master binary >B-index.diff &&
git diff-tree -p -C --full-index master binary >C-index.diff &&
git init other-repo &&
(cd other-repo &&
git fetch .. master &&
git reset --hard FETCH_HEAD
)
"

test_expect_success 'stat binary diff -- should not fail.' \
Expand Down Expand Up @@ -100,6 +109,22 @@ test_expect_success 'apply binary diff (copy) -- should fail.' \
'do_reset &&
test_must_fail git apply --index C.diff'

test_expect_success 'apply binary diff with full-index' '
do_reset &&
git apply B-index.diff
'

test_expect_success 'apply binary diff with full-index (copy)' '
do_reset &&
git apply C-index.diff
'

test_expect_success 'apply full-index binary diff in new repo' '
(cd other-repo &&
do_reset &&
test_must_fail git apply ../B-index.diff)
'

test_expect_success 'apply binary diff without replacement.' \
'do_reset &&
git apply BF.diff'
Expand Down

0 comments on commit 24305cd

Please sign in to comment.