checkout-index: fix --temp relative path mangling

checkout-index --temp only properly prints relative paths which are
descendants of the current directory. Paths in ancestor or sibling
directories (or their children) are often printed in mangled form. For
example:

    mkdir a bbb &&
    >file &&
    >bbb/file &&
    git update-index --add file bbb/file &&
    cd a &&
    git checkout-index --temp ../file ../bbb/file

prints:

    .merge_file_ooblek  le
    .merge_file_igloo0  b/file

rather than the correct:

    .merge_file_ooblek  ../file
    .merge_file_igloo0  ../bbb/file

Internally, given the above example, checkout-index prefixes each input
argument with the name of the current directory ("a/", in this case),
and then assumes that it can simply skip forward by strlen("a/") bytes
to recover the original name. This works for files in the current
directory or its descendants, but fails for files in ancestors or
siblings (or their children) due to path normalization.

For instance, given "../file", "a/" is prepended, giving "a/../file".
Path normalization folds out "a/../", resulting in "file". Attempting
to recover the original name by skipping strlen("a/") bytes gives the
incorrect "le" rather than the desired "../file".

Fix this by taking advantage of write_name_quoted_relative() to recover
the original name properly, rather than assuming that it can be
recovered by skipping strlen(prefix) bytes.

As a bonus, this also fixes a bug in which checkout-index --temp
accessed and printed memory beyond the end-of-string. For instance,
within a subdirectory named "subdirectory", and given argument
"../file", prefixing would give "subdirectory/../file", which would
become "file" after normalization. checkout-index would then attempt to
recover the original name by skipping strlen("subdirectory/") bytes of
"file", which placed it well beyond end-of-string. Despite this error,
it often appeared to give the correct result, but only due to an
accident of implementation which left an apparently correct copy of the
path in memory following the normalized value. In particular, handed
"subdirectory/../file", in-place processing by normalize_path_copy_len()
resulted in "file\0rectory/../file". When checkout-index skipped
strlen("subdirectory/") bytes, it ended up back at "../file" and thus
appeared to give the correct answer, despite being past end-of-string.

Reported-by: Russ Cox <rsc@golang.org>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

2 files changed, 9 insertions, 9 deletions

diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index 61e75eb..256310f 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -18,7 +18,7 @@ static char topath[4][TEMPORARY_FILENAME_LENGTH + 1];
 
 static struct checkout state;
 
-static void write_tempfile_record(const char *name, int prefix_length)
+static void write_tempfile_record(const char *name, const char *prefix)
 {
 	int i;
 
@@ -35,14 +35,14 @@ static void write_tempfile_record(const char *name, int prefix_length)
 		fputs(topath[checkout_stage], stdout);
 
 	putchar('\t');
-	write_name_quoted(name + prefix_length, stdout, line_termination);
+	write_name_quoted_relative(name, prefix, stdout, line_termination);
 
 	for (i = 0; i < 4; i++) {
 		topath[i][0] = 0;
 	}
 }
 
-static int checkout_file(const char *name, int prefix_length)
+static int checkout_file(const char *name, const char *prefix)
 {
 	int namelen = strlen(name);
 	int pos = cache_name_pos(name, namelen);
@@ -71,7 +71,7 @@ static int checkout_file(const char *name, int prefix_length)
 
 	if (did_checkout) {
 		if (to_tempfile)
-			write_tempfile_record(name, prefix_length);
+			write_tempfile_record(name, prefix);
 		return errs > 0 ? -1 : 0;
 	}
 
@@ -106,7 +106,7 @@ static void checkout_all(const char *prefix, int prefix_length)
 		if (last_ce && to_tempfile) {
 			if (ce_namelen(last_ce) != ce_namelen(ce)
 			    || memcmp(last_ce->name, ce->name, ce_namelen(ce)))
-				write_tempfile_record(last_ce->name, prefix_length);
+				write_tempfile_record(last_ce->name, prefix);
 		}
 		if (checkout_entry(ce, &state,
 		    to_tempfile ? topath[ce_stage(ce)] : NULL) < 0)
@@ -114,7 +114,7 @@ static void checkout_all(const char *prefix, int prefix_length)
 		last_ce = ce;
 	}
 	if (last_ce && to_tempfile)
-		write_tempfile_record(last_ce->name, prefix_length);
+		write_tempfile_record(last_ce->name, prefix);
 	if (errs)
 		/* we have already done our error reporting.
 		 * exit with the same code as die().
@@ -247,7 +247,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
 		if (read_from_stdin)
 			die("git checkout-index: don't mix '--stdin' and explicit filenames");
 		p = prefix_path(prefix, prefix_length, arg);
-		checkout_file(p, prefix_length);
+		checkout_file(p, prefix);
 		if (p < arg || p > arg + strlen(arg))
 			free((char *)p);
 	}
@@ -267,7 +267,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
 				strbuf_swap(&buf, &nbuf);
 			}
 			p = prefix_path(prefix, prefix_length, buf.buf);
-			checkout_file(p, prefix_length);
+			checkout_file(p, prefix);
 			if (p < buf.buf || p > buf.buf + buf.len)
 				free((char *)p);
 		}
diff --git a/t/t2004-checkout-cache-temp.sh b/t/t2004-checkout-cache-temp.sh
index 3c2b2b9..a12afe9 100755
--- a/t/t2004-checkout-cache-temp.sh
+++ b/t/t2004-checkout-cache-temp.sh
@@ -206,7 +206,7 @@ test_expect_success 'checkout --temp symlink' '
 	test $(cat $p) = path7
 '
 
-test_expect_failure 'emit well-formed relative path' '
+test_expect_success 'emit well-formed relative path' '
 	rm -f path* .merge_* actual .git/index &&
 	>path0123456789 &&
 	git update-index --add path0123456789 &&