From: Junio C Hamano Date: Wed, 30 Jan 2008 00:43:44 GMT Subject: Re: [RFH/PATCH] prefix_path(): disallow absolute paths Message-ID: <7vhcgwkurj.fsf@gitster.siamese.dyndns.org> In-Reply-To: <200801292158.m0TLwA7u019321@mi0.bluebottle.com> しらいしななこ writes: > Quoting Junio C Hamano : >... >> 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.