"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
Re: got patch vs git diff with renames
To:
Stefan Sperling <stsp@stsp.name>
Cc:
gameoftrees@openbsd.org
Date:
Fri, 13 May 2022 17:30:01 +0200

Download raw body.

Thread
Stefan Sperling <stsp@stsp.name> wrote:
> On Fri, May 13, 2022 at 01:06:19PM +0200, Omar Polo wrote:
> > Omar Polo <op@omarpolo.com> wrote:
> > > Stefan Sperling <stsp@stsp.name> wrote:
> > > > On Fri, Apr 29, 2022 at 11:08:41AM +0200, Omar Polo wrote:
> > > > > [...]
> > > > > P.S.: i forgot that we supported the rename with an empty hunk "@@ -0,0
> > > > > +0,0 @@".  Since git doesn't seem to handle it, and neither patch(1), I
> > > > > intend to remove the support for it, but again, in a follow-up commit :)
> > > > 
> > > > Agreed. If nobody else uses such empty hunks as a rename marker, then we
> > > > shouldn't rely on this either.
> > > 
> > > here's the diff for it.
> > > 
> > > I'm still undecided if checking for empty hunks in patch.c too as a
> > > safety net is a good idea given that got-read-patch now fails for those.
> > > 
> > > The rationale would be that if got-read-patch somehow misses that case,
> > > we end up with a NULL h->lines that causes a segfault in patch_file.
> > > Maybe we can just assume the libexec helper sends good data?  I'm
> > > probably too paranoic.
> 
> Well, you should always assume that libexec code is trying to trick you
> by sending crap over imsg pipes. There is nothing that would prevent
> this when someone manages to obtain code execution in libexec helpers.

right, i feared i was too paranoic but yes, it's the libexec helper that
handles the untrusted data, so it can be compromised.  I've committed
this as-is then, thanks!