← Back to context

Comment by Groxx

18 hours ago

Seems like this would probably be solved if github returned a patch file formatted like `git show` provides, specifically with the commit message indented? I do see that `git format-patch` doesn't do this indentation though.

In any case, agreed that it's not a great "feature" to use in-band signaling of when patch data starts, with no escaping. Confusion and misbehavior is pretty much guaranteed.

3 comments

Groxx

Reply
thaumasiotes  16 hours ago

> Seems like this would probably be solved if github returned a patch file formatted like `git show` provides, specifically with the commit message indented? I do see that `git format-patch` doesn't do this indentation though.

This would be "solved" if the patch file only included the patch. That's pretty straightforward. The file github provides includes fake email headers for no particular reason. The commit message appears to be part of the subject header. The subject header is never terminated, so arguably applying this patch shouldn't do anything. (Because the actual patch data is also part of the email headers.) The other headers aren't terminated either, so actually there is no subject header. This shouldn't really matter, because the patch file isn't email, but it does seem to want to pretend to be.

The usual question to ask here would be "why are you applying patch files from an untrusted source?". If patch(1) was stricter about the format of its input files... applying patches from an untrusted source would still be a good way to get owned. If you think I can get you to patch inappropriate files by writing a fake diff into my commit message... wait until you see what I can do by writing those same changes into the real diff.

  • quuxplusone  14 hours ago

    > fake email headers

    That's the output you get from `git format-patch --stdout -1 dd28283`. The idea is that it's suitable for emailing to a mailing list for review (hence the subject line beginning with [PATCH], and so on).

    If you ask for colorized output with `git format-patch --color=always --stdout -1 dd28283` you'll see that `git format-patch` itself knows which bits are the commit message and which bits are the diff. (Well, of course it does, I guess.)

    I suspect that if you sent a patch like this to the mailing list, they'd get mad at you. So `git format-patch` is working OK for its intended use-case. Arguably it's GitHub causing the problem here by "misusing" `git format-patch` as a way to deliver patches that are (these days) expected to be machine-readable — something you can just curl and pipe into `patch`. `git format-patch` doesn't do that.

    That said, yeah, it's amusing that (as TFA says)

        git format-patch -1 HEAD --stdout > 0001 ;
    git checkout HEAD~ ;
    git am 0001

    isn't a clean round-trip. `git am` applies the fake diff.

    > If you think I can get you to patch inappropriate files by writing a fake diff into my commit message... wait until you see what I can do by writing those same changes into the real diff.

    Well put. :)