Re: Bug in 'git am' when applying a broken patch

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 26, 2015, 20:58 UTC
Message-ID: <xmqq7fqqtceq.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CAGZ79kYbyTOeEvJBPqWOX8fxbB637N5aV3Q=yENQXu4v9FzBPQ@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

Show 5 quoted lines

> In the hunk header we can learn about the
> expected lines to read for this hunk and after the hunk we only have
> 3 possible lines:
>
>   * it's the next hunk, then the line starts with @@

This is true.

>   * it's a new file, so the line starts with "diff --git"

This is true with s/--git//.

>   * it's the end of the patch, so the line is "--\n" and the line there after
>     is version number as git describe puts (not sure we want to test on that)

This is not true in general, as we do not want to limit "git apply" to only what "git diff" produces. You can write anything after a patch and that is still a valid patch. And that anything could be a line that begins with '-', ' ' and '+'; as long as the line numbers in the hunk header are correct, we'd ignore it.

So as you said, the change you are responding to is "better than nothing", and would only help when you truncate the patch (or break the numbers), but does not protect against arbitrary breakage.

One thing we _could_ do is after seeing the end of a message (i.e. we did not see "@@" that signals there are more hunks in the current patch, and we did not see "diff " that signals there are more patches), we keep scanning and declare breakage if we see lines that begin with something that looks like a hunk "@@ ... @@".