[PATCH] cgit: ignore SIGPIPE

[John Keeping]

We check the return value from write() and die with an error message, so
the only effects of using the default SIGPIPE handler is to suppress the
error message and change the exit status.

Web servers tend to ignore the exit code, so it is likely to be more
helpful to administrators if we print some form of error message with a
more generic exit status rather than die silently with
WTERMSIG() == SIGPIPE.

Signed-off-by: John Keeping <john at keeping.me.uk>
---
On Sat, Oct 01, 2016 at 12:33:00PM +0100, John Keeping wrote:
> On Thu, Sep 29, 2016 at 04:38:41PM +0200, Vadim Zeitlin wrote:
> >  As this is the first time I'm posting here, let me start by thanking you
> > for developing cgit! I'm using it since quite some time under Debian
> > (currently Jessie) and it works very well but recently I've mistakenly
> > removed highlight package because I thought it was redundant with other
> > similar tools and completely forgot that cgit depended on it.
> > 
> >  Since then, viewing any files in cgit didn't work, which was probably only
> > to be expected, but viewing sufficiently large files resulted in 500 server
> > error due to, after debugging, SIGPIPE received but not handled by cgit
> > when reading from the filter subprocess.
> > 
> >  I'm not sure what exactly should be done here: maybe nothing, and people
> > should be just more careful with uninstalling dependencies. But I think
> > ideally cgit should give an error indicating that filter execution failed
> > and I'd at least expect it not to just die without any further information.
> 
> If we ignore SIGPIPE we should just hit the die_errno() in
> html.c::html_raw() which will say "write error on html output".  That's
> not really as useful as it could be, since that code doesn't care
> whether or not it's writing via a filter.
> 
> I'm not sure if we want to start keeping track of whether we're writing
> through a filter, but some form of error message rather than a silent
> failure does seem like an improvment.  I think we just need to ignore
> SIGPIPE to make that happen.

Something like this.

 cgit.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/cgit.c b/cgit.c
index 2f29aa6..1a94144 100644
--- a/cgit.c
+++ b/cgit.c
@@ -1031,6 +1031,9 @@ int cmd_main(int argc, const char **argv)
 	const char *path;
 	int err, ttl;
 
+	if (signal(SIGPIPE, SIG_IGN) == SIG_ERR)
+		fprintf(stderr, "cgit warning: failed to ignore SIGPIPE\n");
+
 	cgit_init_filters();
 	atexit(cgit_cleanup_filters);
 
-- 
2.10.0.278.g4f427b1