Re: [PATCH] apply: refuse touching a file beyond symlink

2015-01-30 Thread Junio C Hamano
Jeff King p...@peff.net writes: Ah, OK. Yeah, doing it progressively can only be accurate if our name-checks follow the same order as applying, because we are checking against a particular state. But could we instead pull this check to just before the write-out time? That is, to let any

Re: [PATCH] apply: refuse touching a file beyond symlink

2015-01-30 Thread Junio C Hamano
Jeff King p...@peff.net writes: I had the impression that we did not apply in any arbitrary order that could work, but rather that we did deletions first followed by additions. But I am fairly ignorant of the apply code. No, you are thinking about the write-out of the finished result, which

Re: [PATCH] apply: refuse touching a file beyond symlink

2015-01-30 Thread Jeff King
On Fri, Jan 30, 2015 at 12:20:02PM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: I had the impression that we did not apply in any arbitrary order that could work, but rather that we did deletions first followed by additions. But I am fairly ignorant of the apply code.

Re: [PATCH] apply: refuse touching a file beyond symlink

2015-01-30 Thread Junio C Hamano
Jeff King p...@peff.net writes: But could we instead pull this check to just before the write-out time? That is, to let any horrible thing happen in-core, as long as what we write out to the index and the filesystem is sane? The check in-core is somewhat tricky, because we would need to (1)

Re: [PATCH] apply: refuse touching a file beyond symlink

2015-01-30 Thread Junio C Hamano
Jeff King p...@peff.net writes: +if (!patch-is_delete path_is_beyond_symlink(patch-new_name)) +return error(_(affected file '%s' is beyond a symbolic link), + patch-new_name); Why does this not kick in when deleting a file? If it is not OK to add

Re: [PATCH] apply: refuse touching a file beyond symlink

2015-01-30 Thread Junio C Hamano
Jeff King p...@peff.net writes: On Thu, Jan 29, 2015 at 12:45:22PM -0800, Junio C Hamano wrote: +if (!patch-is_delete path_is_beyond_symlink(patch-new_name)) +return error(_(affected file '%s' is beyond a symbolic link), + patch-new_name); Why does

Re: [PATCH] apply: refuse touching a file beyond symlink

2015-01-30 Thread Junio C Hamano
Jeff King p...@peff.net writes: Hrm. That only works in the current code because we apply the deletion in the directory (and then clean up the now-empty directory) first. So I think you would need to check the paths progressively as you apply them, since those other parts of the diff haven't

Re: [PATCH] apply: refuse touching a file beyond symlink

2015-01-30 Thread Jeff King
On Thu, Jan 29, 2015 at 12:45:22PM -0800, Junio C Hamano wrote: +static int path_is_beyond_symlink(const char *name_) +{ + struct strbuf name = STRBUF_INIT; + + strbuf_addstr(name, name_); + do { + struct patch *previous; + + while (--name.len

Re: [PATCH] apply: refuse touching a file beyond symlink

2015-01-30 Thread Jeff King
On Fri, Jan 30, 2015 at 11:48:14AM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: + if (!patch-is_delete path_is_beyond_symlink(patch-new_name)) + return error(_(affected file '%s' is beyond a symbolic link), + patch-new_name); Why does

Re: [PATCH] apply: refuse touching a file beyond symlink

2015-01-30 Thread Jeff King
On Fri, Jan 30, 2015 at 12:11:30PM -0800, Junio C Hamano wrote: I am not sure how to fix this, without completely ripping out the misguided We should be able to concatenate outputs from multiple invocations of 'git diff' into a single file and apply the result with a single invocation of 'git

Re: [PATCH] apply: refuse touching a file beyond symlink

2015-01-30 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes: Jeff King p...@peff.net writes: + if (!patch-is_delete path_is_beyond_symlink(patch-new_name)) + return error(_(affected file '%s' is beyond a symbolic link), +patch-new_name); Why does this not kick in when

Re: [PATCH] apply: refuse touching a file beyond symlink

2015-01-30 Thread Jeff King
On Fri, Jan 30, 2015 at 11:42:49AM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: On Thu, Jan 29, 2015 at 12:45:22PM -0800, Junio C Hamano wrote: + if (!patch-is_delete path_is_beyond_symlink(patch-new_name)) + return error(_(affected file '%s' is beyond a

Re: [PATCH] apply: refuse touching a file beyond symlink

2015-01-30 Thread Christian Couder
On Thu, Jan 29, 2015 at 9:45 PM, Junio C Hamano gits...@pobox.com wrote: Instead, for any patch in the input that leaves a path (i.e. a non deletion) in the result, we check all leading paths against interim result and then either the index or the working tree. The interim results of

[PATCH] apply: refuse touching a file beyond symlink

2015-01-29 Thread Junio C Hamano
Because Git tracks symbolic links as symbolic links, a path that has a symbolic link in its leading part (e.g. path/to/dir/file, where path/to/dir is a symbolic link to somewhere else, be it inside or outside the working tree) can never appear in a patch that validly applies, unless the same patch

Re: [PATCH] apply: refuse touching a file beyond symlink

2015-01-29 Thread Stefan Beller
On Thu, Jan 29, 2015 at 12:45 PM, Junio C Hamano gits...@pobox.com wrote: + +test_expect_success SYMLINKS 'do not follow symbolic link (same input)' ' + + # same input creates a confusihng symbolic link s/confusihng/confusing/ -- To unsubscribe from this list: send the line unsubscribe