Re: [PATCH v2 1/4] apply: reject input that touches outside $cwd

From: Jeff King <peff@peff.net>
Date: Feb 3, 2015, 00:45 UTC
Message-ID: <20150203004507.GA31946@peff.net>
In-Reply-To: <1422919650-13346-2-git-send-email-gitster@pobox.com>

On Mon, Feb 02, 2015 at 03:27:27PM -0800, Junio C Hamano wrote:

Show 12 quoted lines

> By default, a patch that affects outside the working area is
> rejected as a mistake (or a mischief); Git itself does not create
> such a patch, unless the user bends backwards and specifies a
> non-standard prefix to "git diff" and friends.
> 
> When `git apply` is used without either `--index` or `--cached`
> option as a "better GNU patch", the user can pass `--unsafe-paths`
> option to override this safety check.  This cannot be used to escape
> outside the working tree when using `--index` or `--cached` to apply
> the patch to the index.
> 
> The new test was stolen from Jeff King with slight enhancements.

I notice that this includes the symlink-crossing tests, marked as failures. Reading the series, I know what is going to happen later, but do you want to leave a note like:

  Note that we also add tests for leaving the working directory by
  crossing symlink boundaries, which is not addressed in this patch.
  That is a separate issue caused following symlinks, which will come
  later.

or something to help later readers of "git log"?

Show 5 quoted lines

> +--unsafe-paths::
> +	By default, a patch that affects outside the working area is
> +	rejected as a mistake (or a mischief); Git itself never
> +	creates such a patch unless the user bends backwards and
> +	specifies nonstandard prefix to "git diff" and friends.

Minor wordsmithing: the usual idiom is "bend over backwards", and you probably want s/specifies/& a/.

> ++
> +When `git apply` is used without either `--index` or `--cached`
> +option as a "better GNU patch", the user can pass `--unsafe-paths`
> +option to override this safety check.

Similarly, probably every instance of "foo option" would read better as "the foo option".

> This cannot be used to escape
> +outside the working tree when using `--index` or `--cached` to apply
> +the patch to the index.

I had trouble figuring out what this meant. Would it be simpler to just say:

  This option has no effect when `--index` or `--cached` is in use.

Or is there some other subtlety that you are trying to convey that I am missing?

-Peff