6 files changed, 50 insertions, 4 deletions

diff --git a/source/cache.c b/source/cache.cindex 8c81f5c..62bb623 100644--- a/source/cache.c+++ b/source/cache.c@@ -175,7 +175,8 @@ static int serve_slot(struct cache_slot *slot) 	int err;  	err = print_slot(slot);-	if (err)+	// A client that went away mid-page is not trouble worth a line.+	if (err && err != EPIPE) 		log_error("[cgit] Unable to send slot %s: %s (%d)\n", slot->path, strerror(err), err); 	return err; }
diff --git a/source/cache.c b/source/cache.c
index 8c81f5c..62bb623 100644
--- a/source/cache.c
+++ b/source/cache.c
@@ -175,7 +175,8 @@ static int serve_slot(struct cache_slot *slot)
 	int err;
 
 	err = print_slot(slot);
-	if (err)
+	// A client that went away mid-page is not trouble worth a line.
+	if (err && err != EPIPE)
 		log_error("[cgit] Unable to send slot %s: %s (%d)\n", slot->path, strerror(err), err);
 	return err;
 }
diff --git a/source/cgit.c b/source/cgit.cindex 3e47dbe..633b011 100644--- a/source/cgit.c+++ b/source/cgit.c@@ -1380,7 +1380,9 @@ int cmd_main(int argc, const char **argv) 	strbuf_release(&cache_key);  	cgit_cleanup_filters();-	if (err)+	// A slot that could not be sent because the client went away is not+	// worth an error page nobody will read.+	if (err && err != EPIPE) 		cgit_print_error("Error processing page: %s (%d)", strerror(err), err); 	return err; }
diff --git a/source/cgit.c b/source/cgit.c
index 3e47dbe..633b011 100644
--- a/source/cgit.c
+++ b/source/cgit.c
@@ -1380,7 +1380,9 @@ int cmd_main(int argc, const char **argv)
 	strbuf_release(&cache_key);
 
 	cgit_cleanup_filters();
-	if (err)
+	// A slot that could not be sent because the client went away is not
+	// worth an error page nobody will read.
+	if (err && err != EPIPE)
 		cgit_print_error("Error processing page: %s (%d)", strerror(err), err);
 	return err;
 }
diff --git a/source/filter.c b/source/filter.cindex dd1eaf0..6e16537 100644--- a/source/filter.c+++ b/source/filter.c@@ -450,6 +450,11 @@ void cgit_abort_filters(void) #endif } +int cgit_filter_holds_stdout(void)+{+	return running_exec != NULL;+}+ static const struct { 	const char *prefix; 	struct cgit_filter *(*create)(const char *cmd, int argument_count);
diff --git a/source/filter.c b/source/filter.c
index dd1eaf0..6e16537 100644
--- a/source/filter.c
+++ b/source/filter.c
@@ -450,6 +450,11 @@ void cgit_abort_filters(void)
 #endif
 }
 
+int cgit_filter_holds_stdout(void)
+{
+	return running_exec != NULL;
+}
+
 static const struct {
 	const char *prefix;
 	struct cgit_filter *(*create)(const char *cmd, int argument_count);
diff --git a/source/filter.h b/source/filter.hindex b5a478f..70fa4ff 100644--- a/source/filter.h+++ b/source/filter.h@@ -59,4 +59,10 @@ extern void cgit_cleanup_filters(void);  */ extern void cgit_abort_filters(void); +/*+ * Whether an exec filter holds stdout at the moment, which is when a failed+ * write means the filter went away rather than the client.+ */+extern int cgit_filter_holds_stdout(void);+ #endif // CGIT_FILTER_H
diff --git a/source/filter.h b/source/filter.h
index b5a478f..70fa4ff 100644
--- a/source/filter.h
+++ b/source/filter.h
@@ -59,4 +59,10 @@ extern void cgit_cleanup_filters(void);
  */
 extern void cgit_abort_filters(void);
 
+/*
+ * Whether an exec filter holds stdout at the moment, which is when a failed
+ * write means the filter went away rather than the client.
+ */
+extern int cgit_filter_holds_stdout(void);
+
 #endif // CGIT_FILTER_H
diff --git a/source/html.c b/source/html.cindex 96b1b52..caa5a29 100644--- a/source/html.c+++ b/source/html.c@@ -10,6 +10,7 @@  #include "cgit.h" #include "cache.h"+#include "filter.h" #include "html.h"  #define HTML_WRITE_BUFSIZE (64 * 1024)@@ -57,6 +58,8 @@ static struct strbuf *capture;  static void write_out(const char *data, size_t size) {+	int err;+ 	// A blob or snapshot is well past what one write can move onto a 	// pipe, so short writes are resumed rather than reported. 	while (size) {@@ -69,12 +72,25 @@ static void write_out(const char *data, size_t size) 	} 	if (!size) 		return;+	err = errno; 	// While a cache slot is being filled stdout is a file, and a full disk 	// must not cost the visitor the page. Abandoning the fill puts stdout 	// back on the client along with what the file already holds, so only 	// the rest is written again.-	if (!cache_abandon_fill() || write_in_full(STDOUT_FILENO, data, size) < 0)-		die_errno("Unable to write the page");+	if (cache_abandon_fill()) {+		if (write_in_full(STDOUT_FILENO, data, size) < 0)+			die_errno("Unable to write the page");+		return;+	}+	// A client that has gone away fails the write with EPIPE and leaves+	// nobody to read an error page, so the request ends quietly. A filter+	// that exited early fails it the same way, and that page has a reader.+	if (err == EPIPE && !cgit_filter_holds_stdout()) {+		html_discard();+		exit(0);+	}+	errno = err;+	die_errno("Unable to write the page"); }  /*
diff --git a/source/html.c b/source/html.c
index 96b1b52..caa5a29 100644
--- a/source/html.c
+++ b/source/html.c
@@ -10,6 +10,7 @@
 
 #include "cgit.h"
 #include "cache.h"
+#include "filter.h"
 #include "html.h"
 
 #define HTML_WRITE_BUFSIZE (64 * 1024)
@@ -57,6 +58,8 @@ static struct strbuf *capture;
 
 static void write_out(const char *data, size_t size)
 {
+	int err;
+
 	// A blob or snapshot is well past what one write can move onto a
 	// pipe, so short writes are resumed rather than reported.
 	while (size) {
@@ -69,12 +72,25 @@ static void write_out(const char *data, size_t size)
 	}
 	if (!size)
 		return;
+	err = errno;
 	// While a cache slot is being filled stdout is a file, and a full disk
 	// must not cost the visitor the page. Abandoning the fill puts stdout
 	// back on the client along with what the file already holds, so only
 	// the rest is written again.
-	if (!cache_abandon_fill() || write_in_full(STDOUT_FILENO, data, size) < 0)
-		die_errno("Unable to write the page");
+	if (cache_abandon_fill()) {
+		if (write_in_full(STDOUT_FILENO, data, size) < 0)
+			die_errno("Unable to write the page");
+		return;
+	}
+	// A client that has gone away fails the write with EPIPE and leaves
+	// nobody to read an error page, so the request ends quietly. A filter
+	// that exited early fails it the same way, and that page has a reader.
+	if (err == EPIPE && !cgit_filter_holds_stdout()) {
+		html_discard();
+		exit(0);
+	}
+	errno = err;
+	die_errno("Unable to write the page");
 }
 
 /*
diff --git a/tests/t0303-robustness.sh b/tests/t0303-robustness.shindex 55a83c3..fd1c02e 100755--- a/tests/t0303-robustness.sh+++ b/tests/t0303-robustness.sh@@ -529,6 +529,22 @@ test_expect_success 'the dumb transport withholds the alternates file' ' 	grep "^Status: 200" tmp ' +# A client that goes away mid-page leaves nobody to send an error page to,+# so the request ends quietly instead of dying twice into the log. The page+# has to outgrow the pipe for the write after the reader has gone to fail.+test_expect_success 'a client that disconnects ends the request quietly' '+	(+		cd repos/rob &&+		awk "BEGIN{for(i=0;i<20000;i++) print \"line \" i}" >long.txt &&+		git add long.txt &&+		git commit -m long+	) &&+	{ robq "url=rob/tree/long.txt" 2>err; echo $? >status; } | head -c 1 >/dev/null &&+	test "$(cat status)" = 0 &&+	! grep -i "broken pipe" err &&+	! grep "die()" err+'+ # The about page redirects to its trailing-slash form so relative links # resolve, and the branch asked for has to survive that hop, as does the # hop back to the summary of a repository without a readme.
diff --git a/tests/t0303-robustness.sh b/tests/t0303-robustness.sh
index 55a83c3..fd1c02e 100755
--- a/tests/t0303-robustness.sh
+++ b/tests/t0303-robustness.sh
@@ -529,6 +529,22 @@ test_expect_success 'the dumb transport withholds the alternates file' '
 	grep "^Status: 200" tmp
 '
 
+# A client that goes away mid-page leaves nobody to send an error page to,
+# so the request ends quietly instead of dying twice into the log. The page
+# has to outgrow the pipe for the write after the reader has gone to fail.
+test_expect_success 'a client that disconnects ends the request quietly' '
+	(
+		cd repos/rob &&
+		awk "BEGIN{for(i=0;i<20000;i++) print \"line \" i}" >long.txt &&
+		git add long.txt &&
+		git commit -m long
+	) &&
+	{ robq "url=rob/tree/long.txt" 2>err; echo $? >status; } | head -c 1 >/dev/null &&
+	test "$(cat status)" = 0 &&
+	! grep -i "broken pipe" err &&
+	! grep "die()" err
+'
+
 # The about page redirects to its trailing-slash form so relative links
 # resolve, and the branch asked for has to survive that hop, as does the
 # hop back to the summary of a repository without a readme.