Without this fix, "git ls-files --others /" would list _all_ files,
except for those tracked in the current repository. Worse, "git clean /"
would start removing them.
Noticed by Johannes Sixt.
Incidentally, it fixes some strange code in builtin-mv.c by yours truly,
where a slash was added to "dst" but then ignored, and instead taken from
the source path. This triggered the new check for absolute paths.
Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
---
On Mon, 28 Jan 2008, Johannes Sixt wrote:
> Junio C Hamano schrieb:
> > Johannes Sixt <j.sixt@xxxxxxxxxxxxx> writes:
> >
> >> The "problem" is not only with git-clean, but also in others,
> >> like git-ls-files. Try this in you favorite repository:
> >>
> >> $ git ls-files -o /*bin
> >>
> >> The output does not make a lot of sense. (Here it lists the
> >> contents of /bin and /sbin.) Not that it hurts with ls-files,
> >> but
> >>
> >> $ git clean -f /
> >>
> >> is basically a synonym for
> >>
> >> $ rm -rf /
> >
> > Yeah, /*bin is not inside the repository so it should not even
> > be reported as "others". Shouldn't the commands detect this
> > and reject feeding such paths outside the work tree to the
> > core, which always expect you to talk about paths inside?
>
> That's what I had expected. But look:
>
> $ git ls-files -o /
> [... tons of file names ...]
>
> $ git ls-files -o ..
> fatal: '..' is outside repository
>
> $ git clean -n / # with Shawn's patch
> Would remove /bin/
> [... etc ...]
>
> $ git clean -n ..
> fatal: '..' is outside repository
>
> Some mechanism for this is already there; it's just not complete
> enough.
This patch cannot be applied as-is: t3101 is failing (t7001 is
fixed by the builtin-mv.c part).
The failure of t3101 has something to do with ls-tree filtering
out invalid paths; I maintain that this behaviour is wrong to
begin with.
So the help I am requesting is this: so late in the game for 1.5.4
I would hate to introduce a change in prefix_path(), because it
affects apparently too much. However, the "git clean /" bug is a
real one, and should at least be prevented. What to do?
builtin-mv.c | 4 ++--
setup.c | 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/builtin-mv.c b/builtin-mv.c
index 990e213..94f6dd2 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -164,7 +164,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
}
dst = add_slash(dst);
- dst_len = strlen(dst) - 1;
+ dst_len = strlen(dst);
for (j = 0; j < last - first; j++) {
const char *path =
@@ -172,7 +172,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
source[argc + j] = path;
destination[argc + j] =
prefix_path(dst, dst_len,
- path + length);
+ path + length + 1);
modes[argc + j] = INDEX;
}
argc += last - first;
diff --git a/setup.c b/setup.c
index 2174e78..5a4aadc 100644
--- a/setup.c
+++ b/setup.c
@@ -13,6 +13,8 @@ const char *get_current_prefix()
const char *prefix_path(const char *prefix, int len, const char *path)
{
const char *orig = path;
+ if (is_absolute_path(path))
+ die("no absolute paths allowed: '%s'", path);
for (;;) {
char c;
if (*path != '.')
--
1.5.4.rc5.15.g8231f
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
