Skip to content

Commit

Permalink
Merge branch 'jc/apply-binary-p0' into maint-1.7.11
Browse files Browse the repository at this point in the history 
"git apply -p0" did not parse pathnames on "diff --git" line
correctly.  This caused patches that had pathnames in no other
places to be mistakenly rejected (most notably, binary patch that
does not rename nor change mode).  Textual patches, renames or mode
changes have preimage and postimage pathnames in different places in
a form that can be parsed unambiguously and did not suffer from this
problem.

* jc/apply-binary-p0:
  apply: compute patch->def_name correctly under -p0
  • Loading branch information
@gitster
gitster committed Sep 12, 2012
2 parents eaff724 + 6a2abdc commit 1403db4
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 46 deletions.
68 changes: 43 additions & 25 deletions builtin/apply.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1086,15 +1086,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;
}
Expand Down Expand Up @@ -1124,12 +1132,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.
Expand All @@ -1143,22 +1150,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);
Expand All @@ -1170,10 +1176,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
Expand All @@ -1187,10 +1192,9 @@ static char *git_header_name(const 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 &&
Expand Down Expand Up @@ -1222,13 +1226,27 @@ static char *git_header_name(const 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);
}
}
}
}
Expand Down
54 changes: 33 additions & 21 deletions t/t4103-apply-binary.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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_PATH" -pe 'y/x/\000/' <file1 >file3 &&
"$PERL_PATH" -pe "y/x/\000/" <file1 >file3 &&
cat file3 >file4 &&
git add file2 &&
"$PERL_PATH" -pe 'y/\000/v/' <file3 >file1 &&
"$PERL_PATH" -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 &&
Expand All @@ -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'
Expand Down Expand Up @@ -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

0 comments on commit 1403db4

Please sign in to comment.