Skip to content

Commit 74c4de5

Browse files
sunshinecogitster
authored andcommitted
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 <[email protected]> Signed-off-by: Eric Sunshine <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 052b255 commit 74c4de5

File tree

2 files changed

+9
-9
lines changed

2 files changed

+9
-9
lines changed

builtin/checkout-index.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ static char topath[4][TEMPORARY_FILENAME_LENGTH + 1];
1818

1919
static struct checkout state;
2020

21-
static void write_tempfile_record(const char *name, int prefix_length)
21+
static void write_tempfile_record(const char *name, const char *prefix)
2222
{
2323
int i;
2424

@@ -35,14 +35,14 @@ static void write_tempfile_record(const char *name, int prefix_length)
3535
fputs(topath[checkout_stage], stdout);
3636

3737
putchar('\t');
38-
write_name_quoted(name + prefix_length, stdout, line_termination);
38+
write_name_quoted_relative(name, prefix, stdout, line_termination);
3939

4040
for (i = 0; i < 4; i++) {
4141
topath[i][0] = 0;
4242
}
4343
}
4444

45-
static int checkout_file(const char *name, int prefix_length)
45+
static int checkout_file(const char *name, const char *prefix)
4646
{
4747
int namelen = strlen(name);
4848
int pos = cache_name_pos(name, namelen);
@@ -71,7 +71,7 @@ static int checkout_file(const char *name, int prefix_length)
7171

7272
if (did_checkout) {
7373
if (to_tempfile)
74-
write_tempfile_record(name, prefix_length);
74+
write_tempfile_record(name, prefix);
7575
return errs > 0 ? -1 : 0;
7676
}
7777

@@ -106,15 +106,15 @@ static void checkout_all(const char *prefix, int prefix_length)
106106
if (last_ce && to_tempfile) {
107107
if (ce_namelen(last_ce) != ce_namelen(ce)
108108
|| memcmp(last_ce->name, ce->name, ce_namelen(ce)))
109-
write_tempfile_record(last_ce->name, prefix_length);
109+
write_tempfile_record(last_ce->name, prefix);
110110
}
111111
if (checkout_entry(ce, &state,
112112
to_tempfile ? topath[ce_stage(ce)] : NULL) < 0)
113113
errs++;
114114
last_ce = ce;
115115
}
116116
if (last_ce && to_tempfile)
117-
write_tempfile_record(last_ce->name, prefix_length);
117+
write_tempfile_record(last_ce->name, prefix);
118118
if (errs)
119119
/* we have already done our error reporting.
120120
* exit with the same code as die().
@@ -247,7 +247,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
247247
if (read_from_stdin)
248248
die("git checkout-index: don't mix '--stdin' and explicit filenames");
249249
p = prefix_path(prefix, prefix_length, arg);
250-
checkout_file(p, prefix_length);
250+
checkout_file(p, prefix);
251251
if (p < arg || p > arg + strlen(arg))
252252
free((char *)p);
253253
}
@@ -267,7 +267,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
267267
strbuf_swap(&buf, &nbuf);
268268
}
269269
p = prefix_path(prefix, prefix_length, buf.buf);
270-
checkout_file(p, prefix_length);
270+
checkout_file(p, prefix);
271271
if (p < buf.buf || p > buf.buf + buf.len)
272272
free((char *)p);
273273
}

t/t2004-checkout-cache-temp.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ test_expect_success 'checkout --temp symlink' '
206206
test $(cat $p) = path7
207207
'
208208

209-
test_expect_failure 'emit well-formed relative path' '
209+
test_expect_success 'emit well-formed relative path' '
210210
rm -f path* .merge_* actual .git/index &&
211211
>path0123456789 &&
212212
git update-index --add path0123456789 &&

0 commit comments

Comments
 (0)