[PATCH 1/3] tests: document handling of repository names with spaces

[PATCH 1/3] tests: document handling of repository names with spaces

[john]

t0103-log.sh has a test to check that we don't properly escape spaces in
repository URLs, but no other tests do.

It turns out that the handling of this case in the index page is broken,
so mark this as test_expect_failure while adding successful tests for
all other pages.

Signed-off-by: John Keeping <john at keeping.me.uk>
---
 tests/t0101-index.sh   | 1 +
 tests/t0102-summary.sh | 7 +++++++
 tests/t0104-tree.sh    | 7 +++++++
 tests/t0105-commit.sh  | 7 +++++++
 tests/t0106-diff.sh    | 7 +++++++
 5 files changed, 29 insertions(+)

diff --git a/tests/t0101-index.sh b/tests/t0101-index.sh
index 82ef9b0..6450b9b 100755
--- a/tests/t0101-index.sh
+++ b/tests/t0101-index.sh
@@ -11,6 +11,7 @@ test_expect_success 'find bar description' 'grep "the bar repo" tmp'
 test_expect_success 'find foo+bar repo' 'grep ">foo+bar<" tmp'
 test_expect_success 'verify foo+bar link' 'grep "/foo+bar/" tmp'
 test_expect_success 'verify "with%20space" link' 'grep "/with%20space/" tmp'
+test_expect_failure 'no links with space in path' '! grep "href=./with space/" tmp'
 test_expect_success 'no tree-link' '! grep "foo/tree" tmp'
 test_expect_success 'no log-link' '! grep "foo/log" tmp'
 
diff --git a/tests/t0102-summary.sh b/tests/t0102-summary.sh
index b8864cb..1d91dca 100755
--- a/tests/t0102-summary.sh
+++ b/tests/t0102-summary.sh
@@ -22,4 +22,11 @@ test_expect_success 'clone-url expanded correctly' '
 	grep "git://example.org/bar.git" tmp
 '
 
+test_expect_success 'generate with-space summary' '
+	cgit_url "with20space" >tmp
+'
+test_expect_success 'no links with space in path' '
+	! grep "href=./with space/" tmp
+'
+
 test_done
diff --git a/tests/t0104-tree.sh b/tests/t0104-tree.sh
index 2e140f5..de8c4f8 100755
--- a/tests/t0104-tree.sh
+++ b/tests/t0104-tree.sh
@@ -29,4 +29,11 @@ test_expect_success 'verify a+b?h=1+2 link' '
 	grep "/foo+bar/tree/a+b?h=1%2b2" tmp
 '
 
+test_expect_success 'generate with%20space/tree' '
+	cgit_url "with%20space/tree" >tmp
+'
+test_expect_success 'no links with space in path' '
+	! grep "href=./with space/" tmp
+'
+
 test_done
diff --git a/tests/t0105-commit.sh b/tests/t0105-commit.sh
index 9cdf55c..1932d5f 100755
--- a/tests/t0105-commit.sh
+++ b/tests/t0105-commit.sh
@@ -33,4 +33,11 @@ test_expect_success 'root commit contains diff' '
 	grep "<div class=.add.>+1</div>" tmp
 '
 
+test_expect_success 'generate with%20space/commit' '
+	cgit_url "with%20space/commit" >tmp
+'
+test_expect_success 'no links with space in path' '
+	! grep "href=./with space/" tmp
+'
+
 test_done
diff --git a/tests/t0106-diff.sh b/tests/t0106-diff.sh
index 82b645e..89532f5 100755
--- a/tests/t0106-diff.sh
+++ b/tests/t0106-diff.sh
@@ -16,4 +16,11 @@ test_expect_success 'find added line' '
 	grep "<div class=.add.>+5</div>" tmp
 '
 
+test_expect_success 'generate with%20space/diff' '
+	cgit_url "with%20space/diff" >tmp
+'
+test_expect_success 'no links with space in path' '
+	! grep "href=./with space/" tmp
+'
+
 test_done
-- 
2.12.0.415.g6c14041932.dirty

[PATCH 2/3] ui-{repolist,atom}: fix repository URL escaping

[john]

Remove cgit_repourl() and replace it with a new function that prints a
suitable escaped version of the repository URL designed for use in an
attribute value.  There are only two places that use this at the moment
and both want the value to be used in attribute context.

We could introduce new escaping functions to let us keep cgit_repourl()
and return a suitable escaped string, but there's no need for us to be
doing memory management here so this ends up simplifying the call sites.

Signed-off-by: John Keeping <john at keeping.me.uk>
---
 tests/t0101-index.sh |  2 +-
 ui-atom.c            |  4 +---
 ui-repolist.c        | 11 ++++-------
 ui-shared.c          | 13 ++++++++-----
 ui-shared.h          |  3 ++-
 5 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/tests/t0101-index.sh b/tests/t0101-index.sh
index 6450b9b..91c1355 100755
--- a/tests/t0101-index.sh
+++ b/tests/t0101-index.sh
@@ -11,7 +11,7 @@ test_expect_success 'find bar description' 'grep "the bar repo" tmp'
 test_expect_success 'find foo+bar repo' 'grep ">foo+bar<" tmp'
 test_expect_success 'verify foo+bar link' 'grep "/foo+bar/" tmp'
 test_expect_success 'verify "with%20space" link' 'grep "/with%20space/" tmp'
-test_expect_failure 'no links with space in path' '! grep "href=./with space/" tmp'
+test_expect_success 'no links with space in path' '! grep "href=./with space/" tmp'
 test_expect_success 'no tree-link' '! grep "foo/tree" tmp'
 test_expect_success 'no log-link' '! grep "foo/log" tmp'
 
diff --git a/ui-atom.c b/ui-atom.c
index 41838d3..671fc15 100644
--- a/ui-atom.c
+++ b/ui-atom.c
@@ -130,13 +130,11 @@ void cgit_print_atom(char *tip, char *path, int max_count)
 	html_txt(ctx.repo->desc);
 	html("</subtitle>\n");
 	if (host) {
-		char *repourl = cgit_repourl(ctx.repo->url);
 		html("<link rel='alternate' type='text/html' href='");
 		html(cgit_httpscheme());
 		html_attr(host);
-		html_attr(repourl);
+		cgit_repo_url_path(ctx.repo->url);
 		html("'/>\n");
-		free(repourl);
 	}
 	while ((commit = get_revision(&rev)) != NULL) {
 		add_entry(commit, host);
diff --git a/ui-repolist.c b/ui-repolist.c
index b57ea60..e1302b4 100644
--- a/ui-repolist.c
+++ b/ui-repolist.c
@@ -275,7 +275,6 @@ void cgit_print_repolist(void)
 	int i, columns = 3, hits = 0, header = 0;
 	char *last_section = NULL;
 	char *section;
-	char *repourl;
 	int sorted = 0;
 
 	if (!any_repos_visible()) {
@@ -330,13 +329,11 @@ void cgit_print_repolist(void)
 		htmlf("<tr><td class='%s'>",
 		      !sorted && section ? "sublevel-repo" : "toplevel-repo");
 		cgit_summary_link(ctx.repo->name, ctx.repo->name, NULL, NULL);
-		html("</td><td>");
-		repourl = cgit_repourl(ctx.repo->url);
-		html_link_open(repourl, NULL, NULL);
-		free(repourl);
+		html("</td><td><a href='");
+		cgit_repo_url_path(ctx.repo->url);
+		html("'>");
 		html_ntxt(ctx.cfg.max_repodesc_len, ctx.repo->desc);
-		html_link_close();
-		html("</td><td>");
+		html("</a></td><td>");
 		if (ctx.cfg.enable_index_owner) {
 			if (ctx.repo->owner_filter) {
 				cgit_open_filter(ctx.repo->owner_filter);
diff --git a/ui-shared.c b/ui-shared.c
index 2e4fcd9..4aca983 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -92,12 +92,15 @@ const char *cgit_loginurl(void)
 	return login_url;
 }
 
-char *cgit_repourl(const char *reponame)
+void cgit_repo_url_path(const char *reponame)
 {
-	if (ctx.cfg.virtual_root)
-		return fmtalloc("%s%s/", ctx.cfg.virtual_root, reponame);
-	else
-		return fmtalloc("?r=%s", reponame);
+	if (ctx.cfg.virtual_root) {
+		html_url_path(ctx.cfg.virtual_root);
+		html_url_path(reponame);
+	} else {
+		html("?r=");
+		html_url_arg(reponame);
+	}
 }
 
 char *cgit_fileurl(const char *reponame, const char *pagename,
diff --git a/ui-shared.h b/ui-shared.h
index 87799f1..90dfcbc 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -6,7 +6,6 @@ extern char *cgit_hosturl(void);
 extern const char *cgit_rooturl(void);
 extern char *cgit_currenturl(void);
 extern const char *cgit_loginurl(void);
-extern char *cgit_repourl(const char *reponame);
 extern char *cgit_fileurl(const char *reponame, const char *pagename,
 			  const char *filename, const char *query);
 extern char *cgit_pageurl(const char *reponame, const char *pagename,
@@ -14,6 +13,8 @@ extern char *cgit_pageurl(const char *reponame, const char *pagename,
 
 extern void cgit_add_clone_urls(void (*fn)(const char *));
 
+extern void cgit_repo_url_path(const char *reponame);
+
 extern void cgit_index_link(const char *name, const char *title,
 			    const char *class, const char *pattern, const char *sort, int ofs, int always_root);
 extern void cgit_summary_link(const char *name, const char *title,
-- 
2.12.0.415.g6c14041932.dirty

[PATCH 3/3] html: remove unused functions

[john]

The previous commit removed the only use of these functions, so let's
remove them.

Signed-off-by: John Keeping <john at keeping.me.uk>
---
 html.c | 20 --------------------
 html.h |  2 --
 2 files changed, 22 deletions(-)

diff --git a/html.c b/html.c
index e7e6e07..e4dbb05 100644
--- a/html.c
+++ b/html.c
@@ -295,26 +295,6 @@ void html_intoption(int value, const char *text, int selected_value)
 	html("</option>");
 }
 
-void html_link_open(const char *url, const char *title, const char *class)
-{
-	html("<a href='");
-	html_attr(url);
-	if (title) {
-		html("' title='");
-		html_attr(title);
-	}
-	if (class) {
-		html("' class='");
-		html_attr(class);
-	}
-	html("'>");
-}
-
-void html_link_close(void)
-{
-	html("</a>");
-}
-
 void html_fileperm(unsigned short mode)
 {
 	htmlf("%c%c%c", (mode & 4 ? 'r' : '-'),
diff --git a/html.h b/html.h
index 1b24e55..e0f5195 100644
--- a/html.h
+++ b/html.h
@@ -27,8 +27,6 @@ extern void html_header_arg_in_quotes(const char *txt);
 extern void html_hidden(const char *name, const char *value);
 extern void html_option(const char *value, const char *text, const char *selected_value);
 extern void html_intoption(int value, const char *text, int selected_value);
-extern void html_link_open(const char *url, const char *title, const char *class);
-extern void html_link_close(void);
 extern void html_fileperm(unsigned short mode);
 extern int html_include(const char *filename);
 
-- 
2.12.0.415.g6c14041932.dirty