Git Mailing List Archive mirror
 help / color / mirror / Atom feed
* Non-zero exit code of custom filter process is not handled
@ 2023-04-26 14:21 Michael Voříšek - ČVUT FJFI
  2023-04-27  8:40 ` Jeff King
  0 siblings, 1 reply; 2+ messages in thread
From: Michael Voříšek - ČVUT FJFI @ 2023-04-26 14:21 UTC (permalink / raw)
  To: git

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



^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: Non-zero exit code of custom filter process is not handled
  2023-04-26 14:21 Non-zero exit code of custom filter process is not handled Michael Voříšek - ČVUT FJFI
@ 2023-04-27  8:40 ` Jeff King
  0 siblings, 0 replies; 2+ messages in thread
From: Jeff King @ 2023-04-27  8:40 UTC (permalink / raw)
  To: Michael Voříšek - ČVUT FJFI; +Cc: git

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-04-27  8:40 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-26 14:21 Non-zero exit code of custom filter process is not handled Michael Voříšek - ČVUT FJFI
2023-04-27  8:40 ` Jeff King

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).