Harden the request path, scan and error recovery

1 file changed, 49 insertions, 22 deletions

diff --git a/source/cache.c b/source/cache.c
index 5c1c808..0c86210 100644
--- a/source/cache.c
+++ b/source/cache.c
@@ -39,6 +39,7 @@ __attribute__((format (printf,1,2)))
 static void log_error(const char *format, ...)
 {
 	va_list args;
+
 	va_start(args, format);
 	vfprintf(stderr, format, args);
 	va_end(args);
@@ -108,6 +109,7 @@ static int key_fits_slot(const char *key)
 static int close_slot(struct cache_slot *slot)
 {
 	int err = 0;
+
 	if (slot->cache_fd > 0) {
 		if (close(slot->cache_fd))
 			err = errno;
@@ -169,7 +171,7 @@ static int serve_slot(struct cache_slot *slot)
 
 	err = print_slot(slot);
 	if (err)
-		log_error("[cgit] Error printing slot %s: %s (%d)\n", slot->path, strerror(err), err);
+		log_error("[cgit] Unable to send slot %s: %s (%d)\n", slot->path, strerror(err), err);
 	return err;
 }
 
@@ -200,6 +202,7 @@ static int is_modified(struct cache_slot *slot)
 static int close_lock(struct cache_slot *slot)
 {
 	int err = 0;
+
 	if (slot->lock_fd > 0) {
 		if (close(slot->lock_fd))
 			err = errno;
@@ -278,23 +281,40 @@ static int unlock_slot(struct cache_slot *slot, int replace_old_slot)
 	return 0;
 }
 
-// Only one slot is ever being filled at a time, so a single pointer is enough
-// for cache_abandon_fill to find its way back to the client.
+// Only one slot is ever being filled at a time, so cache_abandon_fill finds
+// it here.
 static struct cache_slot *slot_being_filled;
 
-void cache_abandon_fill(void)
+/*
+ * Copies the page the lock file holds so far to stdout, which by then is the
+ * client again.
+ */
+static void replay_lock_file(struct cache_slot *slot)
+{
+	off_t off = slot->keylen + 1;
+	ssize_t got;
+
+	if (lseek(slot->lock_fd, off, SEEK_SET) != off)
+		return;
+	while ((got = xread(slot->lock_fd, slot->buf, sizeof(slot->buf))) > 0) {
+		if (write_in_full(STDOUT_FILENO, slot->buf, got) < 0)
+			return;
+	}
+}
+
+int cache_abandon_fill(void)
 {
 	struct cache_slot *slot = slot_being_filled;
 
 	if (!slot)
-		return;
+		return 0;
 	slot_being_filled = NULL;
 	slot->abandoned = 1;
 
-	// The page is sitting in html.c's buffer and in stdio's. Empty both
-	// while stdout still points at the lock file so the half rendered
-	// page never reaches the client.
-	html_flush();
+	// What git wrote through stdio joins the lock file first, then the
+	// file is played back so the visitor gets the page up to the point it
+	// stopped. The buffer in html.c is the caller's to flush, since a
+	// caller that could not write it must not try again here.
 	fflush(stdout);
 
 	if (slot->saved_stdout >= 0) {
@@ -302,11 +322,15 @@ void cache_abandon_fill(void)
 		close(slot->saved_stdout);
 		slot->saved_stdout = -1;
 	}
+	replay_lock_file(slot);
 	unlink(slot->lock_path);
+	return 1;
 }
 
-// The page is served from the descriptor either way, so a failed rename only
-// costs the next request a render.
+/*
+ * The page is served from the descriptor either way, so a failed rename only
+ * costs the next request a render.
+ */
 static void publish_slot(struct cache_slot *slot)
 {
 	int err = unlock_slot(slot, 1);
@@ -403,20 +427,21 @@ static int process_slot(struct cache_slot *slot)
 	// and two popular pages sharing one slot evict each other on every
 	// alternating visit.
 	if (!err)
-		log_error("[cgit] Cache slot %s holds a different key, consider a larger cache-size\n", slot->path);
+		log_error("[cgit] Cache slot %s holds a different key, consider a larger cache-size\n",
+			slot->path);
 
 	// If any part of creating a slot fails the page is still rendered
 	// straight to the client and the caller is told the request succeeded,
 	// because it did.
 	close_slot(slot);
 	if ((err = lock_slot(slot)) != 0) {
-		log_error("[cgit] Error locking slot %s: %s (%d)\n", slot->lock_path, strerror(err), err);
+		log_error("[cgit] Unable to lock slot %s: %s (%d)\n", slot->lock_path, strerror(err), err);
 		slot->fn();
 		return 0;
 	}
 
 	if ((err = fill_slot(slot)) != 0) {
-		log_error("[cgit] Error filling slot %s: %s (%d)\n", slot->lock_path, strerror(err), err);
+		log_error("[cgit] Unable to fill slot %s: %s (%d)\n", slot->lock_path, strerror(err), err);
 		unlock_slot(slot, 0);
 		close_lock(slot);
 		// Rendering again is only right when nothing was delivered,
@@ -441,14 +466,16 @@ static int process_slot(struct cache_slot *slot)
 	return err;
 }
 
-// The result lives in a static buffer and is only good until the next call.
-static char *format_time(const char *format, time_t when)
+/*
+ * The result lives in a static buffer and is only good until the next call.
+ */
+static const char *format_time(const char *format, time_t when)
 {
 	static char buf[64];
 	struct tm tm;
 
 	if (!when)
-		return NULL;
+		return "-";
 	gmtime_r(&when, &tm);
 	strftime(buf, sizeof(buf) - 1, format, &tm);
 	return buf;
@@ -529,8 +556,8 @@ int cache_ls(const char *path)
 	struct dirent *ent;
 	int err = 0;
 	// A NULL key leaves open_slot with nothing to compare against, so
-	// every slot it opens is simply read.
-	struct cache_slot slot = { NULL };
+	// every slot it opens is read.
+	struct cache_slot slot = { 0 };
 	struct strbuf slot_path = STRBUF_INIT;
 	size_t prefixlen;
 	char *nul;
@@ -543,20 +570,20 @@ int cache_ls(const char *path)
 	dir = opendir(path);
 	if (!dir) {
 		err = errno;
-		log_error("[cgit] Error opening %s: %s (%d)\n", path, strerror(err), err);
+		log_error("[cgit] Unable to open %s: %s (%d)\n", path, strerror(err), err);
 		return err;
 	}
 	strbuf_addstr(&slot_path, path);
 	strbuf_ensure_end(&slot_path, '/');
 	prefixlen = slot_path.len;
-	while ((ent = readdir(dir)) != NULL) {
+	while ((ent = readdir(dir))) {
 		if (strlen(ent->d_name) != SLOT_NAME_LEN)
 			continue;
 		strbuf_setlen(&slot_path, prefixlen);
 		strbuf_addstr(&slot_path, ent->d_name);
 		slot.path = slot_path.buf;
 		if ((err = open_slot(&slot)) != 0) {
-			log_error("[cgit] Error opening %s: %s (%d)\n", slot_path.buf, strerror(err), err);
+			log_error("[cgit] Unable to open %s: %s (%d)\n", slot_path.buf, strerror(err), err);
 			close_slot(&slot);
 			continue;
 		}