Drop the query-string form of every link

cgit falls back to SCRIPT_NAME for virtual_root whenever cgitrc leaves
it out, so the ?url= and ?r= forms only ever reached an error page
raised while cgitrc was still being read. The page argument of
cgit_add_hidden_formfields fed only the url field, so it is unused
until its callers stop passing it.

2 files changed, 31 insertions, 86 deletions

diff --git a/source/ui-shared.c b/source/ui-shared.cindex 506cfe2..0f1f27a 100644--- a/source/ui-shared.c+++ b/source/ui-shared.c@@ -1,11 +1,10 @@ /*  * The output every cgit page has in common. A link from one page to another is- * built here because only this file knows whether a site addresses a request- * as a path below a virtual root or as a query string, and the two are spelled- * differently down to the separator between arguments. The headers, the- * document head, the header block, the footer, the error page and the age- * column are here so that a page handler is left with only its own body to- * produce.+ * built here, as a path below the virtual root followed by the arguments the+ * target page needs, so that every page spells its urls the same way. The+ * headers, the document head, the header block, the footer, the error page and+ * the age column are here so that a page handler is left with only its own body+ * to produce.  */  #define USE_THE_REPOSITORY_VARIABLE@@ -35,20 +34,12 @@ // into a tab title while it still leaves room to spare in that buffer. #define MAX_TITLE_PATH_LEN 256 -static const char *root_url(void)-{-	if (ctx.cfg.virtual_root)-		return ctx.cfg.virtual_root;-	else-		return ctx.cfg.script_name;-}- static void site_url(const char *page, const char *search, const char *sort, int ofs, int always_root) { 	const char *delim = "?";  	if (always_root || page)-		html_attr(root_url());+		html_attr(ctx.cfg.virtual_root); 	else { 		char *currenturl = cgit_currenturl(); 		html_attr(currenturl);@@ -111,36 +102,20 @@ static void site_link(const char *page, const char *name, const char *title, }  /*- * Write the url naming the repository, the page and the path, and return- * whether the url has opened its query string already.+ * Write the url naming the repository, the page and the path.  */-static int write_page_url(const char *page, const char *path)-{-	if (ctx.cfg.virtual_root) {-		html_url_path(ctx.cfg.virtual_root);-		html_url_path(ctx.repo->url);-		if (ctx.repo->url[strlen(ctx.repo->url) - 1] != '/')-			html("/");-		if (page) {-			html_url_path(page);-			html("/");-			if (path)-				html_url_path(path);-		}-		return 0;-	}-	html_url_path(ctx.cfg.script_name);-	html("?url=");-	html_url_arg(ctx.repo->url);+static void write_page_url(const char *page, const char *path)+{+	html_url_path(ctx.cfg.virtual_root);+	html_url_path(ctx.repo->url); 	if (ctx.repo->url[strlen(ctx.repo->url) - 1] != '/') 		html("/"); 	if (page) {-		html_url_arg(page);+		html_url_path(page); 		html("/"); 		if (path)-			html_url_arg(path);+			html_url_path(path); 	}-	return 1; }  /*@@ -160,7 +135,7 @@ static int head_is_named(const char *head) static const char *repolink(const char *title, const char *class, const char *page, 	const char *head, const char *path) {-	const char *delim;+	const char *delim = "?";  	html("<a"); 	if (title) {@@ -170,7 +145,7 @@ static const char *repolink(const char *title, const char *class, const char *pa 	} 	link_class(class); 	html(" href='");-	delim = write_page_url(page, path) ? "&amp;" : "?";+	write_page_url(page, path); 	if (head_is_named(head)) { 		html(delim); 		html("h=");@@ -492,7 +467,7 @@ static void print_header(void) 		if (logo_link && *logo_link) 			html_attr(logo_link); 		else-			html_attr(root_url());+			html_attr(ctx.cfg.virtual_root); 		html("'><img src='"); 		html_attr(logo); 		html("' alt='cgit logo'></a>\n");@@ -628,8 +603,7 @@ 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)-		write_page_url("log", ctx.qry.vpath);+	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");@@ -804,7 +778,7 @@ char *cgit_hosturl(void)  char *cgit_currenturl(void) {-	const char *root = root_url();+	const char *root = ctx.cfg.virtual_root;  	if (!ctx.qry.url) 		return xstrdup(root);@@ -845,7 +819,7 @@ char *cgit_currentquery(void)  char *cgit_currentfullurl(void) {-	const char *root = root_url();+	const char *root = ctx.cfg.virtual_root; 	char *query = cgit_currentquery(); 	char *ret; @@ -864,7 +838,7 @@ const char *cgit_loginurl(void) 	static const char *login_url;  	if (!login_url)-		login_url = cgit_fmtalloc("%s?p=login", root_url());+		login_url = cgit_fmtalloc("%s?p=login", ctx.cfg.virtual_root); 	return login_url; } @@ -872,14 +846,9 @@ char *cgit_repourl(const char *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);-	}+	strbuf_addstr(&sb, ctx.cfg.virtual_root);+	cgit_add_url_path(&sb, reponame);+	strbuf_addch(&sb, '/'); 	return strbuf_detach(&sb, NULL); } @@ -887,21 +856,12 @@ char *cgit_fileurl(const char *reponame, const char *pagename, const char *filen 	const char *query) { 	struct strbuf sb = STRBUF_INIT;-	const char *delim; -	// The query joiner is a bare ampersand, because each sink escapes the-	// whole string for wherever it lands.-	if (ctx.cfg.virtual_root) {-		strbuf_addstr(&sb, ctx.cfg.virtual_root);-		delim = "?";-	} else {-		strbuf_addstr(&sb, "?url=");-		delim = "&";-	}+	strbuf_addstr(&sb, ctx.cfg.virtual_root); 	cgit_add_url_path(&sb, reponame); 	strbuf_addf(&sb, "/%s/%s", pagename, filename ? filename : ""); 	if (query)-		strbuf_addf(&sb, "%s%s", delim, query);+		strbuf_addf(&sb, "?%s", query); 	return strbuf_detach(&sb, NULL); } @@ -912,16 +872,13 @@ char *cgit_pageurl(const char *reponame, const char *pagename, const char *query  void cgit_tree_url(const char *head, struct strbuf *base, struct strbuf *query) {-	int query_open;- 	html_capture_begin(base);-	query_open = write_page_url("tree", NULL);+	write_page_url("tree", NULL); 	html_capture_end(); 	if (!head_is_named(head)) 		return; 	html_capture_begin(query);-	html(query_open ? "&" : "?");-	html("h=");+	html("?h="); 	html_url_arg(head); 	html_capture_end(); }@@ -1568,8 +1525,8 @@ void cgit_print_docstart(void) 		html("<link rel='alternate' title='Atom feed' href='"); 		html(cgit_httpscheme()); 		html_attr(host);-		html(write_page_url("atom", ctx.qry.vpath) ? "&amp;" : "?");-		html("h=");+		write_page_url("atom", ctx.qry.vpath);+		html("?h="); 		html_url_arg(ctx.qry.head); 		html("' type='application/atom+xml'>\n"); 		free(host);@@ -1721,18 +1678,8 @@ void cgit_print_clone_row(const char *url, int colspan) 	free(fallback); } -void cgit_add_hidden_formfields(int incl_head, int incl_search, const char *page)+void cgit_add_hidden_formfields(int incl_head, int incl_search, const char *page UNUSED) {-	if (!ctx.cfg.virtual_root) {-		struct strbuf url = STRBUF_INIT;--		strbuf_addf(&url, "%s/%s", ctx.qry.repo, page);-		if (ctx.qry.vpath)-			strbuf_addf(&url, "/%s", ctx.qry.vpath);-		html_hidden("url", url.buf);-		strbuf_release(&url);-	}- 	if (incl_head && ctx.qry.head && ctx.repo->defbranch && strcmp(ctx.qry.head, ctx.repo->defbranch)) 		html_hidden("h", ctx.qry.head); 
diff --git a/source/ui-shared.c b/source/ui-shared.c
index 506cfe2..0f1f27a 100644
--- a/source/ui-shared.c
+++ b/source/ui-shared.c
@@ -1,11 +1,10 @@
 /*
  * The output every cgit page has in common. A link from one page to another is
- * built here because only this file knows whether a site addresses a request
- * as a path below a virtual root or as a query string, and the two are spelled
- * differently down to the separator between arguments. The headers, the
- * document head, the header block, the footer, the error page and the age
- * column are here so that a page handler is left with only its own body to
- * produce.
+ * built here, as a path below the virtual root followed by the arguments the
+ * target page needs, so that every page spells its urls the same way. The
+ * headers, the document head, the header block, the footer, the error page and
+ * the age column are here so that a page handler is left with only its own body
+ * to produce.
  */
 
 #define USE_THE_REPOSITORY_VARIABLE
@@ -35,20 +34,12 @@
 // into a tab title while it still leaves room to spare in that buffer.
 #define MAX_TITLE_PATH_LEN 256
 
-static const char *root_url(void)
-{
-	if (ctx.cfg.virtual_root)
-		return ctx.cfg.virtual_root;
-	else
-		return ctx.cfg.script_name;
-}
-
 static void site_url(const char *page, const char *search, const char *sort, int ofs, int always_root)
 {
 	const char *delim = "?";
 
 	if (always_root || page)
-		html_attr(root_url());
+		html_attr(ctx.cfg.virtual_root);
 	else {
 		char *currenturl = cgit_currenturl();
 		html_attr(currenturl);
@@ -111,36 +102,20 @@ static void site_link(const char *page, const char *name, const char *title,
 }
 
 /*
- * Write the url naming the repository, the page and the path, and return
- * whether the url has opened its query string already.
+ * Write the url naming the repository, the page and the path.
  */
-static int write_page_url(const char *page, const char *path)
-{
-	if (ctx.cfg.virtual_root) {
-		html_url_path(ctx.cfg.virtual_root);
-		html_url_path(ctx.repo->url);
-		if (ctx.repo->url[strlen(ctx.repo->url) - 1] != '/')
-			html("/");
-		if (page) {
-			html_url_path(page);
-			html("/");
-			if (path)
-				html_url_path(path);
-		}
-		return 0;
-	}
-	html_url_path(ctx.cfg.script_name);
-	html("?url=");
-	html_url_arg(ctx.repo->url);
+static void write_page_url(const char *page, const char *path)
+{
+	html_url_path(ctx.cfg.virtual_root);
+	html_url_path(ctx.repo->url);
 	if (ctx.repo->url[strlen(ctx.repo->url) - 1] != '/')
 		html("/");
 	if (page) {
-		html_url_arg(page);
+		html_url_path(page);
 		html("/");
 		if (path)
-			html_url_arg(path);
+			html_url_path(path);
 	}
-	return 1;
 }
 
 /*
@@ -160,7 +135,7 @@ static int head_is_named(const char *head)
 static const char *repolink(const char *title, const char *class, const char *page,
 	const char *head, const char *path)
 {
-	const char *delim;
+	const char *delim = "?";
 
 	html("<a");
 	if (title) {
@@ -170,7 +145,7 @@ static const char *repolink(const char *title, const char *class, const char *pa
 	}
 	link_class(class);
 	html(" href='");
-	delim = write_page_url(page, path) ? "&amp;" : "?";
+	write_page_url(page, path);
 	if (head_is_named(head)) {
 		html(delim);
 		html("h=");
@@ -492,7 +467,7 @@ static void print_header(void)
 		if (logo_link && *logo_link)
 			html_attr(logo_link);
 		else
-			html_attr(root_url());
+			html_attr(ctx.cfg.virtual_root);
 		html("'><img src='");
 		html_attr(logo);
 		html("' alt='cgit logo'></a>\n");
@@ -628,8 +603,7 @@ 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)
-		write_page_url("log", ctx.qry.vpath);
+	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");
@@ -804,7 +778,7 @@ char *cgit_hosturl(void)
 
 char *cgit_currenturl(void)
 {
-	const char *root = root_url();
+	const char *root = ctx.cfg.virtual_root;
 
 	if (!ctx.qry.url)
 		return xstrdup(root);
@@ -845,7 +819,7 @@ char *cgit_currentquery(void)
 
 char *cgit_currentfullurl(void)
 {
-	const char *root = root_url();
+	const char *root = ctx.cfg.virtual_root;
 	char *query = cgit_currentquery();
 	char *ret;
 
@@ -864,7 +838,7 @@ const char *cgit_loginurl(void)
 	static const char *login_url;
 
 	if (!login_url)
-		login_url = cgit_fmtalloc("%s?p=login", root_url());
+		login_url = cgit_fmtalloc("%s?p=login", ctx.cfg.virtual_root);
 	return login_url;
 }
 
@@ -872,14 +846,9 @@ char *cgit_repourl(const char *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);
-	}
+	strbuf_addstr(&sb, ctx.cfg.virtual_root);
+	cgit_add_url_path(&sb, reponame);
+	strbuf_addch(&sb, '/');
 	return strbuf_detach(&sb, NULL);
 }
 
@@ -887,21 +856,12 @@ char *cgit_fileurl(const char *reponame, const char *pagename, const char *filen
 	const char *query)
 {
 	struct strbuf sb = STRBUF_INIT;
-	const char *delim;
 
-	// The query joiner is a bare ampersand, because each sink escapes the
-	// whole string for wherever it lands.
-	if (ctx.cfg.virtual_root) {
-		strbuf_addstr(&sb, ctx.cfg.virtual_root);
-		delim = "?";
-	} else {
-		strbuf_addstr(&sb, "?url=");
-		delim = "&";
-	}
+	strbuf_addstr(&sb, ctx.cfg.virtual_root);
 	cgit_add_url_path(&sb, reponame);
 	strbuf_addf(&sb, "/%s/%s", pagename, filename ? filename : "");
 	if (query)
-		strbuf_addf(&sb, "%s%s", delim, query);
+		strbuf_addf(&sb, "?%s", query);
 	return strbuf_detach(&sb, NULL);
 }
 
@@ -912,16 +872,13 @@ char *cgit_pageurl(const char *reponame, const char *pagename, const char *query
 
 void cgit_tree_url(const char *head, struct strbuf *base, struct strbuf *query)
 {
-	int query_open;
-
 	html_capture_begin(base);
-	query_open = write_page_url("tree", NULL);
+	write_page_url("tree", NULL);
 	html_capture_end();
 	if (!head_is_named(head))
 		return;
 	html_capture_begin(query);
-	html(query_open ? "&" : "?");
-	html("h=");
+	html("?h=");
 	html_url_arg(head);
 	html_capture_end();
 }
@@ -1568,8 +1525,8 @@ void cgit_print_docstart(void)
 		html("<link rel='alternate' title='Atom feed' href='");
 		html(cgit_httpscheme());
 		html_attr(host);
-		html(write_page_url("atom", ctx.qry.vpath) ? "&amp;" : "?");
-		html("h=");
+		write_page_url("atom", ctx.qry.vpath);
+		html("?h=");
 		html_url_arg(ctx.qry.head);
 		html("' type='application/atom+xml'>\n");
 		free(host);
@@ -1721,18 +1678,8 @@ void cgit_print_clone_row(const char *url, int colspan)
 	free(fallback);
 }
 
-void cgit_add_hidden_formfields(int incl_head, int incl_search, const char *page)
+void cgit_add_hidden_formfields(int incl_head, int incl_search, const char *page UNUSED)
 {
-	if (!ctx.cfg.virtual_root) {
-		struct strbuf url = STRBUF_INIT;
-
-		strbuf_addf(&url, "%s/%s", ctx.qry.repo, page);
-		if (ctx.qry.vpath)
-			strbuf_addf(&url, "/%s", ctx.qry.vpath);
-		html_hidden("url", url.buf);
-		strbuf_release(&url);
-	}
-
 	if (incl_head && ctx.qry.head && ctx.repo->defbranch && strcmp(ctx.qry.head, ctx.repo->defbranch))
 		html_hidden("h", ctx.qry.head);
 
diff --git a/source/ui-shared.h b/source/ui-shared.hindex 9abd4c0..59751ae 100644--- a/source/ui-shared.h+++ b/source/ui-shared.h@@ -3,9 +3,7 @@  * handler renders its own body and calls in here for all that surrounds it,  * which is the HTTP headers, the document head and footer, the header block  * with its tabs and search form, the error page, and an anchor to any other- * cgit page. Whether a site addresses a request as a path below a virtual root- * or as a query string is settled behind these calls, so that nothing else has- * to know which of the two it is looking at.+ * cgit page.  */  #ifndef CGIT_UI_SHARED_H
diff --git a/source/ui-shared.h b/source/ui-shared.h
index 9abd4c0..59751ae 100644
--- a/source/ui-shared.h
+++ b/source/ui-shared.h
@@ -3,9 +3,7 @@
  * handler renders its own body and calls in here for all that surrounds it,
  * which is the HTTP headers, the document head and footer, the header block
  * with its tabs and search form, the error page, and an anchor to any other
- * cgit page. Whether a site addresses a request as a path below a virtual root
- * or as a query string is settled behind these calls, so that nothing else has
- * to know which of the two it is looking at.
+ * cgit page.
  */
 
 #ifndef CGIT_UI_SHARED_H