TL;DR: wanna break gotwebd?

	$ touch "' onclick='alert(\"p0wn3d\");' foo='"
	$ got add -R .
	$ got commit -m ...
	$ got send

enjoy!

(this doesn't work on modern browsers due to the CSP, but gives the
idea.)


Longer version: gotwebd doesn't encode the arguments in the generated
URLs.  This is a companion diff to the other diff i've sent ("gotwebd:
urldecode querystring") and deals with how URLs are generated.

Some URL parameters needs to be encoded as they contain data that may
otherwise break the HTML.  Among these `folder', `file' and `path'.
Instead of making the current URL construction even more complex, I
decided to take it as a chance to refactor a bit how we generate URLs.

Diff below introduces a new helper: `gotweb_link'.  It takes the
request, a struct gotweb_url that represent the request, followed by a
printf-format string and arguments.  It makes gotwebd gain a few lines
of codes because now the various fcgi_printf calls needs to be
splitted, but otherwise I think it's an improvement.

What do y'all think?  Is it OK to use the syntax

	r = gotweb_link(c, &(struct gotweb_url){
		.index_page = -1,
		.action = DIFF,
		.page = -1,
		.path = repo_dir->name,
		.commit = bline->id_str,
	    }, "%.8s", bline->id_str);

to pass an anounymous struct pointer?

diff /home/op/w/got
commit - e5e662e42c45f0d30f5f97fb0e2ad5f3c4f8b488
path + /home/op/w/got
blob - e77b8424231bf88c153a32dd90a19292c41dc39d
file + gotwebd/fcgi.c
--- 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 - 14d7cbf7261ca9171a51aa4286a2d0870ffcf937
file + gotwebd/got_operations.c
--- gotwebd/got_operations.c
+++ gotwebd/got_operations.c
@@ -882,42 +882,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 */
+			struct gotweb_url url = {
+				.index_page = -1,
+				.page = -1,
+				.action = TREE,
+				.commit = rc->commit_id,
+				.path = qs->path,
+				/* folder 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", /* .tree_wrapper */
-			    index_page_str, qs->path, rc->commit_id,
-			    folder, name, escaped_name, modestr);
-			if (r == -1)
+			    "</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;
@@ -1155,19 +1189,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){
+			.index_page = -1,
+			.action = DIFF,
+			.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 - 00c0cad7a1dfa41e7edf76bc95925af1573138c8 (staged)
file + gotwebd/gotweb.c
--- gotwebd/gotweb.c
+++ gotwebd/gotweb.c
@@ -669,6 +669,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;
@@ -718,10 +719,22 @@ 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){
+				.index_page = -1,
+				.page = -1,
+				.action = SUMMARY,
+				.path = qs->path,
+			    }, "%s", epath);
+			free(epath);
+
 			if (r == -1)
 				goto done;
 		}
@@ -801,8 +814,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)
@@ -811,140 +823,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 = {
+				.index_page = qs->index_page - 1,
+				.action = -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 = {
+				.index_page = -1,
+				.path = qs->path,
+				.page = qs->page - 1,
+				.action = BRIEFS,
+				.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 = {
+				.index_page = -1,
+				.path = qs->path,
+				.page = qs->page - 1,
+				.action = COMMIT,
+				.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 = {
+				.index_page = -1,
+				.path = qs->path,
+				.page = qs->page - 1,
+				.action = TAGS,
+				.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 = {
+				.index_page = qs->index_page + 1,
+				.action = -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 = {
+				.index_page = -1,
+				.path = qs->path,
+				.page = qs->page + 1,
+				.action = BRIEFS,
+				.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 = {
+				.index_page = -1,
+				.path = qs->path,
+				.page = qs->page + 1,
+				.action = COMMIT,
+				.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 = {
+				.index_page = -1,
+				.path = qs->path,
+				.page = qs->page + 1,
+				.action = TAGS,
+				.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:
@@ -952,8 +958,6 @@ done:
 	t->next_id = NULL;
 	free(t->prev_id);
 	t->prev_id = NULL;
-	free(phref);
-	free(nhref);
 	return error;
 }
 
@@ -1066,17 +1070,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){
+			.index_page = -1,
+			.page = -1,
+			.action = SUMMARY,
+			.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"
@@ -1099,32 +1108,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){
+			.index_page = -1,
+			.page = -1,
+			.action = SUMMARY,
+			.path = repo_dir->name
+		    }, "summary");
+		if (r == -1)
+			goto done;
+
+		if (fcgi_printf(c, " | ") == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.index_page = -1,
+			.page = -1,
+			.action = BRIEFS,
+			.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){
+			.index_page = -1,
+			.page = -1,
+			.action = COMMITS,
+			.path = repo_dir->name
+		    }, "commits");
+		if (r == -1)
+			goto done;
+
+		if (fcgi_printf(c, " | ") == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.index_page = -1,
+			.page = -1,
+			.action = TAGS,
+			.path = repo_dir->name
+		    }, "tags");
+		if (r == -1)
+			goto done;
+
+		if (fcgi_printf(c, " | ") == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.index_page = -1,
+			.page = -1,
+			.action = TREE,
+			.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;
 
@@ -1262,16 +1310,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){
+			.index_page = -1,
+			.action = DIFF,
+			.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;
 
@@ -1288,18 +1342,38 @@ 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 class='navs'>");
+		if (r == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.index_page = -1,
+			.page = -1,
+			.action = DIFF,
+			.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){
+			.index_page = -1,
+			.page = -1,
+			.action = TREE,
+			.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",
-		    index_page_str, repo_dir->name, rc->commit_id, qs->headref,
-		    index_page_str, repo_dir->name, rc->commit_id, qs->headref);
-		if (r == -1)
+		    "<div class='dotted_line'></div>\n") == -1)
 			goto done;
 
 		free(age);
@@ -1379,19 +1453,36 @@ 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 */
+		if (fcgi_printf(c, "<div class='navs_wrapper'>\n"
+		    "<div class='navs'>") == -1)
+			goto done;
+
+		r = gotweb_link(c, &(struct gotweb_url){
+			.index_page = -1,
+			.page = -1,
+			.action = DIFF,
+			.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){
+			.index_page = -1,
+			.page = -1,
+			.action = TREE,
+			.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",
-		    index_page_str, repo_dir->name, rc->commit_id,
-		    index_page_str, repo_dir->name, rc->commit_id);
-		if (r == -1)
+		    "<div class='dotted_line'></div>\n") == -1)
 			goto done;
 
 		free(age);
@@ -1424,12 +1515,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",
@@ -1446,7 +1535,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;
@@ -1473,41 +1561,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){
+			.index_page = -1,
+			.page = -1,
+			.action = SUMMARY,
+			.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){
+			.index_page = -1,
+			.page = -1,
+			.action = SUMMARY,
+			.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){
+			.index_page = -1,
+			.page = -1,
+			.action = BRIEFS,
+			.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){
+			.index_page = -1,
+			.page = -1,
+			.action = COMMITS,
+			.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);
+		    "</div>\n");		/* .branches_wrapper */
+		if (r == -1)
+			goto done;
+
+		free(age);
+		age = NULL;
 		free(escaped_refname);
-		if (r == -1)
-			goto done;
-
-		free(age);
-		age = NULL;
-
+		escaped_refname = NULL;
 	}
 	fcgi_printf(c, "</div>\n"); /* #branches_content */
 done:
 	free(age);
+	free(escaped_refname);
 	got_ref_list_free(&refs);
 	return error;
 }
@@ -1787,12 +1911,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);
@@ -1842,32 +1963,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){
+			.index_page = -1,
+			.page = -1,
+			.action = TAG,
+			.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){
+			.index_page = -1,
+			.page = -1,
+			.action = TAG,
+			.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){
+			.index_page = -1,
+			.page = -1,
+			.action = BRIEFS,
+			.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){
+			.index_page = -1,
+			.page = -1,
+			.action = COMMITS,
+			.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;
 
@@ -1955,6 +2110,219 @@ 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;
+	char *eip = NULL, *ep = NULL, *ef = NULL, *efile = NULL;
+	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) {
+		if (fcgi_printf(c, "%sheadref=%s", sep, url->headref) == -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 - ac7f962b1abdcfe1c535e8bc210c9f619e7a2cbc
file + gotwebd/gotwebd.h
--- gotwebd/gotwebd.h
+++ gotwebd/gotwebd.h
@@ -341,6 +341,19 @@ struct gotwebd {
 	char		 unix_socket_name[PATH_MAX];
 };
 
+struct gotweb_url {
+	int		 action;
+	const char	*commit;
+	const char	*previd;
+	const char	*prevset;
+	const char	*file;
+	const char	*folder;
+	const char	*headref;
+	int		 index_page;
+	const char	*path;
+	int		 page;
+};
+
 struct querystring {
 	uint8_t		 action;
 	char		*commit;
@@ -411,6 +424,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 +442,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)));