Fix ampersand handling in generated urls

author: Bryce Kwon <bryce@brycekwon.com> 2026-08-12 20:31:56 -1000
committer: Bryce Kwon <bryce@brycekwon.com> 2026-08-12 20:31:56 -1000
commit: 80c877efd69192c576a2258a26265d5a70cdee04 (patch)
parent: fa00b5ee9677afff26259626ce9b1a29d4895717 (diff)
tree: a01147abb97913fa49fe476ab7a2404b1971ae29
download: cgit-80c877efd69192c576a2258a26265d5a70cdee04.tar.gz
cgit-80c877efd69192c576a2258a26265d5a70cdee04.zip
4 files changed, 35 insertions, 30 deletions
diff --git a/source/html.c b/source/html.c
index 58ccf3b..f69f109 100644
--- a/source/html.c
+++ b/source/html.c
@@ -18,37 +18,37 @@
 // carry as themselves. Those are the letters, the digits, and !$()*,-./:;@[]_~
 static const char *url_escape_table[256] = {
 	"%00", "%01", "%02", "%03", "%04", "%05", "%06", "%07",
-	"%08", "%09", "%0a", "%0b", "%0c", "%0d", "%0e", "%0f",
+	"%08", "%09", "%0A", "%0B", "%0C", "%0D", "%0E", "%0F",
 	"%10", "%11", "%12", "%13", "%14", "%15", "%16", "%17",
-	"%18", "%19", "%1a", "%1b", "%1c", "%1d", "%1e", "%1f",
+	"%18", "%19", "%1A", "%1B", "%1C", "%1D", "%1E", "%1F",
 	"%20", NULL,  "%22", "%23", NULL,  "%25", "%26", "%27",
-	NULL,  NULL,  NULL,  "%2b", NULL,  NULL,  NULL,  NULL,
+	NULL,  NULL,  NULL,  "%2B", NULL,  NULL,  NULL,  NULL,
 	NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,
-	NULL,  NULL,  NULL,  NULL,  "%3c", "%3d", "%3e", "%3f",
+	NULL,  NULL,  NULL,  NULL,  "%3C", "%3D", "%3E", "%3F",
 	NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,
 	NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,
 	NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,
-	NULL,  NULL,  NULL,  NULL,  "%5c", NULL,  "%5e", NULL,
+	NULL,  NULL,  NULL,  NULL,  "%5C", NULL,  "%5E", NULL,
 	"%60", NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,
 	NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,
 	NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,
-	NULL,  NULL,  NULL,  "%7b", "%7c", "%7d", NULL,  "%7f",
+	NULL,  NULL,  NULL,  "%7B", "%7C", "%7D", NULL,  "%7F",
 	"%80", "%81", "%82", "%83", "%84", "%85", "%86", "%87",
-	"%88", "%89", "%8a", "%8b", "%8c", "%8d", "%8e", "%8f",
+	"%88", "%89", "%8A", "%8B", "%8C", "%8D", "%8E", "%8F",
 	"%90", "%91", "%92", "%93", "%94", "%95", "%96", "%97",
-	"%98", "%99", "%9a", "%9b", "%9c", "%9d", "%9e", "%9f",
-	"%a0", "%a1", "%a2", "%a3", "%a4", "%a5", "%a6", "%a7",
-	"%a8", "%a9", "%aa", "%ab", "%ac", "%ad", "%ae", "%af",
-	"%b0", "%b1", "%b2", "%b3", "%b4", "%b5", "%b6", "%b7",
-	"%b8", "%b9", "%ba", "%bb", "%bc", "%bd", "%be", "%bf",
-	"%c0", "%c1", "%c2", "%c3", "%c4", "%c5", "%c6", "%c7",
-	"%c8", "%c9", "%ca", "%cb", "%cc", "%cd", "%ce", "%cf",
-	"%d0", "%d1", "%d2", "%d3", "%d4", "%d5", "%d6", "%d7",
-	"%d8", "%d9", "%da", "%db", "%dc", "%dd", "%de", "%df",
-	"%e0", "%e1", "%e2", "%e3", "%e4", "%e5", "%e6", "%e7",
-	"%e8", "%e9", "%ea", "%eb", "%ec", "%ed", "%ee", "%ef",
-	"%f0", "%f1", "%f2", "%f3", "%f4", "%f5", "%f6", "%f7",
-	"%f8", "%f9", "%fa", "%fb", "%fc", "%fd", "%fe", "%ff"
+	"%98", "%99", "%9A", "%9B", "%9C", "%9D", "%9E", "%9F",
+	"%A0", "%A1", "%A2", "%A3", "%A4", "%A5", "%A6", "%A7",
+	"%A8", "%A9", "%AA", "%AB", "%AC", "%AD", "%AE", "%AF",
+	"%B0", "%B1", "%B2", "%B3", "%B4", "%B5", "%B6", "%B7",
+	"%B8", "%B9", "%BA", "%BB", "%BC", "%BD", "%BE", "%BF",
+	"%C0", "%C1", "%C2", "%C3", "%C4", "%C5", "%C6", "%C7",
+	"%C8", "%C9", "%CA", "%CB", "%CC", "%CD", "%CE", "%CF",
+	"%D0", "%D1", "%D2", "%D3", "%D4", "%D5", "%D6", "%D7",
+	"%D8", "%D9", "%DA", "%DB", "%DC", "%DD", "%DE", "%DF",
+	"%E0", "%E1", "%E2", "%E3", "%E4", "%E5", "%E6", "%E7",
+	"%E8", "%E9", "%EA", "%EB", "%EC", "%ED", "%EE", "%EF",
+	"%F0", "%F1", "%F2", "%F3", "%F4", "%F5", "%F6", "%F7",
+	"%F8", "%F9", "%FA", "%FB", "%FC", "%FD", "%FE", "%FF"
 };
 
 static char out_buf[HTML_WRITE_BUFSIZE];
@@ -256,11 +256,13 @@ void html_url_path(const char *txt)
 	const char *p = txt;
 	while (p && *p) {
 		unsigned char c = *p;
+		// A raw ampersand or plus is legal in a URL path, but the
+		// paths written here land in attribute values, where a bare
+		// ampersand can start a character reference and quietly turn
+		// "a&copy.txt" into a different filename. Encoding both keeps
+		// the output byte-safe in every sink.
 		const char *esc = url_escape_table[c];
-		// The table is shared with the query string case, where a plus
-		// means a space and an ampersand separates parameters. Neither
-		// carries that meaning in a path.
-		if (esc && c != '+' && c != '&') {
+		if (esc) {
 			html_raw(txt, p - txt);
 			html(esc);
 			txt = p + 1;
diff --git a/source/ui-shared.c b/source/ui-shared.c
index a1538f8..49c9ef0 100644
--- a/source/ui-shared.c
+++ b/source/ui-shared.c
@@ -795,6 +795,8 @@ char *cgit_fileurl(const char *reponame, const char *pagename,
 	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.
 	if (ctx.cfg.virtual_root) {
 		strbuf_addf(&sb, "%s%s/%s/%s", ctx.cfg.virtual_root, reponame,
 			    pagename, (filename ? filename:""));
@@ -802,7 +804,7 @@ char *cgit_fileurl(const char *reponame, const char *pagename,
 	} else {
 		strbuf_addf(&sb, "?url=%s/%s/%s", reponame, pagename,
 			    (filename ? filename : ""));
-		delim = "&amp;";
+		delim = "&";
 	}
 	if (query)
 		strbuf_addf(&sb, "%s%s", delim, query);
diff --git a/source/ui-ssdiff.c b/source/ui-ssdiff.c
index 2ea7021..a58766a 100644
--- a/source/ui-ssdiff.c
+++ b/source/ui-ssdiff.c
@@ -187,7 +187,7 @@ static void print_lineno_cell(struct diff_filespec *file,
 		strbuf_add_percentencode(&path, file->path, 0);
 	fileurl = cgit_fileurl(ctx.repo->url, "tree", path.buf, query);
 	html("<td class='lineno'><a href='");
-	html(fileurl);
+	html_attr(fileurl);
 	htmlf("'>%s</a>", anchor + 1);
 	html("</td>");
 	free(fileurl);
diff --git a/tests/t0104-tree.sh b/tests/t0104-tree.sh
index c5c5c4c..f4039b6 100755
--- a/tests/t0104-tree.sh
+++ b/tests/t0104-tree.sh
@@ -3,8 +3,9 @@
 # The tree page browses a repository at one revision, either as a listing of a
 # directory or as a single file with an anchor on every line. The checks
 # against the repository named foo+bar cover a file name and a branch name
-# that both contain a plus, which cgit has to spell one way inside a path and
-# another way inside a query.
+# that both contain a plus, which cgit percent-encodes in a path as well as in
+# a query, since a bare plus in a served path would read back as a space when
+# the link is requested through a query string.
 
 test_description='Check content on tree page'
 . ./setup.sh
@@ -26,13 +27,13 @@ test_expect_success 'no line 2' '
 test_expect_success 'generate foo+bar/tree' 'cgit_url "foo%2bbar/tree" >tmp'
 
 test_expect_success 'verify a+b link' '
-	grep "/foo+bar/tree/a+b" tmp
+	grep "/foo%2Bbar/tree/a%2Bb" tmp
 '
 
 test_expect_success 'generate foo+bar/tree?h=1+2' 'cgit_url "foo%2bbar/tree&h=1%2b2" >tmp'
 
 test_expect_success 'verify a+b?h=1+2 link' '
-	grep "/foo+bar/tree/a+b?h=1%2b2" tmp
+	grep "/foo%2Bbar/tree/a%2Bb?h=1%2B2" tmp
 '
 
 test_done
author	Bryce Kwon <bryce@brycekwon.com>	2026-08-12 20:31:56 -1000
committer	Bryce Kwon <bryce@brycekwon.com>	2026-08-12 20:31:56 -1000
commit	80c877efd69192c576a2258a26265d5a70cdee04 (patch)
parent	fa00b5ee9677afff26259626ce9b1a29d4895717 (diff)
tree	a01147abb97913fa49fe476ab7a2404b1971ae29
download	cgit-80c877efd69192c576a2258a26265d5a70cdee04.tar.gz cgit-80c877efd69192c576a2258a26265d5a70cdee04.zip
-rw-r--r--	source/html.c	50
-rw-r--r--	source/ui-shared.c	4
-rw-r--r--	source/ui-ssdiff.c	2
-rwxr-xr-x	tests/t0104-tree.sh	9