diff: don't attempt to strip prefix from absolute Windows paths

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>

Johannes Sixt committed Oct 19, 2018 at 18:58 UTC ffd04e92e2c512cbbcb99526b064af302b443cb2
2 files changed +12 -2
diff.c
+2 -2
@@ -3958,12 +3958,12 @@ static void diff_fill_oid_info(struct diff_filespec *one)
3958 static void strip_prefix(int prefix_length, const char **namep, const char **otherp)
3959 {
3960 /* Strip the prefix but do not molest /dev/null and absolute paths */
3961 - if (*namep && **namep != '/') {
3961 + if (*namep && !is_absolute_path(*namep)) {
3962 *namep += prefix_length;
3963 if (**namep == '/')
3964 ++*namep;
3965 }
3966 - if (*otherp && **otherp != '/') {
3966 + if (*otherp && !is_absolute_path(*otherp)) {
3967 *otherp += prefix_length;
3968 if (**otherp == '/')
3969 ++*otherp;
t/t4053-diff-no-index.sh
+10
@@ -127,4 +127,14 @@ test_expect_success 'diff --no-index from repo subdir respects config (implicit)
127 test_cmp expect actual.head
128 '
129
130 +test_expect_success 'diff --no-index from repo subdir with absolute paths' '
131 + cat <<-EOF >expect &&
132 + 1 1 $(pwd)/non/git/{a => b}
133 + EOF
134 + test_expect_code 1 \
135 + git -C repo/sub diff --numstat \
136 + "$(pwd)/non/git/a" "$(pwd)/non/git/b" >actual &&
137 + test_cmp expect actual
138 +'
139 +
140 test_done