commit 8d02314fcaebb323c9ca3b102f8a085d66ea4633
from: Omar Polo <op@omarpolo.com>
date: Wed Sep 07 09:25:51 2022 UTC

gotwebd: percent-encode generated URLs

This adds a new helper function `gotweb_link' that outputs an HTML link
with the parameter correctly escaped.

ok stsp@

commit - 6e64c55881ec2855a3d5583227c02e4958db0ba9
commit + 8d02314fcaebb323c9ca3b102f8a085d66ea4633
blob - e77b8424231bf88c153a32dd90a19292c41dc39d
blob + 04e9e72ece31adeacc0aef6f6fb7cffbaace078a
--- gotwebd/fcgi.c
+++ gotwebd/fcgi.c
@@ -288,16 +288,12 @@ fcgi_timeout(int fd, short events, void *arg)
 }
 
 int
-fcgi_printf(struct request *c, const char *fmt, ...)
+fcgi_vprintf(struct request *c, const char *fmt, va_list ap)
 {
-	va_list ap;
 	char *str;
 	int r;
 
-	va_start(ap, fmt);
 	r = vasprintf(&str, fmt, ap);
-	va_end(ap);
-
 	if (r == -1) {
 		log_warn("%s: asprintf", __func__);
 		return -1;
@@ -309,6 +305,19 @@ fcgi_printf(struct request *c, const char *fmt, ...)
 }
 
 int
+fcgi_printf(struct request *c, const char *fmt, ...)
+{
+	va_list ap;
+	int r;
+
+	va_start(ap, fmt);
+	r = fcgi_vprintf(c, fmt, ap);
+	va_end(ap);
+
+	return r;
+}
+
+int
 fcgi_gen_binary_response(struct request *c, const uint8_t *data, int len)
 {
 	int r;
blob - 04dc980392274b236587daf89d34a45c992b64c1
blob + 5211e085ce0097066a9ec6cb94c843ffc952f29f
--- gotwebd/got_operations.c
+++ gotwebd/got_operations.c
@@ -810,7 +810,7 @@ got_output_repo_tree(struct request *c)
 	struct got_reflist_head refs;
 	struct got_tree_object *tree = NULL;
 	struct repo_dir *repo_dir = t->repo_dir;
-	const char *name, *index_page_str, *folder;
+	const char *name, *folder;
 	char *escaped_name = NULL, *path = NULL;
 	int nentries, i, r;
 
@@ -849,7 +849,6 @@ got_output_repo_tree(struct request *c)
 
 	nentries = got_object_tree_get_nentries(tree);
 
-	index_page_str = qs->index_page_str ? qs->index_page_str : "";
 	folder = qs->folder ? qs->folder : "";
 
 	for (i = 0; i < nentries; i++) {
@@ -877,42 +876,76 @@ got_output_repo_tree(struct request *c)
 			goto done;
 
 		if (S_ISDIR(mode)) {
-			r = fcgi_printf(c,
-			    "<div class='tree_wrapper'>\n"
-			    "<div class='tree_line'>"
-			    "<a class='diff_directory' "
-			    "href='?index_page=%s&path=%s&action=tree"
-			    "&commit=%s&folder=%s/%s'>%s%s</a>"
-			    "</div>\n" /* .tree_line */
-			    "<div class='tree_line_blank'>&nbsp;</div>\n"
-			    "</div>\n", /* .tree_wrapper */
-			    index_page_str, qs->path, rc->commit_id,
-			    folder, name, escaped_name, modestr);
+			struct gotweb_url url = {
+				.index_page = -1,
+				.page = -1,
+				.action = TREE,
+				.commit = rc->commit_id,
+				.path = qs->path,
+				/* `folder' is filled later */
+			};
+			char *path = NULL;
+
+			if (fcgi_printf(c,"<div class='tree_wrapper'>\n"
+			    "<div class='tree_line'>") == -1)
+				goto done;
+
+			if (asprintf(&path, "%s/%s", folder, name) == -1) {
+				error = got_error_from_errno("asprintf");
+				goto done;
+			}
+			url.folder = path;
+			r = gotweb_link(c, &url, "%s%s", escaped_name,
+			    modestr);
+			free(path);
 			if (r == -1)
 				goto done;
+
+			if (fcgi_printf(c, "</div>\n" /* .tree_line */
+			    "<div class='tree_line_blank'>&nbsp;</div>\n"
+			    "</div>\n") == -1)
+				goto done;
 		} else {
-			r = fcgi_printf(c,
-			    "<div class='tree_wrapper'>\n"
-			    "<div class='tree_line'>"
-			    "<a href='?index_page=%s&path=%s&action=blob"
-			    "&commit=%s&folder=%s&file=%s'>%s%s</a>"
-			    "</div>\n" /* .tree_line */
-			    "<div class='tree_line_blank'>"
-			    "<a href='?index_page=%s&path=%s&action=commits"
-			    "&commit=%s&folder=%s&file=%s'>commits</a>\n"
-			    " | \n"
-			    "<a href='?index_page=%s&path=%s&action=blame"
-			    "&commit=%s&folder=%s&file=%s'>blame</a>\n"
-			    "</div>\n"  /* .tree_line_blank */
-			    "</div>\n", /* .tree_wrapper */
-			    index_page_str, qs->path, rc->commit_id,
-			    folder, name, escaped_name, modestr,
-			    index_page_str, qs->path, rc->commit_id,
-			    folder, name,
-			    index_page_str, qs->path, rc->commit_id,
-			    folder, name);
+			struct gotweb_url url = {
+				.index_page = -1,
+				.page = -1,
+				.path = qs->path,
+				.commit = rc->commit_id,
+				.folder = folder,
+				.file = name,
+			};
+
+			if (fcgi_printf(c, "<div class='tree_wrapper'>\n"
+			    "<div class='tree_line'>") == -1)
+				goto done;
+
+			url.action = BLOB;
+			r = gotweb_link(c, &url, "%s%s", escaped_name,
+			    modestr);
 			if (r == -1)
 				goto done;
+
+			if (fcgi_printf(c, "</div>\n" /* .tree_line */
+			    "<div class='tree_line_blank'>") == -1)
+				goto done;
+
+			url.action = COMMITS;
+			r = gotweb_link(c, &url, "commits");
+			if (r == -1)
+				goto done;
+
+			if (fcgi_printf(c, " | ") == -1)
+				goto done;
+
+			url.action = BLAME;
+			r = gotweb_link(c, &url, "blame");
+			if (r == -1)
+				goto done;
+
+			if (fcgi_printf(c,
+			    "</div>\n"		/* .tree_line_blank */
+			    "</div>\n") == -1)	/* .tree_wrapper */
+				goto done;
 		}
 		free(escaped_name);
 		escaped_name = NULL;
@@ -1069,9 +1102,7 @@ got_gotweb_blame_cb(void *arg, int nlines, int lineno,
 	struct blame_line *bline;
 	struct request *c = a->c;
 	struct transport *t = c->t;
-	struct querystring *qs = t->qs;
 	struct repo_dir *repo_dir = t->repo_dir;
-	const char *index_page_str;
 	char *line = NULL, *eline = NULL;
 	size_t linesize = 0;
 	off_t offset;
@@ -1121,8 +1152,6 @@ got_gotweb_blame_cb(void *arg, int nlines, int lineno,
 		goto done;
 	}
 
-	index_page_str = qs->index_page_str ? qs->index_page_str : "";
-
 	while (a->lineno_cur <= a->nlines && bline->annotated) {
 		char *smallerthan, *at, *nl, *committer;
 		size_t len;
@@ -1152,19 +1181,28 @@ got_gotweb_blame_cb(void *arg, int nlines, int lineno,
 		if (err)
 			goto done;
 
-		r = fcgi_printf(c, "<div class='blame_wrapper'>"
+		if (fcgi_printf(c, "<div class='blame_wrapper'>"
 		    "<div class='blame_number'>%.*d</div>"
-		    "<div class='blame_hash'>"
-		    "<a href='?index_page=%s&path=%s&action=diff&commit=%s'>"
-		    "%.8s</a>"
+		    "<div class='blame_hash'>",
+		    a->nlines_prec, a->lineno_cur) == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.action = DIFF,
+			.index_page = -1,
+			.page = -1,
+			.path = repo_dir->name,
+			.commit = bline->id_str,
+		    }, "%.8s", bline->id_str);
+		if (r == -1)
+			goto done;
+
+		r = fcgi_printf(c,
 		    "</div>"
 		    "<div class='blame_date'>%s</div>"
 		    "<div class='blame_author'>%s</div>"
 		    "<div class='blame_code'>%s</div>"
 		    "</div>",	/* .blame_wrapper */
-		    a->nlines_prec, a->lineno_cur,
-		    index_page_str, repo_dir->name, bline->id_str,
-		    bline->id_str,
 		    bline->datebuf,
 		    committer,
 		    eline);
blob - 045fe4910629bd84e416a1c27e13e918b0403be7
blob + aafa0224a0f81173ac396f4607674dc80127339a
--- gotwebd/gotweb.c
+++ gotwebd/gotweb.c
@@ -694,6 +694,7 @@ gotweb_render_content_type_file(struct request *c, con
 static const struct got_error *
 gotweb_render_header(struct request *c)
 {
+	const struct got_error *err = NULL;
 	struct server *srv = c->srv;
 	struct querystring *qs = c->t->qs;
 	int r;
@@ -743,10 +744,21 @@ gotweb_render_header(struct request *c)
 
 	if (qs != NULL) {
 		if (qs->path != NULL) {
-			r = fcgi_printf(c, " / "
-			    "<a href='?index_page=%d&path=%s&action=summary'>"
-			    "%s</a>",
-			    qs->index_page, qs->path, qs->path);
+			char *epath;
+
+			if (fcgi_printf(c, " / ") == -1)
+				goto done;
+
+			err = gotweb_escape_html(&epath, qs->path);
+			if (err)
+				return err;
+			r = gotweb_link(c, &(struct gotweb_url){
+				.action = SUMMARY,
+				.index_page = -1,
+				.page = -1,
+				.path = qs->path,
+			    }, "%s", epath);
+			free(epath);
 			if (r == -1)
 				goto done;
 		}
@@ -826,8 +838,7 @@ gotweb_render_navs(struct request *c)
 	struct transport *t = c->t;
 	struct querystring *qs = t->qs;
 	struct server *srv = c->srv;
-	char *nhref = NULL, *phref = NULL;
-	int r, disp = 0;
+	int r;
 
 	r = fcgi_printf(c, "<div id='np_wrapper'>\n<div id='nav_prev'>\n");
 	if (r == -1)
@@ -836,140 +847,134 @@ gotweb_render_navs(struct request *c)
 	switch(qs->action) {
 	case INDEX:
 		if (qs->index_page > 0) {
-			if (asprintf(&phref, "index_page=%d",
-			    qs->index_page - 1) == -1) {
-				error = got_error_from_errno2("%s: asprintf",
-				    __func__);
-				goto done;
-			}
-			disp = 1;
+			struct gotweb_url url = {
+				.action = -1,
+				.index_page = qs->index_page - 1,
+				.page = -1,
+			};
+
+			r = gotweb_link(c, &url, "Previous");
 		}
 		break;
 	case BRIEFS:
 		if (t->prev_id && qs->commit != NULL &&
 		    strcmp(qs->commit, t->prev_id) != 0) {
-			if (asprintf(&phref, "index_page=%d&path=%s&page=%d"
-			    "&action=briefs&commit=%s&headref=%s",
-			    qs->index_page, qs->path, qs->page - 1, t->prev_id,
-			    qs->headref) == -1) {
-				error = got_error_from_errno2("%s: asprintf",
-				    __func__);
-				goto done;
-			}
-			disp = 1;
+			struct gotweb_url url = {
+				.action = BRIEFS,
+				.index_page = -1,
+				.page = qs->page - 1,
+				.path = qs->path,
+				.commit = t->prev_id,
+				.headref = qs->headref,
+			};
+
+			r = gotweb_link(c, &url, "Previous");
 		}
 		break;
 	case COMMITS:
 		if (t->prev_id && qs->commit != NULL &&
 		    strcmp(qs->commit, t->prev_id) != 0) {
-			if (asprintf(&phref, "index_page=%d&path=%s&page=%d"
-			    "&action=commits&commit=%s&headref=%s&folder=%s"
-			    "&file=%s",
-			    qs->index_page, qs->path, qs->page - 1, t->prev_id,
-			    qs->headref, qs->folder ? qs->folder : "",
-			    qs->file ? qs->file : "") == -1) {
-				error = got_error_from_errno2("%s: asprintf",
-				    __func__);
-				goto done;
-			}
-			disp = 1;
+			struct gotweb_url url = {
+				.action = COMMIT,
+				.index_page = -1,
+				.page = qs->page - 1,
+				.path = qs->path,
+				.commit = t->prev_id,
+				.headref = qs->headref,
+				.folder = qs->folder,
+				.file = qs->file,
+			};
+
+			r = gotweb_link(c, &url, "Previous");
 		}
 		break;
 	case TAGS:
 		if (t->prev_id && qs->commit != NULL &&
 		    strcmp(qs->commit, t->prev_id) != 0) {
-			if (asprintf(&phref, "index_page=%d&path=%s&page=%d"
-			    "&action=tags&commit=%s&headref=%s",
-			    qs->index_page, qs->path, qs->page - 1, t->prev_id,
-			    qs->headref) == -1) {
-				error = got_error_from_errno2("%s: asprintf",
-				    __func__);
-				goto done;
-			}
-			disp = 1;
+			struct gotweb_url url = {
+				.action = TAGS,
+				.index_page = -1,
+				.page = qs->page - 1,
+				.path = qs->path,
+				.commit = t->prev_id,
+				.headref = qs->headref,
+			};
+
+			r = gotweb_link(c, &url, "Previous");
 		}
 		break;
-	default:
-		disp = 0;
-		break;
 	}
 
-	if (disp) {
-		r = fcgi_printf(c, "<a href='?%s'>Previous</a>", phref);
-		if (r == -1)
-			goto done;
-	}
+	if (r == -1)
+		goto done;
 
 	r = fcgi_printf(c, "</div>\n"	/* #nav_prev */
 	    "<div id='nav_next'>");
 	if (r == -1)
 		goto done;
 
-	disp = 0;
 	switch(qs->action) {
 	case INDEX:
 		if (t->next_disp == srv->max_repos_display &&
 		    t->repos_total != (qs->index_page + 1) *
 		    srv->max_repos_display) {
-			if (asprintf(&nhref, "index_page=%d",
-			    qs->index_page + 1) == -1) {
-				error = got_error_from_errno2("%s: asprintf",
-				    __func__);
-				goto done;
-			}
-			disp = 1;
+			struct gotweb_url url = {
+				.action = -1,
+				.index_page = qs->index_page + 1,
+				.page = -1,
+			};
+
+			r = gotweb_link(c, &url, "Next");
 		}
 		break;
 	case BRIEFS:
 		if (t->next_id) {
-			if (asprintf(&nhref, "index_page=%d&path=%s&page=%d"
-			    "&action=briefs&commit=%s&headref=%s",
-			    qs->index_page, qs->path, qs->page + 1, t->next_id,
-			    qs->headref) == -1) {
-				error = got_error_from_errno2("%s: asprintf",
-				    __func__);
-				goto done;
-			}
-			disp = 1;
+			struct gotweb_url url = {
+				.action = BRIEFS,
+				.index_page = -1,
+				.page = qs->page + 1,
+				.path = qs->path,
+				.commit = t->next_id,
+				.headref = qs->headref,
+			};
+
+			r = gotweb_link(c, &url, "Next");
 		}
 		break;
 	case COMMITS:
 		if (t->next_id) {
-			if (asprintf(&nhref, "index_page=%d&path=%s&page=%d"
-			    "&action=commits&commit=%s&headref=%s&folder=%s"
-			    "&file=%s",
-			    qs->index_page, qs->path, qs->page + 1, t->next_id,
-			    qs->headref, qs->folder ? qs->folder : "",
-			    qs->file ? qs->file : "") == -1) {
-				error = got_error_from_errno2("%s: asprintf",
-				    __func__);
-				goto done;
-			}
-			disp = 1;
+			struct gotweb_url url = {
+				.action = COMMIT,
+				.index_page = -1,
+				.page = qs->page + 1,
+				.path = qs->path,
+				.commit = t->next_id,
+				.headref = qs->headref,
+				.folder = qs->folder,
+				.file = qs->file,
+			};
+
+			r = gotweb_link(c, &url, "Next");
 		}
 		break;
 	case TAGS:
 		if (t->next_id) {
-			if (asprintf(&nhref, "index_page=%d&path=%s&page=%d"
-			    "&action=tags&commit=%s&headref=%s",
-			    qs->index_page, qs->path, qs->page + 1, t->next_id,
-			    qs->headref) == -1) {
-				error = got_error_from_errno2("%s: asprintf",
-				    __func__);
-				goto done;
-			}
-			disp = 1;
+			struct gotweb_url url = {
+				.action = TAGS,
+				.index_page = -1,
+				.page = qs->page + 1,
+				.path = qs->path,
+				.commit = t->next_id,
+				.headref = qs->headref,
+			};
+
+			r = gotweb_link(c, &url, "Next");
 		}
 		break;
-	default:
-		disp = 0;
-		break;
 	}
-	if (disp) {
-		r = fcgi_printf(c, "<a href='?%s'>Next</a>", nhref);
-		if (r == -1)
-			goto done;
-	}
+	if (r == -1)
+		goto done;
+
 	fcgi_printf(c, "</div>\n"); /* #nav_next */
 	fcgi_printf(c, "</div>\n"); /* #np_wrapper */
 done:
@@ -977,8 +982,6 @@ done:
 	t->next_id = NULL;
 	free(t->prev_id);
 	t->prev_id = NULL;
-	free(phref);
-	free(nhref);
 	return error;
 }
 
@@ -992,14 +995,11 @@ gotweb_render_index(struct request *c)
 	struct repo_dir *repo_dir = NULL;
 	DIR *d;
 	struct dirent **sd_dent = NULL;
-	const char *index_page_str;
 	char *c_path = NULL;
 	struct stat st;
 	unsigned int d_cnt, d_i, d_disp = 0;
 	int r;
 
-	index_page_str = qs->index_page_str ? qs->index_page_str : "";
-
 	d = opendir(srv->repos_path);
 	if (d == NULL) {
 		error = got_error_from_errno2("opendir", srv->repos_path);
@@ -1091,17 +1091,22 @@ render:
 		d_disp++;
 		t->prev_disp++;
 
-		r = fcgi_printf(c, "<div class='index_wrapper'>\n"
-		    "<div class='index_project'>"
-		    "<a href='?index_page=%s&path=%s&action=summary'>"
-		    " %s "
-		    "</a>"
-		    "</div>",	/* .index_project */
-		    index_page_str, repo_dir->name,
-		    repo_dir->name);
+		if (fcgi_printf(c, "<div class='index_wrapper'>\n"
+		    "<div class='index_project'>") == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.action = SUMMARY,
+			.index_page = -1,
+			.page = -1,
+			.path = repo_dir->name,
+		    }, "%s", repo_dir->name);
 		if (r == -1)
 			goto done;
 
+		if (fcgi_printf(c, "</div>") == -1) /* .index_project */
+			goto done;
+
 		if (srv->show_repo_description) {
 			r = fcgi_printf(c,
 			    "<div class='index_project_description'>\n"
@@ -1124,32 +1129,71 @@ render:
 				goto done;
 		}
 
-		r = fcgi_printf(c, "<div class='navs_wrapper'>"
-		    "<div class='navs'>"
-		    "<a href='?index_page=%s&path=%s&action=summary'>"
-		    "summary"
-		    "</a> | "
-		    "<a href='?index_page=%s&path=%s&action=briefs'>"
-		    "commit briefs"
-		    "</a> | "
-		    "<a href='?index_page=%s&path=%s&action=commits'>"
-		    "commits"
-		    "</a> | "
-		    "<a href='?index_page=%s&path=%s&action=tags'>"
-		    "tags"
-		    "</a> | "
-		    "<a href='?index_page=%s&path=%s&action=tree'>"
-		    "tree"
-		    "</a>"
-		    "</div>"	/* .navs */
+		if (fcgi_printf(c, "<div class='navs_wrapper'>"
+		    "<div class='navs'>") == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.action = SUMMARY,
+			.index_page = -1,
+			.page = -1,
+			.path = repo_dir->name
+		    }, "summary");
+		if (r == -1)
+			goto done;
+
+		if (fcgi_printf(c, " | ") == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.action = BRIEFS,
+			.index_page = -1,
+			.page = -1,
+			.path = repo_dir->name
+		    }, "commit briefs");
+		if (r == -1)
+			goto done;
+
+		if (fcgi_printf(c, " | ") == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.action = COMMITS,
+			.index_page = -1,
+			.page = -1,
+			.path = repo_dir->name
+		    }, "commits");
+		if (r == -1)
+			goto done;
+
+		if (fcgi_printf(c, " | ") == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.action = TAGS,
+			.index_page = -1,
+			.page = -1,
+			.path = repo_dir->name
+		    }, "tags");
+		if (r == -1)
+			goto done;
+
+		if (fcgi_printf(c, " | ") == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.action = TREE,
+			.index_page = -1,
+			.page = -1,
+			.path = repo_dir->name
+		    }, "tree");
+		if (r == -1)
+			goto done;
+
+		r = fcgi_printf(c, "</div>"	/* .navs */
 		    "<div class='dotted_line'></div>\n"
-		    "</div>\n"	/* .navs_wrapper */
-		    "</div>\n",	/* .index_wrapper */
-		    index_page_str, repo_dir->name,
-		    index_page_str, repo_dir->name,
-		    index_page_str, repo_dir->name,
-		    index_page_str, repo_dir->name,
-		    index_page_str, repo_dir->name);
+		    "</div>\n"			/* .navs_wrapper */
+		    "</div>\n");		/* .index_wrapper */
 		if (r == -1)
 			goto done;
 
@@ -1243,13 +1287,10 @@ gotweb_render_briefs(struct request *c)
 	struct transport *t = c->t;
 	struct querystring *qs = t->qs;
 	struct repo_dir *repo_dir = t->repo_dir;
-	const char *index_page_str;
 	char *smallerthan, *newline;
 	char *age = NULL, *author = NULL, *msg = NULL;
 	int r;
 
-	index_page_str = qs->index_page_str ? qs->index_page_str : "";
-
 	r = fcgi_printf(c, "<div id='briefs_title_wrapper'>\n"
 	    "<div id='briefs_title'>Commit Briefs</div>\n"
 	    "</div>\n"	/* #briefs_title_wrapper */
@@ -1287,16 +1328,22 @@ gotweb_render_briefs(struct request *c)
 
 		r = fcgi_printf(c, "<div class='briefs_age'>%s</div>\n"
 		    "<div class='briefs_author'>%s</div>\n"
-		    "<div class='briefs_log'>"
-		    "<a href='?index_page=%s&path=%s&action=diff&commit=%s"
-		    "&headref=%s'>%s</a>",
-		    age,
-		    author,
-		    index_page_str, repo_dir->name, rc->commit_id, qs->headref,
-		    msg);
+		    "<div class='briefs_log'>",
+		    age, author);
 		if (r == -1)
 			goto done;
 
+		r = gotweb_link(c, &(struct gotweb_url){
+			.action = DIFF,
+			.index_page = -1,
+			.page = -1,
+			.path = repo_dir->name,
+			.commit = rc->commit_id,
+			.headref = qs->headref,
+		    }, "%s", msg);
+		if (r == -1)
+			goto done;
+
 		if (rc->refs_str) {
 			char *refs;
 
@@ -1313,20 +1360,40 @@ gotweb_render_briefs(struct request *c)
 			goto done;
 
 		r = fcgi_printf(c, "<div class='navs_wrapper'>\n"
-		    "<div class='navs'>"
-		    "<a href='?index_page=%s&path=%s&action=diff&commit=%s"
-		    "&headref=%s'>diff</a>"
-		    " | "
-		    "<a href='?index_page=%s&path=%s&action=tree&commit=%s"
-		    "&headref=%s'>tree</a>"
-		    "</div>\n"	/* .navs */
-		    "</div>\n"	/* .navs_wrapper */
-		    "<div class='dotted_line'></div>\n",
-		    index_page_str, repo_dir->name, rc->commit_id, qs->headref,
-		    index_page_str, repo_dir->name, rc->commit_id, qs->headref);
+		    "<div class='navs'>");
 		if (r == -1)
 			goto done;
 
+		r = gotweb_link(c, &(struct gotweb_url){
+			.action = DIFF,
+			.index_page = -1,
+			.page = -1,
+			.path = repo_dir->name,
+			.commit = rc->commit_id,
+			.headref = qs->headref,
+		    }, "diff");
+		if (r == -1)
+			goto done;
+
+		if (fcgi_printf(c, " | ") == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.action = TREE,
+			.index_page = -1,
+			.page = -1,
+			.path = repo_dir->name,
+			.commit = rc->commit_id,
+			.headref = qs->headref,
+		    }, "tree");
+		if (r == -1)
+			goto done;
+
+		if (fcgi_printf(c, "</div>\n"	/* .navs */
+		    "</div>\n"	/* .navs_wrapper */
+		    "<div class='dotted_line'></div>\n") == -1)
+			goto done;
+
 		free(age);
 		age = NULL;
 		free(author);
@@ -1355,14 +1422,10 @@ gotweb_render_commits(struct request *c)
 	struct repo_commit *rc = NULL;
 	struct server *srv = c->srv;
 	struct transport *t = c->t;
-	struct querystring *qs = t->qs;
 	struct repo_dir *repo_dir = t->repo_dir;
-	const char *index_page_str;
 	char *age = NULL, *author = NULL, *msg = NULL;
 	int r;
 
-	index_page_str = qs->index_page_str ? qs->index_page_str : "";
-
 	r = fcgi_printf(c, "<div class='commits_title_wrapper'>\n"
 	    "<div class='commits_title'>Commits</div>\n"
 	    "</div>\n"		/* .commits_title_wrapper */
@@ -1404,21 +1467,38 @@ gotweb_render_commits(struct request *c)
 		if (r == -1)
 			goto done;
 
-		r = fcgi_printf(c, "<div class='navs_wrapper'>\n"
-		    "<div class='navs'>"
-		    "<a href='?index_page=%s&path=%s&action=diff&commit=%s'>"
-		    "diff</a>"
-		    " | "
-		    "<a href='?index_page=%s&path=%s&action=tree&commit=%s'>"
-		    "tree</a>"
-		    "</div>\n"	/* .navs */
-		    "</div>\n"	/* .navs_wrapper */
-		    "<div class='dotted_line'></div>\n",
-		    index_page_str, repo_dir->name, rc->commit_id,
-		    index_page_str, repo_dir->name, rc->commit_id);
+		if (fcgi_printf(c, "<div class='navs_wrapper'>\n"
+		    "<div class='navs'>") == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.action = DIFF,
+			.index_page = -1,
+			.page = -1,
+			.path = repo_dir->name,
+			.commit = rc->commit_id,
+		    }, "diff");
 		if (r == -1)
 			goto done;
 
+		if (fcgi_printf(c, " | ") == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.action = TREE,
+			.index_page = -1,
+			.page = -1,
+			.path = repo_dir->name,
+			.commit = rc->commit_id,
+		    }, "tree");
+		if (r == -1)
+			goto done;
+
+		if (fcgi_printf(c, "</div>\n"	/* .navs */
+		    "</div>\n"	/* .navs_wrapper */
+		    "<div class='dotted_line'></div>\n") == -1)
+			goto done;
+
 		free(age);
 		age = NULL;
 		free(author);
@@ -1449,12 +1529,10 @@ gotweb_render_branches(struct request *c)
 	struct transport *t = c->t;
 	struct querystring *qs = t->qs;
 	struct got_repository *repo = t->repo;
-	const char *index_page_str;
+	char *escaped_refname = NULL;
 	char *age = NULL;
 	int r;
 
-	index_page_str = qs->index_page_str ? qs->index_page_str : "";
-
 	TAILQ_INIT(&refs);
 
 	error = got_ref_list(&refs, repo, "refs/heads",
@@ -1471,7 +1549,6 @@ gotweb_render_branches(struct request *c)
 
 	TAILQ_FOREACH(re, &refs, entry) {
 		const char *refname = NULL;
-		char *escaped_refname = NULL;
 
 		if (got_ref_is_symbolic(re->ref))
 			continue;
@@ -1498,41 +1575,77 @@ gotweb_render_branches(struct request *c)
 		r = fcgi_printf(c, "<div class='branches_wrapper'>\n"
 		    "<div class='branches_age'>%s</div>\n"
 		    "<div class='branches_space'>&nbsp;</div>\n"
-		    "<div class='branch'>"
-		    "<a href='?index_page=%s&path=%s&action=summary&headref=%s'>"
-		    "%s</a>"
-		    "</div>\n"	/* .branch */
+		    "<div class='branch'>", age);
+		if (r == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.action = SUMMARY,
+			.index_page = -1,
+			.page = -1,
+			.path = qs->path,
+			.headref = refname,
+		    }, "%s", escaped_refname);
+		if (r == -1)
+			goto done;
+
+		if (fcgi_printf(c, "</div>\n"	/* .branch */
 		    "<div class='navs_wrapper'>\n"
-		    "<div class='navs'>"
-		    "<a href='?index_page=%s&path=%s&action=summary&headref=%s'>"
-		    "summary</a>"
-		    " | "
-		    "<a href='?index_page=%s&path=%s&action=briefs&headref=%s'>"
-		    "commit briefs</a>"
-		    " | "
-		    "<a href='?index_page=%s&path=%s&action=commits&headref=%s'>"
-		    "commits</a>"
-		    "</div>\n"	/* .navs */
-		    "</div>\n"	/* .navs_wrapper */
+		    "<div class='navs'>") == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.action = SUMMARY,
+			.index_page = -1,
+			.page = -1,
+			.path = qs->path,
+			.headref = refname,
+		    }, "summary");
+		if (r == -1)
+			goto done;
+
+		if (fcgi_printf(c, " | ") == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.action = BRIEFS,
+			.index_page = -1,
+			.page = -1,
+			.path = qs->path,
+			.headref = refname,
+		    }, "commit briefs");
+		if (r == -1)
+			goto done;
+
+		if (fcgi_printf(c, " | ") == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.action = COMMITS,
+			.index_page = -1,
+			.page = -1,
+			.path = qs->path,
+			.headref = refname,
+		    }, "commits");
+		if (r == -1)
+			goto done;
+
+		r = fcgi_printf(c, "</div>\n"	/* .navs */
+		    "</div>\n"			/* .navs_wrapper */
 		    "<div class='dotted_line'></div>\n"
-		    "</div>\n",	/* .branches_wrapper */
-		    age ? age : "",
-		    index_page_str, qs->path, refname,
-		    escaped_refname,
-		    index_page_str, qs->path, refname,
-		    index_page_str, qs->path, refname,
-		    index_page_str, qs->path, refname);
-		free(escaped_refname);
+		    "</div>\n");		/* .branches_wrapper */
 		if (r == -1)
 			goto done;
 
 		free(age);
 		age = NULL;
-
+		free(escaped_refname);
+		escaped_refname = NULL;
 	}
 	fcgi_printf(c, "</div>\n"); /* #branches_content */
 done:
 	free(age);
+	free(escaped_refname);
 	got_ref_list_free(&refs);
 	return error;
 }
@@ -1813,12 +1926,9 @@ gotweb_render_tags(struct request *c)
 	struct transport *t = c->t;
 	struct querystring *qs = t->qs;
 	struct repo_dir *repo_dir = t->repo_dir;
-	const char *index_page_str;
 	char *age = NULL, *tagname = NULL, *msg = NULL, *newline;
 	int r, commit_found = 0;
 
-	index_page_str = qs->index_page_str ? qs->index_page_str : "";
-
 	if (qs->action == BRIEFS) {
 		qs->action = TAGS;
 		error = got_get_repo_tags(c, D_MAXSLCOMMDISP);
@@ -1868,32 +1978,66 @@ gotweb_render_tags(struct request *c)
 				goto done;
 		}
 
-		r = fcgi_printf(c, "<div class='tag_age'>%s</div>\n"
+		if (fcgi_printf(c, "<div class='tag_age'>%s</div>\n"
 		    "<div class='tag'>%s</div>\n"
-		    "<div class='tag_log'>"
-		    "<a href='?index_page=%s&path=%s&action=tag&commit=%s'>"
-		    "%s</a>"
-		    "</div>\n"	/* .tag_log */
+		    "<div class='tag_log'>", age, tagname) == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.action = TAG,
+			.index_page = -1,
+			.page = -1,
+			.path = repo_dir->name,
+			.commit = rt->commit_id,
+		    }, "%s", msg ? msg : "");
+		if (r == -1)
+			goto done;
+
+		if (fcgi_printf(c, "</div>\n"	/* .tag_log */
 		    "<div class='navs_wrapper'>\n"
-		    "<div class='navs'>"
-		    "<a href='?index_page=%s&path=%s&action=tag&commit=%s'>"
-		    "tag</a>"
-		    " | "
-		    "<a href='?index_page=%s&path=%s&action=briefs&commit=%s'>"
-		    "commit briefs</a>"
-		    " | "
-		    "<a href='?index_page=%s&path=%s&action=commits&commit=%s'>"
-		    "commits</a>"
+		    "<div class='navs'>") == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.action = TAG,
+			.index_page = -1,
+			.page = -1,
+			.path = repo_dir->name,
+			.commit = rt->commit_id,
+		    }, "tag");
+		if (r == -1)
+			goto done;
+
+		if (fcgi_printf(c, " | ") == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.action = BRIEFS,
+			.index_page = -1,
+			.page = -1,
+			.path = repo_dir->name,
+			.commit = rt->commit_id,
+		    }, "commit briefs");
+		if (r == -1)
+			goto done;
+
+		if (fcgi_printf(c, " | ") == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.action = COMMITS,
+			.index_page = -1,
+			.page = -1,
+			.path = repo_dir->name,
+			.commit = rt->commit_id,
+		    }, "commits");
+		if (r == -1)
+			goto done;
+
+		r = fcgi_printf(c,
 		    "</div>\n"	/* .navs */
 		    "</div>\n"	/* .navs_wrapper */
-		    "<div class='dotted_line'></div>\n",
-		    age,
-		    tagname,
-		    index_page_str, repo_dir->name, rt->commit_id,
-		    msg ? msg : "",
-		    index_page_str, repo_dir->name, rt->commit_id,
-		    index_page_str, repo_dir->name, rt->commit_id,
-		    index_page_str, repo_dir->name, rt->commit_id);
+		    "<div class='dotted_line'></div>\n");
 		if (r == -1)
 			goto done;
 
@@ -1981,6 +2125,223 @@ done:
 	return error;
 }
 
+static inline int
+should_urlencode(int c)
+{
+	if (c <= ' ' || c >= 127)
+		return 1;
+
+	switch (c) {
+		/* gen-delim */
+	case ':':
+	case '/':
+	case '?':
+	case '#':
+	case '[':
+	case ']':
+	case '@':
+		/* sub-delims */
+	case '!':
+	case '$':
+	case '&':
+	case '\'':
+	case '(':
+	case ')':
+	case '*':
+	case '+':
+	case ',':
+	case ';':
+	case '=':
+		return 1;
+	default:
+		return 0;
+	}
+}
+
+static char *
+gotweb_urlencode(const char *str)
+{
+	const char *s;
+	char *escaped;
+	size_t i, len;
+	int a, b;
+
+	len = 0;
+	for (s = str; *s; ++s) {
+		len++;
+		if (should_urlencode(*s))
+			len += 2;
+	}
+
+	escaped = calloc(1, len + 1);
+	if (escaped == NULL)
+		return NULL;
+
+	i = 0;
+	for (s = str; *s; ++s) {
+		if (should_urlencode(*s)) {
+			a = (*s & 0xF0) >> 4;
+			b = (*s & 0x0F);
+
+			escaped[i++] = '%';
+			escaped[i++] = a <= 9 ? ('0' + a) : ('7' + a);
+			escaped[i++] = b <= 9 ? ('0' + b) : ('7' + b);
+		} else
+			escaped[i++] = *s;
+	}
+
+	return escaped;
+}
+
+static inline const char *
+action_name(int action)
+{
+	switch (action) {
+	case BLAME:
+		return "blame";
+	case BLOB:
+		return "blob";
+	case BRIEFS:
+		return "briefs";
+	case COMMITS:
+		return "commits";
+	case DIFF:
+		return "diff";
+	case ERR:
+		return "err";
+	case INDEX:
+		return "index";
+	case SUMMARY:
+		return "summary";
+	case TAG:
+		return "tag";
+	case TAGS:
+		return "tags";
+	case TREE:
+		return "tree";
+	default:
+		return NULL;
+	}
+}
+
+static int
+gotweb_print_url(struct request *c, struct gotweb_url *url)
+{
+	const char *sep = "?", *action;
+	char *tmp;
+	int r;
+
+	action = action_name(url->action);
+	if (action != NULL) {
+		if (fcgi_printf(c, "?action=%s", action) == -1)
+			return -1;
+		sep = "&";
+	}
+
+	if (url->commit) {
+		if (fcgi_printf(c, "%scommit=%s", sep, url->commit) == -1)
+			return -1;
+		sep = "&";
+	}
+
+	if (url->previd) {
+		if (fcgi_printf(c, "%sprevid=%s", sep, url->previd) == -1)
+			return -1;
+		sep = "&";
+	}
+
+	if (url->prevset) {
+		if (fcgi_printf(c, "%sprevset=%s", sep, url->prevset) == -1)
+			return -1;
+		sep = "&";
+	}
+
+	if (url->file) {
+		tmp = gotweb_urlencode(url->file);
+		if (tmp == NULL)
+			return -1;
+		r = fcgi_printf(c, "%sfile=%s", sep, tmp);
+		free(tmp);
+		if (r == -1)
+			return -1;
+		sep = "&";
+	}
+
+	if (url->folder) {
+		tmp = gotweb_urlencode(url->folder);
+		if (tmp == NULL)
+			return -1;
+		r = fcgi_printf(c, "%sfolder=%s", sep, tmp);
+		free(tmp);
+		if (r == -1)
+			return -1;
+		sep = "&";
+	}
+
+	if (url->headref) {
+		tmp = gotweb_urlencode(url->headref);
+		if (tmp == NULL)
+			return -1;
+		r = fcgi_printf(c, "%sheadref=%s", sep, url->headref);
+		free(tmp);
+		if (r == -1)
+			return -1;
+		sep = "&";
+	}
+
+	if (url->index_page != -1) {
+		if (fcgi_printf(c, "%sindex_page=%d", sep,
+		    url->index_page) == -1)
+			return -1;
+		sep = "&";
+	}
+
+	if (url->path) {
+		tmp = gotweb_urlencode(url->path);
+		if (tmp == NULL)
+			return -1;
+		r = fcgi_printf(c, "%spath=%s", sep, tmp);
+		free(tmp);
+		if (r == -1)
+			return -1;
+		sep = "&";
+	}
+
+	if (url->page != -1) {
+		if (fcgi_printf(c, "%spage=%d", sep, url->page) == -1)
+			return -1;
+		sep = "&";
+	}
+
+	return 0;
+}
+
+int
+gotweb_link(struct request *c, struct gotweb_url *url, const char *fmt, ...)
+{
+	va_list ap;
+	int r;
+
+	if (fcgi_printf(c, "<a href='") == -1)
+		return -1;
+
+	if (gotweb_print_url(c, url) == -1)
+		return -1;
+
+	if (fcgi_printf(c, "'>") == -1)
+		return -1;
+
+	va_start(ap, fmt);
+	r = fcgi_vprintf(c, fmt, ap);
+	va_end(ap);
+	if (r == -1)
+		return -1;
+
+	if (fcgi_printf(c, "</a>"))
+		return -1;
+	return 0;
+}
+
 static struct got_repository *
 find_cached_repo(struct server *srv, const char *path)
 {
blob - f73ea6b8311442f992dbc1c563f90486ef813c04
blob + 06f8d01516c1b327e8524a128e571a8cc082ca7e
--- gotwebd/gotwebd.h
+++ gotwebd/gotwebd.h
@@ -341,6 +341,23 @@ struct gotwebd {
 	char		 unix_socket_name[PATH_MAX];
 };
 
+/*
+ * URL parameter for gotweb_link.  NULL values and int set to -1 are
+ * implicitly ignored, and string are properly escaped.
+ */
+struct gotweb_url {
+	int		 action;
+	int		 index_page;
+	int		 page;
+	const char	*commit;
+	const char	*previd;
+	const char	*prevset;
+	const char	*file;
+	const char	*folder;
+	const char	*headref;
+	const char	*path;
+};
+
 struct querystring {
 	uint8_t		 action;
 	char		*commit;
@@ -411,6 +428,9 @@ const struct got_error
 const struct got_error *gotweb_get_time_str(char **, time_t, int);
 const struct got_error *gotweb_init_transport(struct transport **);
 const struct got_error *gotweb_escape_html(char **, const char *);
+int gotweb_link(struct request *, struct gotweb_url *, const char *, ...)
+	__attribute__((__format__(printf, 3, 4)))
+	__attribute__((__nonnull__(3)));
 void gotweb_free_repo_commit(struct repo_commit *);
 void gotweb_free_repo_tag(struct repo_tag *);
 void gotweb_process_request(struct request *);
@@ -426,6 +446,7 @@ void fcgi_timeout(int, short, void *);
 void fcgi_cleanup_request(struct request *);
 void fcgi_create_end_record(struct request *);
 void dump_fcgi_record(const char *, struct fcgi_record_header *);
+int fcgi_vprintf(struct request *, const char *, va_list);
 int fcgi_printf(struct request *, const char *, ...)
 	__attribute__((__format__(printf, 2, 3)))
 	__attribute__((__nonnull__(2)));