Re: [RFH/PATCH] prefix_path(): disallow absolute paths

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 30, 2008, 00:43 UTC
Message-ID: <7vhcgwkurj.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <200801292158.m0TLwA7u019321@mi0.bluebottle.com>

しらいしななこ  <nanako3@bluebottle.com> writes:

Show 10 quoted lines

> Quoting Junio C Hamano <gitster@pobox.com>:
>...
>> And not passing such ambiguous path like "fo//o" to the core
>> level but sanitizing matters.  Then core level can always do
>> memcmp() with "fo/o" to see they are talking about the same
>> path.
>
> I may be mistaken but I think "fo//o" and "fo//o/" are
> returned as two different strings "fo/o" and "fo/o/" from your
> patch. Shouldn't you clean-up the second one to "fo/o", too?

That certainly is a potentially possible sanitization, and I actually thought about it when I did these patches.

However, I chose not to because I was not sure if some core functions want to differentiate pathspecs "foo/" and "foo". The former says "I want to make sure that 'foo' is a directory, and want to affect everything under it", the latter says "I do not care if 'foo' is a blob or a directory, but I want to affect it (if a blob) or everything under it (if a directory)".

In fact, stripping trailing slashes off would break this pair:

	git ls-files --error-unmatch Makefile/
	git ls-files --error-unmatch Makefile

Things like "git add Makefile/" relies on the former to fail loudly. So the answer is no, we do not want to clean it up.

Incidentally, I notice that in addition to the squashed patch from yesterday, we would need to teach "error-unmatch" code that it should trigger when get_pathspec() returns a pathspec that has fewer number of paths than its input.

It should be a pretty straightforward patch, but I haven't looked into fixing it. I'm lazy and I'd rather have other people to do the fixing for me. Hint, hint... ;-)

By the way, please keep your lines to reasonable length by wrapping in your e-mailed messages.