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>

diff --git a/builtin/apply.c b/builtin/apply.c
index c24dc546d0cc3f223c40c12aa20dc75eff13d4f9..2ad8c4820720944f19207c4c09e87e9946116cf0 100644 (file)
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1022,15 +1022,23 @@ static int gitdiff_unrecognized(const char *line, struct patch *patch)
        return -1;
 }
 
        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;
 
        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)
        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;
 }
        }
        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;
 
                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;
                        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.
 
                /*
                 * 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;
                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 */
                                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 */
                                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;
                        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);
                    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 */
        }
 
        /* unquoted first name */

-       name = stop_at_slash(line, llen);
-       if (!name || name == line)
+       name = skip_tree_prefix(line, llen);
+       if (!name)

                return NULL;
                return NULL;

-       name++;

 
        /*
         * since the first name is unquoted, a dq if exists must be
 
        /*
         * 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;
 
                        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;
                                goto free_and_fail2;

-                       np++;

 
                        len = sp.buf + sp.len - np;
                        if (len < second - name &&
 
                        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 ' ':
                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;
                        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);
                                return xmemdupz(name, len);

-                       }

                }
        }
 }
                }
        }
 }

diff --git a/t/t4103-apply-binary.sh b/t/t4103-apply-binary.sh
index dbbf56cba9f5108f79d767ad48f3092dc821a232..1b420e3b5fc2a7130d2b10bcd5660670b9d780c8 100755 (executable)
--- 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
 
 '
 . ./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 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 &&
 
        git checkout -b binary &&

-       perl -pe 'y/x/\000/' <file1 >file3 &&
+       perl -pe "y/x/\000/" <file1 >file3 &&

        cat file3 >file4 &&
        git add file2 &&
        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 &&
        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 &&
 
        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 --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 &&
        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 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'
 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)"'
 
         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
 test_done