Non-zero exit code of custom filter process is not handled

Non-zero exit code of custom filter process is not handled

[Michael Voříšek - ČVUT FJFI]

Hello everyone,


I would like to report https://github.com/gitgitgadget/git/issues/1516 .

When a custom filter process exits with a non-zero code, the git 
currently tries to decode the response, even if it should fail and let 
the user know the problem is not the payload, but instead of the filter 
process.

You can reproduce this with setting a git filter to "php 
non-existent.php", in this case the filter (php binary) will exit with 
non-zero code. Currently "fatal: protocol error: bad line length 
character: Coul" is printed which is very hard to understand as even not 
the whole error string is shown.

Thank you for looking into this.


With kind regards / Mit freundlichen Grüßen / S přátelským pozdravem,

Michael Voříšek, student
ČVUT FJFI

Re: Non-zero exit code of custom filter process is not handled

[Jeff King]

On Wed, Apr 26, 2023 at 04:21:31PM +0200, Michael Voříšek - ČVUT FJFI wrote:

> When a custom filter process exits with a non-zero code, the git currently
> tries to decode the response, even if it should fail and let the user know
> the problem is not the payload, but instead of the filter process.

Isn't there an ordering problem here? We will not see the exit code
until we wait() for the process. And we will not wait for it to finish
until we get EOF on the pipe we are reading from. So we will read what
it says (if anything) before checking the exit code.

What would normally happen is something like:

  1. The process encounters an error. It prints a message to stderr
     (which is usually pointing at the terminal or a log), says nothing
     else to stdout, and then exits with a failing code.

  2. Git sees EOF on the pipe, which has been closed.

  3. Git calls wait() and sees the failing exit code.

> You can reproduce this with setting a git filter to "php non-existent.php",
> in this case the filter (php binary) will exit with non-zero code. Currently
> "fatal: protocol error: bad line length character: Coul" is printed which is
> very hard to understand as even not the whole error string is shown.

The problem here is that php prints "Could not open input file:
non-existent.php" to stdout, not stderr. So it really is sending garbage
over the pipe from which Git expects it to be speaking a particular
protocol. Whether it exits with a non-zero code or not, it has broken
the protocol and Git is correct to complain.

It might be helpful in such a case if the pkt-line reader showed the
whole string it received, rather than just the first four bytes which it
is trying to parse (though I suspect in some cases it may require extra
read() calls, as the data may still be in the pipe buffer).

-Peff