1 file changed, 125 insertions, 52 deletions
diff --git a/source/cgit.c b/source/cgit.c
index 5b26137..00ce97f 100644
--- a/source/cgit.c
+++ b/source/cgit.c
@@ -46,10 +46,8 @@
 // a snapshots mask can carry.
 #define ALL_SNAPSHOT_FORMATS 0xFF
 
-/*
- * The first branch found is the fallback, so a repository whose default branch
- * does not exist still has something to show.
- */
+// The first branch found is the fallback, so a repository whose default branch
+// does not exist still has something to show.
 struct refmatch {
 	char *wanted;
 	char *first;
@@ -76,13 +74,16 @@ static void isolate_git_environment(void)
 /*
  * Turn a die from anywhere inside git into a rendered page, since a CGI that
  * produced no output leaves the visitor with whatever the web server makes of
- * it.
+ * it. Git's message names files and objects on the server, so it goes to the
+ * log and the visitor gets a fixed page.
  */
 static NORETURN void die_routine(const char *msg, va_list params)
 {
-	// The error page abandons any cache fill in progress, so the message
-	// reaches the visitor rather than the cache file stdout points at.
-	cgit_vprint_error_page(400, "Bad Request", msg, params);
+	fputs("[cgit] ", stderr);
+	vfprintf(stderr, msg, params);
+	fputc('\n', stderr);
+	cgit_abort_filters();
+	cgit_print_error_page(500, "Internal Server Error", "Unable to complete the request");
 	exit(0);
 }
 
@@ -154,7 +155,8 @@ static void prepare_context(void)
 	ctx.env.server_port = getenv("SERVER_PORT");
 	ctx.env.http_cookie = getenv("HTTP_COOKIE");
 	ctx.env.http_referer = getenv("HTTP_REFERER");
-	ctx.env.content_length = getenv("CONTENT_LENGTH") ? strtoul(getenv("CONTENT_LENGTH"), NULL, 10) : 0;
+	ctx.env.content_length =
+		getenv("CONTENT_LENGTH") ? strtoul(getenv("CONTENT_LENGTH"), NULL, 10) : 0;
 	ctx.env.authenticated = 0;
 	ctx.page.mimetype = "text/html";
 	ctx.page.charset = PAGE_ENCODING;
@@ -189,6 +191,7 @@ static void print_version(void)
 static int cmp_repos(const void *a, const void *b)
 {
 	const struct cgit_repo *repo_a = a, *repo_b = b;
+
 	return strcmp(repo_a->url, repo_b->url);
 }
 
@@ -213,7 +216,8 @@ static void print_repo(FILE *f, struct cgit_repo *repo)
 
 	fprintf(f, "repo.url=%s\n", repo->url);
 	fprintf(f, "repo.name=%s\n", repo->name);
-	fprintf(f, "repo.path=%s\n", repo->path);
+	if (repo->path)
+		fprintf(f, "repo.path=%s\n", repo->path);
 	if (repo->owner)
 		fprintf(f, "repo.owner=%s\n", repo->owner);
 	if (repo->desc)
@@ -318,7 +322,10 @@ static void parse_args(int argc, const char **argv)
 			ctx.qry.has_oid = 1;
 		} else if (skip_prefix(argv[i], "--ofs=", &arg)) {
 			ctx.qry.ofs = atoi(arg);
-		} else if (skip_prefix(argv[i], "--scan-tree=", &arg) || skip_prefix(argv[i], "--scan-path=", &arg)) {
+		} else if (
+			skip_prefix(argv[i], "--scan-tree=", &arg) ||
+			skip_prefix(argv[i], "--scan-path=", &arg)
+		) {
 			// A repository's snapshots setting is masked with the
 			// global one, and cgitrc has not been read here, so an
 			// empty mask would discard what the repository set.
@@ -358,7 +365,7 @@ static int generate_cached_repolist(const char *path, const char *cached_rc)
 	fd = open(locked_rc.buf, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);
 	if (fd == -1) {
 		err = errno;
-		fprintf(stderr, "[cgit] Error opening %s: %s (%d)\n", locked_rc.buf, strerror(err), err);
+		fprintf(stderr, "[cgit] Unable to open %s: %s (%d)\n", locked_rc.buf, strerror(err), err);
 		goto out;
 	}
 	if (fcntl(fd, F_SETLK, &lock) < 0) {
@@ -368,7 +375,7 @@ static int generate_cached_repolist(const char *path, const char *cached_rc)
 		// on every request and does deserve one.
 		err = errno;
 		if (err != EACCES && err != EAGAIN)
-			fprintf(stderr, "[cgit] Error locking %s: %s (%d)\n", locked_rc.buf, strerror(err), err);
+			fprintf(stderr, "[cgit] Unable to lock %s: %s (%d)\n", locked_rc.buf, strerror(err), err);
 		close(fd);
 		goto out;
 	}
@@ -390,7 +397,7 @@ static int generate_cached_repolist(const char *path, const char *cached_rc)
 	// start from empty now that nobody else can be writing it.
 	if (ftruncate(fd, 0) < 0 || !(f = fdopen(fd, "w"))) {
 		err = errno;
-		fprintf(stderr, "[cgit] Error writing %s: %s (%d)\n", locked_rc.buf, strerror(err), err);
+		fprintf(stderr, "[cgit] Unable to write %s: %s (%d)\n", locked_rc.buf, strerror(err), err);
 		unlink(locked_rc.buf);
 		close(fd);
 		goto out;
@@ -406,14 +413,14 @@ static int generate_cached_repolist(const char *path, const char *cached_rc)
 	// that stops wherever the buffer happened to end.
 	if (fflush(f) || ferror(f)) {
 		err = errno;
-		fprintf(stderr, "[cgit] Error writing %s: %s (%d)\n", locked_rc.buf, strerror(err), err);
+		fprintf(stderr, "[cgit] Unable to write %s: %s (%d)\n", locked_rc.buf, strerror(err), err);
 		unlink(locked_rc.buf);
 		fclose(f);
 		goto out;
 	}
 	if (rename(locked_rc.buf, cached_rc)) {
 		err = errno;
-		fprintf(stderr, "[cgit] Error renaming %s to %s: %s (%d)\n",
+		fprintf(stderr, "[cgit] Unable to rename %s to %s: %s (%d)\n",
 			locked_rc.buf, cached_rc, strerror(err), err);
 		unlink(locked_rc.buf);
 	}
@@ -425,7 +432,9 @@ out:
 	return err;
 }
 
-// A cached repolist is itself a config file, so these two call each other.
+/*
+ * A cached repolist is itself a config file, so these two call each other.
+ */
 static void apply_config(const char *name, const char *value);
 
 static void process_cached_repolist(const char *path)
@@ -539,8 +548,8 @@ static void apply_config(const char *name, const char *value)
 		ctx.cfg.enable_header = atoi(value);
 	else if (!strcmp(name, "snapshots"))
 		ctx.cfg.snapshots = cgit_parse_snapshots_mask(value);
-	else if (!strcmp(name, "trust-scan-filters"))
-		ctx.cfg.trust_scan_filters = atoi(value);
+	else if (!strcmp(name, "trust-scan-config"))
+		ctx.cfg.trust_scan_config = atoi(value);
 	else if (!strcmp(name, "enable-follow-links"))
 		ctx.cfg.enable_follow_links = atoi(value);
 	else if (!strcmp(name, "enable-http-clone"))
@@ -644,6 +653,9 @@ static void apply_config(const char *name, const char *value)
 			scan_projects(cgit_expand_macros(value), ctx.cfg.project_list);
 		else
 			scan_tree(cgit_expand_macros(value));
+		// The scan grows the repository array, so a record taken before
+		// it may have moved, and a repo key after it belongs to nothing.
+		ctx.repo = NULL;
 	} else if (!strcmp(name, "scan-hidden-path"))
 		ctx.cfg.scan_hidden_path = atoi(value);
 	else if (!strcmp(name, "section-from-path"))
@@ -721,7 +733,7 @@ static void apply_query_param(const char *name, const char *value)
 	if (!value)
 		value = "";
 
-	if (!strcmp(name,"r")) {
+	if (!strcmp(name, "r")) {
 		ctx.qry.repo = xstrdup(value);
 		ctx.repo = cgit_get_repoinfo(value);
 	} else if (!strcmp(name, "p")) {
@@ -814,10 +826,12 @@ static void authenticate_post(void)
 	len = ctx.env.content_length;
 	if (len > MAX_AUTHENTICATION_POST_BYTES)
 		len = MAX_AUTHENTICATION_POST_BYTES;
-	if ((got = read(STDIN_FILENO, buffer, len)) < 0)
-		die_errno("Could not read POST from stdin");
-	if (write(STDOUT_FILENO, buffer, got) < 0)
-		die_errno("Could not write POST to stdout");
+	// The body can arrive in more than one write, so read until it is
+	// all there or the server closes the stream.
+	if ((got = read_in_full(STDIN_FILENO, buffer, len)) < 0)
+		die_errno("Unable to read the POST body");
+	if (write_in_full(STDOUT_FILENO, buffer, got) < 0)
+		die_errno("Unable to pass the POST body to the auth filter");
 	cgit_close_filter(ctx.cfg.auth_filter);
 	exit(0);
 }
@@ -856,13 +870,42 @@ static int is_full_oid(const char *rev)
 	if (len != GIT_SHA1_HEXSZ && len != GIT_SHA256_HEXSZ)
 		return 0;
 	for (; *rev; rev++) {
-		if (!isxdigit(*rev))
+		if (!isxdigit((unsigned char)*rev))
 			return 0;
 	}
 	return 1;
 }
 
-// Every cache-*-ttl setting is written in minutes, and so is this.
+/*
+ * Only these pages render the same bytes for the same id for ever. A log or
+ * refs page named with an id still lists branches and tags, which move.
+ */
+static int page_is_static(void)
+{
+	static const char *const pages[] = {
+		"blame",
+		"blob",
+		"commit",
+		"diff",
+		"patch",
+		"plain",
+		"rawdiff",
+		"snapshot",
+		"tag",
+		"tree",
+	};
+	size_t i;
+
+	for (i = 0; i < ARRAY_SIZE(pages); i++) {
+		if (!strcmp(ctx.qry.page, pages[i]))
+			return 1;
+	}
+	return 0;
+}
+
+/*
+ * Every cache-*-ttl setting is written in minutes, and so is this.
+ */
 static int calc_ttl(void)
 {
 	if (!ctx.repo)
@@ -879,6 +922,7 @@ static int calc_ttl(void)
 	// page to rebuild.
 	if (
 		ctx.qry.has_oid								&&
+		page_is_static()							&&
 		(!ctx.qry.oid || is_full_oid(ctx.qry.oid))	&&
 		(!ctx.qry.oid2 || is_full_oid(ctx.qry.oid2))
 	)
@@ -917,14 +961,18 @@ static void build_cache_key(struct strbuf *key)
 	free(hosturl);
 }
 
-// Returning non-zero ends git's walk, so the search stops at the first hit.
+/*
+ * Returning non-zero ends git's walk, so the search stops at the first hit.
+ */
 static int find_current_ref(const struct reference *ref, void *data)
 {
 	struct refmatch *match = data;
 
 	if (!strcmp(ref->name, match->wanted))
 		match->found = 1;
-	if (!match->first)
+	// The fallback has to pass the check every request makes on its head,
+	// or one branch named with a leading dash takes the repository offline.
+	if (!match->first && ref->name[0] != '-')
 		match->first = xstrdup(ref->name);
 	return match->found;
 }
@@ -996,14 +1044,18 @@ static void parse_readme(const char *readme, char **filename, char **ref, struct
 static void choose_readme(struct cgit_repo *repo)
 {
 	int found;
-	char *filename, *ref;
+	char *filename = NULL, *ref = NULL;
+	struct string_list *list;
 	struct string_list_item *entry;
 
-	if (!repo->readme.nr)
+	// A repository without readme settings of its own follows the global
+	// ones, which it must not free.
+	list = repo->readme.nr ? &repo->readme : &ctx.cfg.readme;
+	if (!list->nr)
 		return;
 
 	found = 0;
-	for_each_string_list_item(entry, &repo->readme) {
+	for_each_string_list_item(entry, list) {
 		parse_readme(entry->string, &filename, &ref, repo);
 		if (!filename) {
 			free(ref);
@@ -1028,11 +1080,19 @@ static void choose_readme(struct cgit_repo *repo)
 		string_list_append(&repo->readme, filename)->util = ref;
 }
 
-static void prepare_repo_env(int *nongit)
+static void prepare_repo_env(int *nongit, int *err)
 {
+	// A repository configured without a path has nothing to open.
+	if (!ctx.repo->path) {
+		*nongit = 1;
+		*err = 0;
+		return;
+	}
 	setenv("GIT_DIR", ctx.repo->path, 1);
 
+	errno = 0;
 	setup_git_directory_gently(the_repository, nongit);
+	*err = errno;
 	// Notes come out of the object store, which a repository that failed
 	// to open has none of.
 	if (!*nongit)
@@ -1043,14 +1103,12 @@ static void prepare_repo_env(int *nongit)
  * Returns non-zero once it has written a complete response of its own, in
  * which case the caller must not render a page over the top of it.
  */
-static int prepare_repo_cmd(int nongit)
+static int prepare_repo_cmd(int nongit, int err, int clone)
 {
 	struct object_id oid;
-	int err;
 
 	if (nongit) {
 		const char *name = ctx.repo->name;
-		err = errno;
 		ctx.page.title = cgit_fmtalloc("%s - %s", ctx.cfg.root_title, "config error");
 		ctx.repo = NULL;
 		cgit_print_error_page(
@@ -1070,6 +1128,12 @@ static int prepare_repo_cmd(int nongit)
 	}
 
 	if (!ctx.qry.head) {
+		// The dumb transport serves refs and objects off the disk and
+		// asks nothing of the head, and an empty repository clones.
+		if (clone) {
+			cgit_prepare_repo_env(ctx.repo);
+			return 0;
+		}
 		ctx.empty_repo = 1;
 		// Before the document starts, since the <head> carries
 		// <link rel='vcs-git'> and those clone urls expand macros such
@@ -1083,13 +1147,22 @@ static int prepare_repo_cmd(int nongit)
 		return 1;
 	}
 
-	if (repo_get_oid(the_repository, ctx.qry.head, &oid)) {
+	// Every revision the request names is checked before git sees it,
+	// see cgit_valid_rev, and the head has to resolve as well.
+	if (!cgit_valid_rev(ctx.qry.head) || repo_get_oid(the_repository, ctx.qry.head, &oid)) {
 		char *old_head = ctx.qry.head;
 		ctx.qry.head = xstrdup(ctx.repo->defbranch);
 		cgit_print_error_page(404, "Not Found", "Invalid branch: %s", old_head);
 		free(old_head);
 		return 1;
 	}
+	if (
+		(ctx.qry.oid && !cgit_valid_rev(ctx.qry.oid))	||
+		(ctx.qry.oid2 && !cgit_valid_rev(ctx.qry.oid2))
+	) {
+		cgit_print_error_page(400, "Bad Request", "Invalid revision");
+		return 1;
+	}
 	string_list_sort(&ctx.repo->submodules);
 	cgit_prepare_repo_env(ctx.repo);
 	choose_readme(ctx.repo);
@@ -1099,7 +1172,7 @@ static int prepare_repo_cmd(int nongit)
 static void process_request(void)
 {
 	const struct cgit_cmd *cmd;
-	int nongit = 0;
+	int nongit = 0, err = 0;
 
 	// An unauthenticated request is answered with the filter's own body
 	// whatever page it asked for.
@@ -1115,16 +1188,14 @@ static void process_request(void)
 	}
 
 	if (ctx.repo)
-		prepare_repo_env(&nongit);
+		prepare_repo_env(&nongit, &err);
 
 	cmd = cgit_get_cmd();
-	if (!cmd) {
-		ctx.page.title = "cgit error";
-		cgit_print_error_page(404, "Not Found", "Invalid request");
-		return;
-	}
-
-	if (!ctx.cfg.enable_http_clone && cmd->is_clone) {
+	if (!cmd || (!ctx.cfg.enable_http_clone && cmd->is_clone)) {
+		// The error page carries the repository's own header, whose
+		// branch switcher needs the head resolved first.
+		if (ctx.repo && prepare_repo_cmd(nongit, err, 0))
+			return;
 		ctx.page.title = "cgit error";
 		cgit_print_error_page(404, "Not Found", "Invalid request");
 		return;
@@ -1137,7 +1208,7 @@ static void process_request(void)
 
 	ctx.qry.vpath = cmd->want_vpath ? ctx.qry.path : NULL;
 
-	if (ctx.repo && prepare_repo_cmd(nongit))
+	if (ctx.repo && prepare_repo_cmd(nongit, err, cmd->is_clone))
 		return;
 
 	cmd->fn();
@@ -1205,13 +1276,11 @@ void cgit_repo_config(struct cgit_repo *repo, const char *name, const char *valu
 		repo->section = cgit_strdup_first_line(value);
 	else if (!strcmp(name, "snapshot-prefix"))
 		repo->snapshot_prefix = cgit_strdup_first_line(value);
-	else if (!strcmp(name, "readme") && value != NULL) {
-		if (repo->readme.items == ctx.cfg.readme.items)
-			memset(&repo->readme, 0, sizeof(repo->readme));
+	else if (!strcmp(name, "readme"))
 		string_list_append(&repo->readme, cgit_strdup_first_line(value));
-	} else if (!strcmp(name, "logo") && value != NULL)
+	else if (!strcmp(name, "logo"))
 		repo->logo = cgit_strdup_first_line(value);
-	else if (!strcmp(name, "logo-link") && value != NULL)
+	else if (!strcmp(name, "logo-link"))
 		repo->logo_link = cgit_strdup_first_line(value);
 	else if (!strcmp(name, "hide"))
 		repo->hide = atoi(value);
@@ -1238,6 +1307,9 @@ int cmd_main(int argc, const char **argv)
 	int err, ttl;
 
 	isolate_git_environment();
+	// Ignored, a filter that exits early makes the write fail with EPIPE
+	// and the error page reach the visitor, instead of ending cgit.
+	signal(SIGPIPE, SIG_IGN);
 	cgit_init_filters();
 	atexit(cgit_cleanup_filters);
 	// Registered second so it runs first, since exit is reached from error
@@ -1271,8 +1343,9 @@ int cmd_main(int argc, const char **argv)
 			char *path_and_query = cgit_fmtalloc("%s?%s", path, ctx.qry.raw);
 			free(ctx.qry.raw);
 			ctx.qry.raw = path_and_query;
-		} else
+		} else {
 			ctx.qry.raw = xstrdup(ctx.qry.url);
+		}
 		cgit_parse_url(ctx.qry.url);
 	}