Skip to content

Commit

Permalink
diff: don't attempt to strip prefix from absolute Windows paths
Browse files Browse the repository at this point in the history
git diff can be invoked with absolute paths. Typically, this triggers
the --no-index case. Then the absolute paths remain in the file names
that are printed in the output.

There is one peculiarity, though: When the command is invoked from a
a sub-directory in a repository, then it is attempted to strip the
sub-directory from the beginning of relative paths. Yet, to detect a
relative path the code just checks for an initial forward slash.
This mistakes a Windows style path like "D:/base" as a relative path
and the output looks like this, for example:

  D:\dir\test\one>git -P diff --numstat D:\dir\base D:\dir\diff
  1       1       ir/{base => diff}/1.txt

where the correct output should be

  D:\dir\test\one>git -P diff --numstat D:\dir\base D:\dir\diff
  1       1       D:/dir/{base => diff}/1.txt

If the sub-directory where 'git diff' is invoked is sufficiently deep
that the prefix becomes longer than the path to be printed, then the
subsequent code accesses the path out of bounds.

Use is_absolute_path() to detect Windows style absolute paths.

One might wonder whether the check for a directory separator that
is visible in the patch context should be changed from == '/' to
is_dir_sep() or not. It turns out not to be necessary. That code
only ever investigates paths that have undergone pathspec
normalization, after which there are only forward slashes even on
Windows.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
j6t authored and gitster committed Oct 22, 2018
1 parent 27d05d1 commit ffd04e9
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 2 deletions.
4 changes: 2 additions & 2 deletions diff.c
Expand Up @@ -3958,12 +3958,12 @@ static void diff_fill_oid_info(struct diff_filespec *one)
static void strip_prefix(int prefix_length, const char **namep, const char **otherp)
{
/* Strip the prefix but do not molest /dev/null and absolute paths */
if (*namep && **namep != '/') {
if (*namep && !is_absolute_path(*namep)) {
*namep += prefix_length;
if (**namep == '/')
++*namep;
}
if (*otherp && **otherp != '/') {
if (*otherp && !is_absolute_path(*otherp)) {
*otherp += prefix_length;
if (**otherp == '/')
++*otherp;
Expand Down
10 changes: 10 additions & 0 deletions t/t4053-diff-no-index.sh
Expand Up @@ -127,4 +127,14 @@ test_expect_success 'diff --no-index from repo subdir respects config (implicit)
test_cmp expect actual.head
'

test_expect_success 'diff --no-index from repo subdir with absolute paths' '
cat <<-EOF >expect &&
1 1 $(pwd)/non/git/{a => b}
EOF
test_expect_code 1 \
git -C repo/sub diff --numstat \
"$(pwd)/non/git/a" "$(pwd)/non/git/b" >actual &&
test_cmp expect actual
'

test_done

0 comments on commit ffd04e9

Please sign in to comment.