apply: compute patch->def_name correctly under -p0
Back when "git apply" was written, we made sure that the user can skip more than the default number of path components (i.e. 1) by giving "-p<n>", but the logic for doing so was built around the notion of "we skip N slashes and stop". This obviously does not work well when running under -p0 where we do not want to skip any, but still want to skip SP/HT that separates the pathnames of preimage and postimage and want to reject absolute pathnames. Stop using "stop_at_slash()", and instead introduce a new helper "skip_tree_prefix()" with similar logic but works correctly even for the -p0 case. This is an ancient bug, but has been masked for a long time because most of the patches are text and have other clues to tell us the name of the preimage and the postimage. Noticed by Colin McCabe. Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano committed
Aug 24, 2012 at 22:48 UTC
6a2abdc12516cd3801bceef4ccfba399c962a074
2 files changed
+76
-46
builtin/apply.c
+43
-25
@@ -1022,15 +1022,23 @@ static int gitdiff_unrecognized(const char *line, struct patch *patch)
1022
return -1;
1023
}
1024
1025
-static const char *stop_at_slash(const char *line, int llen)
1025
+/*
1026
+ * Skip p_value leading components from "line"; as we do not accept
1027
+ * absolute paths, return NULL in that case.
1028
+ */
1029
+static const char *skip_tree_prefix(const char *line, int llen)
1030
{
1027
- int nslash = p_value;
1031
+ int nslash;
1032
int i;
1033
1034
+ if (!p_value)
1035
+ return (llen && line[0] == '/') ? NULL : line;
1036
+
1037
+ nslash = p_value;
1038
for (i = 0; i < llen; i++) {
1039
int ch = line[i];
1040
if (ch == '/' && --nslash <= 0)
1033
- return &line[i];
1041
+ return (i == 0) ? NULL : &line[i + 1];
1042
}
1043
return NULL;
1044
}
@@ -1060,12 +1068,11 @@ static char *git_header_name(char *line, int llen)
1068
if (unquote_c_style(&first, line, &second))
1069
goto free_and_fail1;
1070
1063
- /* advance to the first slash */
1064
- cp = stop_at_slash(first.buf, first.len);
1065
- /* we do not accept absolute paths */
1066
- if (!cp || cp == first.buf)
1071
+ /* strip the a/b prefix including trailing slash */
1072
+ cp = skip_tree_prefix(first.buf, first.len);
1073
+ if (!cp)
1074
goto free_and_fail1;
1068
- strbuf_remove(&first, 0, cp + 1 - first.buf);
1075
+ strbuf_remove(&first, 0, cp - first.buf);
1076
1077
/*
1078
* second points at one past closing dq of name.
@@ -1079,22 +1086,21 @@ static char *git_header_name(char *line, int llen)
1086
if (*second == '"') {
1087
if (unquote_c_style(&sp, second, NULL))
1088
goto free_and_fail1;
1082
- cp = stop_at_slash(sp.buf, sp.len);
1083
- if (!cp || cp == sp.buf)
1089
+ cp = skip_tree_prefix(sp.buf, sp.len);
1090
+ if (!cp)
1091
goto free_and_fail1;
1092
/* They must match, otherwise ignore */
1086
- if (strcmp(cp + 1, first.buf))
1093
+ if (strcmp(cp, first.buf))
1094
goto free_and_fail1;
1095
strbuf_release(&sp);
1096
return strbuf_detach(&first, NULL);
1097
}
1098
1099
/* unquoted second */
1093
- cp = stop_at_slash(second, line + llen - second);
1094
- if (!cp || cp == second)
1100
+ cp = skip_tree_prefix(second, line + llen - second);
1101
+ if (!cp)
1102
goto free_and_fail1;
1096
- cp++;
1097
- if (line + llen - cp != first.len + 1 ||
1103
+ if (line + llen - cp != first.len ||
1104
memcmp(first.buf, cp, first.len))
1105
goto free_and_fail1;
1106
return strbuf_detach(&first, NULL);
@@ -1106,10 +1112,9 @@ static char *git_header_name(char *line, int llen)
1112
}
1113
1114
/* unquoted first name */
1109
- name = stop_at_slash(line, llen);
1110
- if (!name || name == line)
1115
+ name = skip_tree_prefix(line, llen);
1116
+ if (!name)
1117
return NULL;
1112
- name++;
1118
1119
/*
1120
* since the first name is unquoted, a dq if exists must be
@@ -1123,10 +1128,9 @@ static char *git_header_name(char *line, int llen)
1128
if (unquote_c_style(&sp, second, NULL))
1129
goto free_and_fail2;
1130
1126
- np = stop_at_slash(sp.buf, sp.len);
1127
- if (!np || np == sp.buf)
1131
+ np = skip_tree_prefix(sp.buf, sp.len);
1132
+ if (!np)
1133
goto free_and_fail2;
1129
- np++;
1134
1135
len = sp.buf + sp.len - np;
1136
if (len < second - name &&
@@ -1158,13 +1162,27 @@ static char *git_header_name(char *line, int llen)
1162
case '\n':
1163
return NULL;
1164
case '\t': case ' ':
1161
- second = stop_at_slash(name + len, line_len - len);
1165
+ /*
1166
+ * Is this the separator between the preimage
1167
+ * and the postimage pathname? Again, we are
1168
+ * only interested in the case where there is
1169
+ * no rename, as this is only to set def_name
1170
+ * and a rename patch has the names elsewhere
1171
+ * in an unambiguous form.
1172
+ */
1173
+ if (!name[len + 1])
1174
+ return NULL; /* no postimage name */
1175
+ second = skip_tree_prefix(name + len + 1,
1176
+ line_len - (len + 1));
1177
if (!second)
1178
return NULL;
1164
- second++;
1165
- if (second[len] == '\n' && !strncmp(name, second, len)) {
1179
+ /*
1180
+ * Does len bytes starting at "name" and "second"
1181
+ * (that are separated by one HT or SP we just
1182
+ * found) exactly match?
1183
+ */
1184
+ if (second[len] == '\n' && !strncmp(name, second, len))
1185
return xmemdupz(name, len);
1167
- }
1186
}
1187
}
1188
}
t/t4103-apply-binary.sh
+33
-21
@@ -8,30 +8,28 @@ test_description='git apply handling binary patches
8
'
9
. ./test-lib.sh
10
11
-# setup
12
-
13
-cat >file1 <<EOF
14
-A quick brown fox jumps over the lazy dog.
15
-A tiny little penguin runs around in circles.
16
-There is a flag with Linux written on it.
17
-A slow black-and-white panda just sits there,
18
-munching on his bamboo.
19
-EOF
20
-cat file1 >file2
21
-cat file1 >file4
22
-
23
-test_expect_success 'setup' "
11
+test_expect_success 'setup' '
12
+ cat >file1 <<-\EOF &&
13
+ A quick brown fox jumps over the lazy dog.
14
+ A tiny little penguin runs around in circles.
15
+ There is a flag with Linux written on it.
16
+ A slow black-and-white panda just sits there,
17
+ munching on his bamboo.
18
+ EOF
19
+ cat file1 >file2 &&
20
+ cat file1 >file4 &&
21
+
22
git update-index --add --remove file1 file2 file4 &&
25
- git commit -m 'Initial Version' 2>/dev/null &&
23
+ git commit -m "Initial Version" 2>/dev/null &&
24
25
git checkout -b binary &&
28
- perl -pe 'y/x/\000/' <file1 >file3 &&
26
+ perl -pe "y/x/\000/" <file1 >file3 &&
27
cat file3 >file4 &&
28
git add file2 &&
31
- perl -pe 'y/\000/v/' <file3 >file1 &&
29
+ perl -pe "y/\000/v/" <file3 >file1 &&
30
rm -f file2 &&
31
git update-index --add --remove file1 file2 file3 file4 &&
34
- git commit -m 'Second Version' &&
32
+ git commit -m "Second Version" &&
33
34
git diff-tree -p master binary >B.diff &&
35
git diff-tree -p -C master binary >C.diff &&
@@ -42,17 +40,25 @@ test_expect_success 'setup' "
40
git diff-tree -p --full-index master binary >B-index.diff &&
41
git diff-tree -p -C --full-index master binary >C-index.diff &&
42
43
+ git diff-tree -p --binary --no-prefix master binary -- file3 >B0.diff &&
44
+
45
git init other-repo &&
46
- (cd other-repo &&
47
- git fetch .. master &&
48
- git reset --hard FETCH_HEAD
46
+ (
47
+ cd other-repo &&
48
+ git fetch .. master &&
49
+ git reset --hard FETCH_HEAD
50
)
50
-"
51
+'
52
53
test_expect_success 'stat binary diff -- should not fail.' \
54
'git checkout master &&
55
git apply --stat --summary B.diff'
56
57
+test_expect_success 'stat binary -p0 diff -- should not fail.' '
58
+ git checkout master &&
59
+ git apply --stat -p0 B0.diff
60
+'
61
+
62
test_expect_success 'stat binary diff (copy) -- should not fail.' \
63
'git checkout master &&
64
git apply --stat --summary C.diff'
@@ -143,4 +149,10 @@ test_expect_success 'apply binary diff (copy).' \
149
git apply --allow-binary-replacement --index CF.diff &&
150
test -z "$(git diff --name-status binary)"'
151
152
+test_expect_success 'apply binary -p0 diff' '
153
+ do_reset &&
154
+ git apply -p0 --index B0.diff &&
155
+ test -z "$(git diff --name-status binary -- file3)"
156
+'
157
+
158
test_done