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.
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.
This means that it will try to apply any unindented diffs in the commit message. But you’re fine if you indent the diff. (Newschool code fencers will have a worse time here.)
I imagine that this worked fine for changes that were authored by one person and submitted by another person via email, or by their friend, or by someone trying to resurrect a previous attempt at getting something upstreamed. Someone is likely to notice that examples diffs are getting applied. But it won’t work well at all if you are some software distributor who is using patch files to apply modifications to packages.
Recall that git-am(1) will not apply indented diffs. Well have a look at my GNU patch 2.7.6:
If the entire diff is indented by a consistent amount, if lines end in
CRLF, or if a diff is encapsulated one or more times by prepending "- "
to lines starting with "-" as specified by Internet RFC 934, this is
taken into account.
Larry Wall wrote the original version of patch.
LiamPowell•1h ago
This wouldn't be an issue if patches were XML or JSON with a well defined schema, but everything must be a boutique undocumented format in the world of Unix tools.
Maybe the worst part about this is that it can entirely come from a patch being exported by git and then imported straight back in to git. If you can't even handle your own undocumented format then what hope do other tools have that want to work with it?
[1]: https://mas.to/@zekjur/116022397626943871
yjftsjthsd-h•1h ago
Patch files are readable by humans. Replacing them with XML or JSON would fix this problem, but at the expense of removing a core feature.
syntheticnature•1h ago
tetha•26m ago
Or, more snarky: tee is also a huge security problem if you pipe untrusted input into `tee -a /etc/passwd`, such as `curl | tee -a /etc/passwd`. Not many things are safe with a `curl |` in front of them. I think yes might be?
pwdisswordfishs•37m ago
Or MIME, even.
kevin_thibedeau•25m ago
chrishill89•23m ago
No one wants to apply diffs in commit messages. But some people use this technique via email:
They’ve used the `---` commit message delimiter in the commit message itself so that everything after it won’t be applied by git-am(1). So that’s intentional loss of round tripping.I would personally use Git notes instead though.