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@xxxxxxxxx>
---
* This time, with a test. The patch is designed to apply to a bit
older codebase, and won't apply cleanly to 'master', but the
result of merging it to newer codebase can be seen near the tip
of 'pu' tonight.
builtin/apply.c | 68 +++++++++++++++++++++++++++++++------------------
t/t4103-apply-binary.sh | 54 ++++++++++++++++++++++++---------------
2 files changed, 76 insertions(+), 46 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index c24dc54..2ad8c48 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1022,15 +1022,23 @@ static int gitdiff_unrecognized(const char *line, struct patch *patch)
return -1;
}
-static const char *stop_at_slash(const char *line, int llen)
+/*
+ * Skip p_value leading components from "line"; as we do not accept
+ * absolute paths, return NULL in that case.
+ */
+static const char *skip_tree_prefix(const char *line, int llen)
{
- int nslash = p_value;
+ int nslash;
int i;
+ if (!p_value)
+ return (llen && line[0] == '/') ? NULL : line;
+
+ nslash = p_value;
for (i = 0; i < llen; i++) {
int ch = line[i];
if (ch == '/' && --nslash <= 0)
- return &line[i];
+ return (i == 0) ? NULL : &line[i + 1];
}
return NULL;
}
@@ -1060,12 +1068,11 @@ static char *git_header_name(char *line, int llen)
if (unquote_c_style(&first, line, &second))
goto free_and_fail1;
- /* advance to the first slash */
- cp = stop_at_slash(first.buf, first.len);
- /* we do not accept absolute paths */
- if (!cp || cp == first.buf)
+ /* strip the a/b prefix including trailing slash */
+ cp = skip_tree_prefix(first.buf, first.len);
+ if (!cp)
goto free_and_fail1;
- strbuf_remove(&first, 0, cp + 1 - first.buf);
+ strbuf_remove(&first, 0, cp - first.buf);
/*
* second points at one past closing dq of name.
@@ -1079,22 +1086,21 @@ static char *git_header_name(char *line, int llen)
if (*second == '"') {
if (unquote_c_style(&sp, second, NULL))
goto free_and_fail1;
- cp = stop_at_slash(sp.buf, sp.len);
- if (!cp || cp == sp.buf)
+ cp = skip_tree_prefix(sp.buf, sp.len);
+ if (!cp)
goto free_and_fail1;
/* They must match, otherwise ignore */
- if (strcmp(cp + 1, first.buf))
+ if (strcmp(cp, first.buf))
goto free_and_fail1;
strbuf_release(&sp);
return strbuf_detach(&first, NULL);
}
/* unquoted second */
- cp = stop_at_slash(second, line + llen - second);
- if (!cp || cp == second)
+ cp = skip_tree_prefix(second, line + llen - second);
+ if (!cp)
goto free_and_fail1;
- cp++;
- if (line + llen - cp != first.len + 1 ||
+ if (line + llen - cp != first.len ||
memcmp(first.buf, cp, first.len))
goto free_and_fail1;
return strbuf_detach(&first, NULL);
@@ -1106,10 +1112,9 @@ static char *git_header_name(char *line, int llen)
}
/* unquoted first name */
- name = stop_at_slash(line, llen);
- if (!name || name == line)
+ name = skip_tree_prefix(line, llen);
+ if (!name)
return NULL;
- name++;
/*
* 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)
if (unquote_c_style(&sp, second, NULL))
goto free_and_fail2;
- np = stop_at_slash(sp.buf, sp.len);
- if (!np || np == sp.buf)
+ np = skip_tree_prefix(sp.buf, sp.len);
+ if (!np)
goto free_and_fail2;
- np++;
len = sp.buf + sp.len - np;
if (len < second - name &&
@@ -1158,13 +1162,27 @@ static char *git_header_name(char *line, int llen)
case '\n':
return NULL;
case '\t': case ' ':
- second = stop_at_slash(name + len, line_len - len);
+ /*
+ * Is this the separator between the preimage
+ * and the postimage pathname? Again, we are
+ * only interested in the case where there is
+ * no rename, as this is only to set def_name
+ * and a rename patch has the names elsewhere
+ * in an unambiguous form.
+ */
+ if (!name[len + 1])
+ return NULL; /* no postimage name */
+ second = skip_tree_prefix(name + len + 1,
+ line_len - (len + 1));
if (!second)
return NULL;
- second++;
- if (second[len] == '\n' && !strncmp(name, second, len)) {
+ /*
+ * Does len bytes starting at "name" and "second"
+ * (that are separated by one HT or SP we just
+ * found) exactly match?
+ */
+ if (second[len] == '\n' && !strncmp(name, second, len))
return xmemdupz(name, len);
- }
}
}
}
diff --git a/t/t4103-apply-binary.sh b/t/t4103-apply-binary.sh
index dbbf56c..1b420e3 100755
--- a/t/t4103-apply-binary.sh
+++ b/t/t4103-apply-binary.sh
@@ -8,30 +8,28 @@ test_description='git apply handling binary patches
'
. ./test-lib.sh
-# setup
-
-cat >file1 <<EOF
-A quick brown fox jumps over the lazy dog.
-A tiny little penguin runs around in circles.
-There is a flag with Linux written on it.
-A slow black-and-white panda just sits there,
-munching on his bamboo.
-EOF
-cat file1 >file2
-cat file1 >file4
-
-test_expect_success 'setup' "
+test_expect_success 'setup' '
+ cat >file1 <<-\EOF &&
+ A quick brown fox jumps over the lazy dog.
+ A tiny little penguin runs around in circles.
+ There is a flag with Linux written on it.
+ A slow black-and-white panda just sits there,
+ munching on his bamboo.
+ EOF
+ cat file1 >file2 &&
+ cat file1 >file4 &&
+
git update-index --add --remove file1 file2 file4 &&
- git commit -m 'Initial Version' 2>/dev/null &&
+ git commit -m "Initial Version" 2>/dev/null &&
git checkout -b binary &&
- perl -pe 'y/x/\000/' <file1 >file3 &&
+ perl -pe "y/x/\000/" <file1 >file3 &&
cat file3 >file4 &&
git add file2 &&
- perl -pe 'y/\000/v/' <file3 >file1 &&
+ perl -pe "y/\000/v/" <file3 >file1 &&
rm -f file2 &&
git update-index --add --remove file1 file2 file3 file4 &&
- git commit -m 'Second Version' &&
+ git commit -m "Second Version" &&
git diff-tree -p master binary >B.diff &&
git diff-tree -p -C master binary >C.diff &&
@@ -42,17 +40,25 @@ test_expect_success 'setup' "
git diff-tree -p --full-index master binary >B-index.diff &&
git diff-tree -p -C --full-index master binary >C-index.diff &&
+ git diff-tree -p --binary --no-prefix master binary -- file3 >B0.diff &&
+
git init other-repo &&
- (cd other-repo &&
- git fetch .. master &&
- git reset --hard FETCH_HEAD
+ (
+ cd other-repo &&
+ git fetch .. master &&
+ git reset --hard FETCH_HEAD
)
-"
+'
test_expect_success 'stat binary diff -- should not fail.' \
'git checkout master &&
git apply --stat --summary B.diff'
+test_expect_success 'stat binary -p0 diff -- should not fail.' '
+ git checkout master &&
+ git apply --stat -p0 B0.diff
+'
+
test_expect_success 'stat binary diff (copy) -- should not fail.' \
'git checkout master &&
git apply --stat --summary C.diff'
@@ -143,4 +149,10 @@ test_expect_success 'apply binary diff (copy).' \
git apply --allow-binary-replacement --index CF.diff &&
test -z "$(git diff --name-status binary)"'
+test_expect_success 'apply binary -p0 diff' '
+ do_reset &&
+ git apply -p0 --index B0.diff &&
+ test -z "$(git diff --name-status binary -- file3)"
+'
+
test_done
--
1.7.12.252.gef4e272
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html