On Fri, Aug 29, 2025 at 03:31:45PM +0200, Omar Polo wrote:
> Stefan Sperling <stsp@stsp.name> wrote:
> > On Fri, Aug 29, 2025 at 11:15:17AM +0200, Stefan Sperling wrote:
> > > If the patch file lacks a trailing newline just before end-of-file
> > > then 'got patch' will exit with an "unexpected privsep message" error.
> > > 
> > > Regression test and fix below.
> > > 
> > > ok?
> > 
> > Imrpoved version which fixes a function name in an error I overlooked
> > ("malloc" -> "strdup") and reorders the checks in recv_patch() for
> > cosmetic reasons.
> 
> mixed feelings, sorry.  I'm not sure about many of these changes from
> the linelen to strlen(): before I was trying to also support the case of
> NUL bytes in the middle of a line.
> 
> Now, I know it's not an important thing, patch is for text stuff, but
> from time to time some binary thing could slip, or maybe it's still text
> but in other encoding (hello utf16), so I think it's worth to at least
> give it a try at not breaking that easily on NUL bytes.
> 
> unfortunately at the moment I lack the time to propose a better diff,
> hopefully i'll be able to take a closer look tonight.

linelen from getline(3) tells us how many bytes were read from the file,
which could indeed be larger than strlen(line) if the line contains a
stray NUL byte.

And we are guaranteed a NUL-terminated output string from getline() but
the trailing newline is not guaranteed.

If a line does not end in a newline then with our current code this check
fails at the receiving end: t[datalen-1] != '\0'

So you'd need to figure out why that happens.
Using strlen() as length indicator is the easiest way I found to avoid this.

I don't think we need to support anything other than UTF-8. But there is
the new test case you can play with if you want to investigate other ways
of fixing this. As long as the tests are passing we should be fine.