Skip to content

Commit 4d5f347

Browse files
peffgitster
peff
authored and
gitster
committed
combine-diff: handle binary files as binary
The combined diff code path is totally different from the regular diff code path, and didn't handle binary files at all. The results of a combined diff on a binary file could range from annoying (since we spewed binary garbage, possibly upsetting the user's terminal), to wrong (embedded NULs caused us to show incorrect diffs, with lines truncated at the NUL character), to potential security problems (embedded NULs could interfere with "-z" output, possibly defeating policy hooks which parse diff output). Instead, we consider a combined diff to be binary if any of the input blobs is binary. To show a binary combined diff, we indicate "Binary blobs differ"; the "index" meta line will show which parents had which blob. Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent c95b99b commit 4d5f347

File tree

2 files changed

+148
-2
lines changed

2 files changed

+148
-2
lines changed

‎combine-diff.c

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include "xdiff-interface.h"
88
#include "log-tree.h"
99
#include "refs.h"
10+
#include "userdiff.h"
1011

1112
static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, int n, int num_parent)
1213
{
@@ -685,7 +686,8 @@ static void show_combined_header(struct combine_diff_path *elem,
685686
int num_parent,
686687
int dense,
687688
struct rev_info *rev,
688-
int mode_differs)
689+
int mode_differs,
690+
int show_file_header)
689691
{
690692
struct diff_options *opt = &rev->diffopt;
691693
int abbrev = DIFF_OPT_TST(opt, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
@@ -739,6 +741,9 @@ static void show_combined_header(struct combine_diff_path *elem,
739741
printf("%s\n", c_reset);
740742
}
741743

744+
if (!show_file_header)
745+
return;
746+
742747
if (added)
743748
dump_quoted_path("--- ", "", "/dev/null",
744749
c_meta, c_reset);
@@ -765,8 +770,13 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
765770
int i, show_hunks;
766771
int working_tree_file = is_null_sha1(elem->sha1);
767772
mmfile_t result_file;
773+
struct userdiff_driver *userdiff;
774+
int is_binary;
768775

769776
context = opt->context;
777+
userdiff = userdiff_find_by_path(elem->path);
778+
if (!userdiff)
779+
userdiff = userdiff_find_by_name("default");
770780

771781
/* Read the result of merge first */
772782
if (!working_tree_file)
@@ -852,6 +862,29 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
852862
}
853863
}
854864

865+
if (userdiff->binary != -1)
866+
is_binary = userdiff->binary;
867+
else {
868+
is_binary = buffer_is_binary(result, result_size);
869+
for (i = 0; !is_binary && i < num_parent; i++) {
870+
char *buf;
871+
unsigned long size;
872+
buf = grab_blob(elem->parent[i].sha1,
873+
elem->parent[i].mode,
874+
&size);
875+
if (buffer_is_binary(buf, size))
876+
is_binary = 1;
877+
free(buf);
878+
}
879+
}
880+
if (is_binary) {
881+
show_combined_header(elem, num_parent, dense, rev,
882+
mode_differs, 0);
883+
printf("Binary files differ\n");
884+
free(result);
885+
return;
886+
}
887+
855888
for (cnt = 0, cp = result; cp < result + result_size; cp++) {
856889
if (*cp == '\n')
857890
cnt++;
@@ -906,7 +939,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
906939

907940
if (show_hunks || mode_differs || working_tree_file) {
908941
show_combined_header(elem, num_parent, dense, rev,
909-
mode_differs);
942+
mode_differs, 1);
910943
dump_sline(sline, cnt, num_parent,
911944
DIFF_OPT_TST(opt, COLOR_DIFF), result_deleted);
912945
}

‎t/t4048-diff-combined-binary.sh

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
#!/bin/sh
2+
3+
test_description='combined and merge diff handle binary files and textconv'
4+
. ./test-lib.sh
5+
6+
test_expect_success 'setup binary merge conflict' '
7+
echo oneQ1 | q_to_nul >binary &&
8+
git add binary &&
9+
git commit -m one &&
10+
echo twoQ2 | q_to_nul >binary &&
11+
git commit -a -m two &&
12+
git checkout -b branch-binary HEAD^ &&
13+
echo threeQ3 | q_to_nul >binary &&
14+
git commit -a -m three &&
15+
test_must_fail git merge master &&
16+
echo resolvedQhooray | q_to_nul >binary &&
17+
git commit -a -m resolved
18+
'
19+
20+
cat >expect <<'EOF'
21+
resolved
22+
23+
diff --git a/binary b/binary
24+
index 7ea6ded..9563691 100644
25+
Binary files a/binary and b/binary differ
26+
resolved
27+
28+
diff --git a/binary b/binary
29+
index 6197570..9563691 100644
30+
Binary files a/binary and b/binary differ
31+
EOF
32+
test_expect_success 'diff -m indicates binary-ness' '
33+
git show --format=%s -m >actual &&
34+
test_cmp expect actual
35+
'
36+
37+
cat >expect <<'EOF'
38+
resolved
39+
40+
diff --combined binary
41+
index 7ea6ded,6197570..9563691
42+
Binary files differ
43+
EOF
44+
test_expect_success 'diff -c indicates binary-ness' '
45+
git show --format=%s -c >actual &&
46+
test_cmp expect actual
47+
'
48+
49+
cat >expect <<'EOF'
50+
resolved
51+
52+
diff --cc binary
53+
index 7ea6ded,6197570..9563691
54+
Binary files differ
55+
EOF
56+
test_expect_success 'diff --cc indicates binary-ness' '
57+
git show --format=%s --cc >actual &&
58+
test_cmp expect actual
59+
'
60+
61+
test_expect_success 'setup non-binary with binary attribute' '
62+
git checkout master &&
63+
test_commit one text &&
64+
test_commit two text &&
65+
git checkout -b branch-text HEAD^ &&
66+
test_commit three text &&
67+
test_must_fail git merge master &&
68+
test_commit resolved text &&
69+
echo text -diff >.gitattributes
70+
'
71+
72+
cat >expect <<'EOF'
73+
resolved
74+
75+
diff --git a/text b/text
76+
index 2bdf67a..2ab19ae 100644
77+
Binary files a/text and b/text differ
78+
resolved
79+
80+
diff --git a/text b/text
81+
index f719efd..2ab19ae 100644
82+
Binary files a/text and b/text differ
83+
EOF
84+
test_expect_success 'diff -m respects binary attribute' '
85+
git show --format=%s -m >actual &&
86+
test_cmp expect actual
87+
'
88+
89+
cat >expect <<'EOF'
90+
resolved
91+
92+
diff --combined text
93+
index 2bdf67a,f719efd..2ab19ae
94+
Binary files differ
95+
EOF
96+
test_expect_success 'diff -c respects binary attribute' '
97+
git show --format=%s -c >actual &&
98+
test_cmp expect actual
99+
'
100+
101+
cat >expect <<'EOF'
102+
resolved
103+
104+
diff --cc text
105+
index 2bdf67a,f719efd..2ab19ae
106+
Binary files differ
107+
EOF
108+
test_expect_success 'diff --cc respects binary attribute' '
109+
git show --format=%s --cc >actual &&
110+
test_cmp expect actual
111+
'
112+
113+
test_done

0 commit comments

Comments
 (0)