Skip to content

Commit

Permalink
apply: compute patch->def_name correctly under -p0
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
Junio C Hamano committed Aug 25, 2012
1 parent 828ea97 commit 6a2abdc
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 @@ -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;
}
Expand Down Expand Up @@ -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.
Expand All @@ -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);
Expand All @@ -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
Expand All @@ -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 &&
Expand Down Expand Up @@ -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);
}
}
}
}
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 -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 &&
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 6a2abdc

Please sign in to comment.