Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries

2013-04-10 Thread Ramkumar Ramachandra
Junio C Hamano wrote: And I also misread we currently don't handle above as but we really should allow adding d/f when d is at the top of the working tree of another project, but that was not what you meant to say. Instead, We do not notice such a bad case in today's code yet was what you

Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries

2013-04-10 Thread Ramkumar Ramachandra
Junio C Hamano wrote: One can have symlinks to anywhere all one wants. We track symlinks. [...] Yes, I know. We store symlinks as blobs containing one line, the path to the file, without a trailing newline. And we have a mode for it to distinguish it from regular files. What I meant is:

Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries

2013-04-10 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes: Exactly. Yeah, I don't think you patch makes sense as a standalone anyway. Yes, it was purely a preparatory step. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More

[PATCH 2/2] add: refuse to add paths beyond repository boundaries

2013-04-09 Thread Ramkumar Ramachandra
Currently, git add has the logic for refusing to add gitlinks using treat_path(), which in turn calls check_path_for_gitlink(). However, this only checks for an in-index submodule (or gitlink cache_entry). A path inside a git repository in the worktree still adds fine, and this is a bug. The

Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries

2013-04-09 Thread Jeff King
On Tue, Apr 09, 2013 at 02:51:37PM +0530, Ramkumar Ramachandra wrote: Currently, git add has the logic for refusing to add gitlinks using treat_path(), which in turn calls check_path_for_gitlink(). However, this only checks for an in-index submodule (or gitlink cache_entry). A path inside a

Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries

2013-04-09 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes: Currently, git add has the logic for refusing to add gitlinks using treat_path(), which in turn calls check_path_for_gitlink(). However, this only checks for an in-index submodule (or gitlink cache_entry). A path inside a git repository in the

Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries

2013-04-09 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes: Ramkumar Ramachandra artag...@gmail.com writes: Currently, git add has the logic for refusing to add gitlinks using treat_path(), which in turn calls check_path_for_gitlink(). However, this only checks for an in-index submodule (or gitlink

Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries

2013-04-09 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes: I looked at the output from grep has_symlink_leading_path and also for die_if_path_beyond; all of these places are checking I have this multi-level path; I want to know if the path does not (should not) be part of the current project, I think.

Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries

2013-04-09 Thread Ramkumar Ramachandra
Junio C Hamano wrote: The first step (renaming and adjusting comments) would look like this. Thanks for this! I like the name die_if_path_outside_our_project(). I'll take care of the rest.` -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to

Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries

2013-04-09 Thread Junio C Hamano
Sorry for repeated rerolls. I had missed another instance in t0008 and also the explanation was lacking. -- 8 -- Subject: [PATCH] symlinks: rename has_symlink_leading_path() to path_outside_our_project() The purpose of the function is to prevent a path from getting added to our project when

Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries

2013-04-09 Thread Ramkumar Ramachandra
Junio C Hamano wrote: I think what the callers of this function care about is if the name is a path that should not be added to our index (i.e. points outside the repository). If you had a symlink d that points at e when our project does have a subdirectory e with file f,

Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries

2013-04-09 Thread Ramkumar Ramachandra
Junio C Hamano wrote: But there are other cases to attempt to add a path that do not belong to our project, which do not have to involve a symbolic link in the leading path. The reader is now wondering what this could possibly be, and why you didn't send this patch earlier. Perhaps clarify

Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries

2013-04-09 Thread Jakub Narębski
W dniu 09.04.2013 19:34, Junio C Hamano pisze: - if (has_symlink_leading_path(path, len)) - return error('%s' is beyond a symbolic link, path); + if (path_outside_our_project(path, len)) + return error('%s' is outside our working tree, path); Don't we lose

Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries

2013-04-09 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: I think what the callers of this function care about is if the name is a path that should not be added to our index (i.e. points outside the repository). If you had a symlink d that points at e when our project does have

Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries

2013-04-09 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes: The reader is now wondering what this could possibly be, and why you didn't send this patch earlier. Because it wasn't written back then? Perhaps clarify with: s/there are cases/there may be cases/ and append One such case that we currently

Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries

2013-04-09 Thread Junio C Hamano
Jakub Narębski jna...@gmail.com writes: W dniu 09.04.2013 19:34, Junio C Hamano pisze: -if (has_symlink_leading_path(path, len)) -return error('%s' is beyond a symbolic link, path); +if (path_outside_our_project(path, len)) +return error('%s' is outside our

Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries

2013-04-09 Thread Jakub Narębski
Junio C Hamano wrote: Jakub Narębski jna...@gmail.com writes: W dniu 09.04.2013 19:34, Junio C Hamano pisze: - if (has_symlink_leading_path(path, len)) - return error('%s' is beyond a symbolic link, path); + if (path_outside_our_project(path, len)) + return

Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries

2013-04-09 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes: Ramkumar Ramachandra artag...@gmail.com writes: The reader is now wondering what this could possibly be, and why you didn't send this patch earlier. Because it wasn't written back then? Perhaps clarify with: s/there are cases/there may be cases/

Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries

2013-04-09 Thread Junio C Hamano
Jakub Narębski jna...@gmail.com writes: Junio C Hamano wrote: Jakub Narębski jna...@gmail.com writes: W dniu 09.04.2013 19:34, Junio C Hamano pisze: - if (has_symlink_leading_path(path, len)) - return error('%s' is beyond a symbolic link, path); + if

Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries

2013-04-09 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: But there are other cases to attempt to add a path that do not belong to our project, which do not have to involve a symbolic link in the leading path. The reader is now wondering what this could possibly be, and why you