4 files changed, 53 insertions, 25 deletions
diff --git a/source/html.c b/source/html.c
index 367cea7..1a6b7c5 100644
--- a/source/html.c
+++ b/source/html.c
@@ -314,6 +314,18 @@ void html_url_path(const char *txt)
 		html(txt);
 }
 
+void cgit_add_url_path(struct strbuf *sb, const char *txt)
+{
+	for (; *txt; txt++) {
+		const char *esc = url_escape_table[(unsigned char)*txt];
+
+		if (esc)
+			strbuf_addstr(sb, esc);
+		else
+			strbuf_addch(sb, *txt);
+	}
+}
+
 void html_url_arg(const char *txt)
 {
 	const char *p = txt;
diff --git a/source/html.h b/source/html.h
index 1960b6a..095804c 100644
--- a/source/html.h
+++ b/source/html.h
@@ -62,6 +62,14 @@ extern ssize_t html_ntxt(const char *txt, size_t len);
 
 extern void html_attr(const char *txt);
 extern void html_url_path(const char *txt);
+
+/*
+ * Append txt to sb with the escaping html_url_path writes, for a url that is
+ * assembled before it reaches a sink. Nothing is captured, so it is safe to
+ * call while a capture is open.
+ */
+extern void cgit_add_url_path(struct strbuf *sb, const char *txt);
+
 extern void html_url_arg(const char *txt);
 extern void html_header_arg_in_quotes(const char *txt);
 extern void html_header_query(const char *txt);
diff --git a/source/ui-shared.c b/source/ui-shared.c
index f762fd7..409ad40 100644
--- a/source/ui-shared.c
+++ b/source/ui-shared.c
@@ -651,11 +651,8 @@ static void print_repo_tabs(void)
 static void print_repo_search(void)
 {
 	html("<form class='search' role='search' method='get' action='");
-	if (ctx.cfg.virtual_root) {
-		char *fileurl = cgit_fileurl(ctx.qry.repo, "log", ctx.qry.vpath, NULL);
-		html_url_path(fileurl);
-		free(fileurl);
-	}
+	if (ctx.cfg.virtual_root)
+		write_page_url("log", ctx.qry.vpath);
 	html("'>\n");
 	cgit_add_hidden_formfields(1, 0, "log");
 	html("<select name='qt' aria-label='Search type'>\n");
@@ -896,10 +893,17 @@ const char *cgit_loginurl(void)
 
 char *cgit_repourl(const char *reponame)
 {
-	if (ctx.cfg.virtual_root)
-		return cgit_fmtalloc("%s%s/", ctx.cfg.virtual_root, reponame);
-	else
-		return cgit_fmtalloc("?r=%s", reponame);
+	struct strbuf sb = STRBUF_INIT;
+
+	if (ctx.cfg.virtual_root) {
+		strbuf_addstr(&sb, ctx.cfg.virtual_root);
+		cgit_add_url_path(&sb, reponame);
+		strbuf_addch(&sb, '/');
+	} else {
+		strbuf_addstr(&sb, "?r=");
+		cgit_add_url_path(&sb, reponame);
+	}
+	return strbuf_detach(&sb, NULL);
 }
 
 char *cgit_fileurl(const char *reponame, const char *pagename, const char *filename,
@@ -908,16 +912,17 @@ char *cgit_fileurl(const char *reponame, const char *pagename, const char *filen
 	struct strbuf sb = STRBUF_INIT;
 	const char *delim;
 
-	// The result is a raw URL, so the query joiner is a bare ampersand
-	// and each sink escapes the whole string for wherever it lands.
+	// The query joiner is a bare ampersand, because each sink escapes the
+	// whole string for wherever it lands.
 	if (ctx.cfg.virtual_root) {
-		strbuf_addf(&sb, "%s%s/%s/%s", ctx.cfg.virtual_root, reponame, pagename,
-			filename ? filename : "");
+		strbuf_addstr(&sb, ctx.cfg.virtual_root);
 		delim = "?";
 	} else {
-		strbuf_addf(&sb, "?url=%s/%s/%s", reponame, pagename, (filename ? filename : ""));
+		strbuf_addstr(&sb, "?url=");
 		delim = "&";
 	}
+	cgit_add_url_path(&sb, reponame);
+	strbuf_addf(&sb, "/%s/%s", pagename, filename ? filename : "");
 	if (query)
 		strbuf_addf(&sb, "%s%s", delim, query);
 	return strbuf_detach(&sb, NULL);
@@ -1509,7 +1514,7 @@ void cgit_redirect(const char *url, const char *query, bool permanent)
 {
 	htmlf("Status: %d %s\n", permanent ? 301 : 302, permanent ? "Moved Permanently" : "Found");
 	html("Location: ");
-	html_url_path(url);
+	html_header_query(url);
 	if (query)
 		html_header_query(query);
 	html("\n\n");
@@ -1580,18 +1585,13 @@ void cgit_print_docstart(void)
 		html("'>\n");
 	}
 	if (host && ctx.repo && ctx.qry.head) {
-		char *fileurl;
-		struct strbuf sb = STRBUF_INIT;
-		strbuf_addf(&sb, "h=%s", ctx.qry.head);
-
 		html("<link rel='alternate' title='Atom feed' href='");
 		html(cgit_httpscheme());
 		html_attr(host);
-		fileurl = cgit_fileurl(ctx.repo->url, "atom", ctx.qry.vpath, sb.buf);
-		html_attr(fileurl);
+		html(write_page_url("atom", ctx.qry.vpath) ? "&amp;" : "?");
+		html("h=");
+		html_url_arg(ctx.qry.head);
 		html("' type='application/atom+xml'>\n");
-		strbuf_release(&sb);
-		free(fileurl);
 	}
 	if (ctx.repo)
 		cgit_add_clone_urls(print_rel_vcs_link);
diff --git a/source/ui-shared.h b/source/ui-shared.h
index fdfdbed..5a1e5fc 100644
--- a/source/ui-shared.h
+++ b/source/ui-shared.h
@@ -25,6 +25,13 @@ extern char *cgit_currenturl(void);
 extern char *cgit_currentfullurl(void);
 extern char *cgit_currentquery(void);
 extern const char *cgit_loginurl(void);
+
+/*
+ * The urls of a repository and of its pages. The repository url is encoded
+ * here, since it is configuration or a scanned path and may hold any byte,
+ * while a page name, file name or query must arrive encoded already. The
+ * result still needs escaping for wherever it lands.
+ */
 extern char *cgit_repourl(const char *reponame);
 extern void cgit_print_clone_row(const char *url, int colspan);
 extern char *cgit_fileurl(const char *reponame, const char *pagename, const char *filename,
@@ -96,8 +103,9 @@ extern struct date_mode cgit_date_mode(enum date_mode_type type);
 extern void cgit_print_age(time_t t, int tz, time_t max_relative);
 extern void cgit_print_http_headers(void);
 /*
- * Answer with a redirect to url, followed by query when one is given, which
- * is written as it arrived.
+ * Answer with a redirect to url, followed by query when one is given. Both
+ * are written as they arrived, encoded already, the url the way cgit_repourl
+ * builds it, and only a byte a header line cannot carry is encoded here.
  */
 extern void cgit_redirect(const char *url, const char *query, bool permanent);
 extern void cgit_print_docstart(void);