{"thread":{"id":"59660","subject":"Non-zero exit code of custom filter process is not handled","startedAt":"2023-04-26T15:57:15Z","lastAt":"2023-04-27T08:40:41Z","messageCount":2,"participants":["Michael Voříšek - ČVUT FJFI","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"476117","messageId":"a7cf2f3bbc6dcd8758e188f79d31e6e0bff44ea3e768fce482309ba532205f6b@mahalux.com","threadId":"59660","inReplyTo":null,"subject":"Non-zero exit code of custom filter process is not handled","fromName":"Michael Voříšek - ČVUT FJFI","fromEmail":"vorismi3@fjfi.cvut.cz","sentAt":"2023-04-26T14:21:31Z","receivedAt":"2023-04-26T15:57:15Z","isPatch":false,"sender":{"key":"vorismi3@fjfi.cvut.cz","avatar":null},"body":"Hello everyone,\n\n\nI would like to report https://github.com/gitgitgadget/git/issues/1516 .\n\nWhen a custom filter process exits with a non-zero code, the git \ncurrently tries to decode the response, even if it should fail and let \nthe user know the problem is not the payload, but instead of the filter \nprocess.\n\nYou can reproduce this with setting a git filter to \"php \nnon-existent.php\", in this case the filter (php binary) will exit with \nnon-zero code. Currently \"fatal: protocol error: bad line length \ncharacter: Coul\" is printed which is very hard to understand as even not \nthe whole error string is shown.\n\nThank you for looking into this.\n\n\nWith kind regards / Mit freundlichen Grüßen / S přátelským pozdravem,\n\nMichael Voříšek, student\nČVUT FJFI\n\n\n"},{"id":"476184","messageId":"20230427084034.GA1479657@coredump.intra.peff.net","threadId":"59660","inReplyTo":"a7cf2f3bbc6dcd8758e188f79d31e6e0bff44ea3e768fce482309ba532205f6b@mahalux.com","subject":"Re: Non-zero exit code of custom filter process is not handled","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-04-27T08:40:34Z","receivedAt":"2023-04-27T08:40:41Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 26, 2023 at 04:21:31PM +0200, Michael Voříšek - ČVUT FJFI wrote:\n\n> When a custom filter process exits with a non-zero code, the git currently\n> tries to decode the response, even if it should fail and let the user know\n> the problem is not the payload, but instead of the filter process.\n\nIsn't there an ordering problem here? We will not see the exit code\nuntil we wait() for the process. And we will not wait for it to finish\nuntil we get EOF on the pipe we are reading from. So we will read what\nit says (if anything) before checking the exit code.\n\nWhat would normally happen is something like:\n\n  1. The process encounters an error. It prints a message to stderr\n     (which is usually pointing at the terminal or a log), says nothing\n     else to stdout, and then exits with a failing code.\n\n  2. Git sees EOF on the pipe, which has been closed.\n\n  3. Git calls wait() and sees the failing exit code.\n\n> You can reproduce this with setting a git filter to \"php non-existent.php\",\n> in this case the filter (php binary) will exit with non-zero code. Currently\n> \"fatal: protocol error: bad line length character: Coul\" is printed which is\n> very hard to understand as even not the whole error string is shown.\n\nThe problem here is that php prints \"Could not open input file:\nnon-existent.php\" to stdout, not stderr. So it really is sending garbage\nover the pipe from which Git expects it to be speaking a particular\nprotocol. Whether it exits with a non-zero code or not, it has broken\nthe protocol and Git is correct to complain.\n\nIt might be helpful in such a case if the pkt-line reader showed the\nwhole string it received, rather than just the first four bytes which it\nis trying to parse (though I suspect in some cases it may require extra\nread() calls, as the data may still be in the pipe buffer).\n\n-Peff\n"}]}