Colin McCabe cmcc...@alumni.cmu.edu writes:
I found a bug in git's handling of binary diff segments.
When applying binary diffs using -p0, the prefix (or --strip) argument
is ignored.
Thanks for a report. An ancient bug well spotted.
Perhaps this will help.
-- 8 --
Subject: 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 -pn, 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.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/apply.c | 55 ++-
1 file changed, 30 insertions(+), 25 deletions(-)
diff --git i/builtin/apply.c w/builtin/apply.c
index 3bf71dc..4a511b3 100644
--- i/builtin/apply.c
+++ w/builtin/apply.c
@@ -1095,15 +1095,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;
}
@@ -1133,12 +1141,11 @@ static char *git_header_name(const 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.
@@ -1152,22 +1159,21 @@ static char *git_header_name(const 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);
@@ -1179,10 +1185,9 @@ static char *git_header_name(const 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
@@ -1196,10 +1201,9 @@ static char *git_header_name(const char *line, int llen)
if (unquote_c_style(sp, second, NULL))
goto free_and_fail2;
-
