6 files changed, 44 insertions, 135 deletions

diff --git a/MANUAL.txt b/MANUAL.txtindex a06ef7f..9f6c6f1 100644--- a/MANUAL.txt+++ b/MANUAL.txt@@ -705,10 +705,7 @@ repo.readme:: 	or by hash may be prepended, followed by a colon, as in 	"master:docs/readme.mkd". If the value begins with a colon, as in 	":docs/readme.rst", the head given in the query or the default branch of-	the repository is used. Sharing any file exposes that entire directory-	tree to the "/about/PATH" endpoints, so be sure that there are no-	non-public files located in the same directory as the readme file.-	Default value: <readme>.+	the repository is used. Default value: <readme>.  repo.section:: 	Override the current section name for this repository. Default value:
diff --git a/MANUAL.txt b/MANUAL.txt
index a06ef7f..9f6c6f1 100644
--- a/MANUAL.txt
+++ b/MANUAL.txt
@@ -705,10 +705,7 @@ repo.readme::
 	or by hash may be prepended, followed by a colon, as in
 	"master:docs/readme.mkd". If the value begins with a colon, as in
 	":docs/readme.rst", the head given in the query or the default branch of
-	the repository is used. Sharing any file exposes that entire directory
-	tree to the "/about/PATH" endpoints, so be sure that there are no
-	non-public files located in the same directory as the readme file.
-	Default value: <readme>.
+	the repository is used. Default value: <readme>.
 
 repo.section::
 	Override the current section name for this repository. Default value:
diff --git a/source/cmd.c b/source/cmd.cindex 49c1eb4..08de123 100644--- a/source/cmd.c+++ b/source/cmd.c@@ -34,39 +34,28 @@ static void head_fn(void)  static void about_fn(void) {-	char *currenturl, *query, *redirect;-	size_t path_info_len;+	char *query, *summary; +	// The page shows the readme alone, and the readme's links lead into+	// the tree, so nothing lives below it.+	if (ctx.qry.path) {+		cgit_print_error_page(404, "Not Found", "Not found");+		return;+	} 	if (!ctx.repo) { 		cgit_print_site_readme(); 		return; 	}--	// The about page resolves relative links against its own URL, so it-	// only works with a trailing slash. The branch and the rest of the-	// query travel along with either redirect.-	path_info_len = ctx.env.path_info ? strlen(ctx.env.path_info) : 0;-	if (!ctx.qry.path &&-		(!ctx.qry.url || !*ctx.qry.url || ctx.qry.url[strlen(ctx.qry.url) - 1] != '/') &&-		(!path_info_len || ctx.env.path_info[path_info_len - 1] != '/')) {-		currenturl = cgit_currenturl();-		query = cgit_currentquery();-		redirect = cgit_fmtalloc("%s/", currenturl);-		cgit_redirect(redirect, query, true);-		free(currenturl);-		free(query);-		free(redirect);-	} else if (ctx.repo->readme.nr) {-		cgit_print_repo_readme(ctx.qry.path);-	} else {-		currenturl = cgit_currenturl();-		query = cgit_currentquery();-		redirect = cgit_fmtalloc("%s../", currenturl);-		cgit_redirect(redirect, query, false);-		free(currenturl);-		free(query);-		free(redirect);+	if (ctx.repo->readme.nr) {+		cgit_print_repo_readme();+		return; 	}+	// The branch and the rest of the query travel along to the summary.+	query = cgit_currentquery();+	summary = cgit_repourl(ctx.repo->url);+	cgit_redirect(summary, query, false);+	free(query);+	free(summary); }  static void atom_fn(void)
diff --git a/source/cmd.c b/source/cmd.c
index 49c1eb4..08de123 100644
--- a/source/cmd.c
+++ b/source/cmd.c
@@ -34,39 +34,28 @@ static void head_fn(void)
 
 static void about_fn(void)
 {
-	char *currenturl, *query, *redirect;
-	size_t path_info_len;
+	char *query, *summary;
 
+	// The page shows the readme alone, and the readme's links lead into
+	// the tree, so nothing lives below it.
+	if (ctx.qry.path) {
+		cgit_print_error_page(404, "Not Found", "Not found");
+		return;
+	}
 	if (!ctx.repo) {
 		cgit_print_site_readme();
 		return;
 	}
-
-	// The about page resolves relative links against its own URL, so it
-	// only works with a trailing slash. The branch and the rest of the
-	// query travel along with either redirect.
-	path_info_len = ctx.env.path_info ? strlen(ctx.env.path_info) : 0;
-	if (!ctx.qry.path &&
-		(!ctx.qry.url || !*ctx.qry.url || ctx.qry.url[strlen(ctx.qry.url) - 1] != '/') &&
-		(!path_info_len || ctx.env.path_info[path_info_len - 1] != '/')) {
-		currenturl = cgit_currenturl();
-		query = cgit_currentquery();
-		redirect = cgit_fmtalloc("%s/", currenturl);
-		cgit_redirect(redirect, query, true);
-		free(currenturl);
-		free(query);
-		free(redirect);
-	} else if (ctx.repo->readme.nr) {
-		cgit_print_repo_readme(ctx.qry.path);
-	} else {
-		currenturl = cgit_currenturl();
-		query = cgit_currentquery();
-		redirect = cgit_fmtalloc("%s../", currenturl);
-		cgit_redirect(redirect, query, false);
-		free(currenturl);
-		free(query);
-		free(redirect);
+	if (ctx.repo->readme.nr) {
+		cgit_print_repo_readme();
+		return;
 	}
+	// The branch and the rest of the query travel along to the summary.
+	query = cgit_currentquery();
+	summary = cgit_repourl(ctx.repo->url);
+	cgit_redirect(summary, query, false);
+	free(query);
+	free(summary);
 }
 
 static void atom_fn(void)
diff --git a/source/ui-summary.c b/source/ui-summary.cindex 61cd6ac..e7b229a 100644--- a/source/ui-summary.c+++ b/source/ui-summary.c@@ -4,8 +4,7 @@  * with the clone urls, reusing the listings ui-refs.c and ui-log.c draw  * elsewhere. The about page renders the readme that config parsing picked for  * the repository, either through the configured about filter or escaped as- * plain text, and it can serve a file sitting beside that readme so links- * inside the readme resolve.+ * plain text.  */  #include "cgit.h"@@ -14,7 +13,6 @@ #include "shared.h" #include "ui-blob.h" #include "ui-log.h"-#include "ui-plain.h" #include "ui-refs.h" #include "ui-shared.h" #include "ui-summary.h"@@ -52,66 +50,6 @@ static void print_clone_url(const char *url) 	cgit_print_clone_row(url, columns); } -/*- * Without the separator boundary a sibling directory that merely shares the- * base as a name prefix, such as repo.git-backup beside repo.git, would pass.- */-static int path_within(const char *base, const char *path)-{-	size_t len = strlen(base);--	return starts_with(path, base) && (path[len] == '\0' || path[len] == '/');-}--/*- * Returns a path the caller must free, or NULL when the request cannot be- * served. A null ref means the readme is a file on the server's disk, and- * such a readme is confined to its own directory, so one named without a- * directory is refused.- */-static char *resolve_about_path(const char *filename, const char *ref, const char *path)-{-	char *copy, *base_dir, *full_path;-	char *resolved_base = NULL, *resolved_full = NULL, *resolved_repo = NULL;--	// dirname is allowed to write into its argument and to return a-	// pointer into it, so base_dir borrows from a copy freed at the end.-	copy = xstrdup(filename);-	base_dir = dirname(copy);-	if (!strcmp(base_dir, ".") || !strcmp(base_dir, "..")) {-		if (!ref) {-			free(copy);-			return NULL;-		}-		full_path = xstrdup(path);-	} else {-		full_path = cgit_fmtalloc("%s/%s", base_dir, path);-	}--	// A readme in the repository directory, or above it, would open the-	// repository's own files, its config and hooks among them.-	if (!ref) {-		resolved_base = realpath(base_dir, NULL);-		resolved_full = realpath(full_path, NULL);-		resolved_repo = realpath(ctx.repo->path, NULL);-		if (-			!resolved_base || !resolved_full || !resolved_repo ||-			path_within(resolved_base, resolved_repo)          ||-			!path_within(resolved_base, resolved_full)-		) {-			free(full_path);-			full_path = NULL;-		}-	}--	free(copy);-	free(resolved_base);-	free(resolved_full);-	free(resolved_repo);--	return full_path;-}- void cgit_print_summary(void) { 	cgit_print_layout_start();@@ -130,34 +68,14 @@ void cgit_print_summary(void) 	cgit_print_layout_end(); } -void cgit_print_repo_readme(const char *path)+void cgit_print_repo_readme(void) {-	char *filename, *ref, *mimetype;-	int free_filename = 0;--	mimetype = cgit_get_mimetype_for_filename(path);-	if (mimetype && (starts_with(mimetype, "image/") || starts_with(mimetype, "video/"))) {-		ctx.page.mimetype = mimetype;-		cgit_print_plain();-		free(mimetype);-		return;-	}-	free(mimetype);+	char *filename, *ref;  	cgit_print_layout_start();-	if (ctx.repo->readme.nr == 0)-		goto done;- 	filename = ctx.repo->readme.items[0].string; 	ref = ctx.repo->readme.items[0].util; -	if (path) {-		free_filename = 1;-		filename = resolve_about_path(filename, ref, path);-		if (!filename)-			goto done;-	}- 	html("<div id='about'>"); 	if (!ctx.repo->about_filter) { 		// With no about-filter there is nothing to turn the readme@@ -182,11 +100,6 @@ void cgit_print_repo_readme(const char *path) 			html_include(filename); 		cgit_close_filter(ctx.repo->about_filter); 	}- 	html("</div>");-	if (free_filename)-		free(filename);--done: 	cgit_print_layout_end(); }
diff --git a/source/ui-summary.c b/source/ui-summary.c
index 61cd6ac..e7b229a 100644
--- a/source/ui-summary.c
+++ b/source/ui-summary.c
@@ -4,8 +4,7 @@
  * with the clone urls, reusing the listings ui-refs.c and ui-log.c draw
  * elsewhere. The about page renders the readme that config parsing picked for
  * the repository, either through the configured about filter or escaped as
- * plain text, and it can serve a file sitting beside that readme so links
- * inside the readme resolve.
+ * plain text.
  */
 
 #include "cgit.h"
@@ -14,7 +13,6 @@
 #include "shared.h"
 #include "ui-blob.h"
 #include "ui-log.h"
-#include "ui-plain.h"
 #include "ui-refs.h"
 #include "ui-shared.h"
 #include "ui-summary.h"
@@ -52,66 +50,6 @@ static void print_clone_url(const char *url)
 	cgit_print_clone_row(url, columns);
 }
 
-/*
- * Without the separator boundary a sibling directory that merely shares the
- * base as a name prefix, such as repo.git-backup beside repo.git, would pass.
- */
-static int path_within(const char *base, const char *path)
-{
-	size_t len = strlen(base);
-
-	return starts_with(path, base) && (path[len] == '\0' || path[len] == '/');
-}
-
-/*
- * Returns a path the caller must free, or NULL when the request cannot be
- * served. A null ref means the readme is a file on the server's disk, and
- * such a readme is confined to its own directory, so one named without a
- * directory is refused.
- */
-static char *resolve_about_path(const char *filename, const char *ref, const char *path)
-{
-	char *copy, *base_dir, *full_path;
-	char *resolved_base = NULL, *resolved_full = NULL, *resolved_repo = NULL;
-
-	// dirname is allowed to write into its argument and to return a
-	// pointer into it, so base_dir borrows from a copy freed at the end.
-	copy = xstrdup(filename);
-	base_dir = dirname(copy);
-	if (!strcmp(base_dir, ".") || !strcmp(base_dir, "..")) {
-		if (!ref) {
-			free(copy);
-			return NULL;
-		}
-		full_path = xstrdup(path);
-	} else {
-		full_path = cgit_fmtalloc("%s/%s", base_dir, path);
-	}
-
-	// A readme in the repository directory, or above it, would open the
-	// repository's own files, its config and hooks among them.
-	if (!ref) {
-		resolved_base = realpath(base_dir, NULL);
-		resolved_full = realpath(full_path, NULL);
-		resolved_repo = realpath(ctx.repo->path, NULL);
-		if (
-			!resolved_base || !resolved_full || !resolved_repo ||
-			path_within(resolved_base, resolved_repo)          ||
-			!path_within(resolved_base, resolved_full)
-		) {
-			free(full_path);
-			full_path = NULL;
-		}
-	}
-
-	free(copy);
-	free(resolved_base);
-	free(resolved_full);
-	free(resolved_repo);
-
-	return full_path;
-}
-
 void cgit_print_summary(void)
 {
 	cgit_print_layout_start();
@@ -130,34 +68,14 @@ void cgit_print_summary(void)
 	cgit_print_layout_end();
 }
 
-void cgit_print_repo_readme(const char *path)
+void cgit_print_repo_readme(void)
 {
-	char *filename, *ref, *mimetype;
-	int free_filename = 0;
-
-	mimetype = cgit_get_mimetype_for_filename(path);
-	if (mimetype && (starts_with(mimetype, "image/") || starts_with(mimetype, "video/"))) {
-		ctx.page.mimetype = mimetype;
-		cgit_print_plain();
-		free(mimetype);
-		return;
-	}
-	free(mimetype);
+	char *filename, *ref;
 
 	cgit_print_layout_start();
-	if (ctx.repo->readme.nr == 0)
-		goto done;
-
 	filename = ctx.repo->readme.items[0].string;
 	ref = ctx.repo->readme.items[0].util;
 
-	if (path) {
-		free_filename = 1;
-		filename = resolve_about_path(filename, ref, path);
-		if (!filename)
-			goto done;
-	}
-
 	html("<div id='about'>");
 	if (!ctx.repo->about_filter) {
 		// With no about-filter there is nothing to turn the readme
@@ -182,11 +100,6 @@ void cgit_print_repo_readme(const char *path)
 			html_include(filename);
 		cgit_close_filter(ctx.repo->about_filter);
 	}
-
 	html("</div>");
-	if (free_filename)
-		free(filename);
-
-done:
 	cgit_print_layout_end();
 }
diff --git a/source/ui-summary.h b/source/ui-summary.hindex 0f61da5..ecb1a5c 100644--- a/source/ui-summary.h+++ b/source/ui-summary.h@@ -1,14 +1,13 @@ /*  * The summary and about pages, the two a repository opens with, one listing  * its refs and recent commits and the other built from its readme. cmd.c- * reaches both, and it hands the about page whatever path follows the page- * name in the url, so a readme can link to a file beside it.+ * reaches both.  */  #ifndef CGIT_UI_SUMMARY_H #define CGIT_UI_SUMMARY_H  extern void cgit_print_summary(void);-extern void cgit_print_repo_readme(const char *path);+extern void cgit_print_repo_readme(void);  #endif // CGIT_UI_SUMMARY_H
diff --git a/source/ui-summary.h b/source/ui-summary.h
index 0f61da5..ecb1a5c 100644
--- a/source/ui-summary.h
+++ b/source/ui-summary.h
@@ -1,14 +1,13 @@
 /*
  * The summary and about pages, the two a repository opens with, one listing
  * its refs and recent commits and the other built from its readme. cmd.c
- * reaches both, and it hands the about page whatever path follows the page
- * name in the url, so a readme can link to a file beside it.
+ * reaches both.
  */
 
 #ifndef CGIT_UI_SUMMARY_H
 #define CGIT_UI_SUMMARY_H
 
 extern void cgit_print_summary(void);
-extern void cgit_print_repo_readme(const char *path);
+extern void cgit_print_repo_readme(void);
 
 #endif // CGIT_UI_SUMMARY_H
diff --git a/tests/t0301-security.sh b/tests/t0301-security.shindex 1a5ee44..ce2b5da 100755--- a/tests/t0301-security.sh+++ b/tests/t0301-security.sh@@ -445,7 +445,7 @@ test_expect_success 'the dumb transport refuses a path that climbs out' '  test_expect_success 'the about page serves nothing above the readme directory' ' 	cveq "url=cve/about/../cverc" >tmp &&-	grep "^Status: 200" tmp &&+	grep "^Status: 404" tmp && 	! grep "virtual-root" tmp && 	cveq "url=cve/about/" >tmp && 	grep "readme text" tmp
diff --git a/tests/t0301-security.sh b/tests/t0301-security.sh
index 1a5ee44..ce2b5da 100755
--- a/tests/t0301-security.sh
+++ b/tests/t0301-security.sh
@@ -445,7 +445,7 @@ test_expect_success 'the dumb transport refuses a path that climbs out' '
 
 test_expect_success 'the about page serves nothing above the readme directory' '
 	cveq "url=cve/about/../cverc" >tmp &&
-	grep "^Status: 200" tmp &&
+	grep "^Status: 404" tmp &&
 	! grep "virtual-root" tmp &&
 	cveq "url=cve/about/" >tmp &&
 	grep "readme text" tmp
diff --git a/tests/t0303-robustness.sh b/tests/t0303-robustness.shindex 96c7897..d889a32 100755--- a/tests/t0303-robustness.sh+++ b/tests/t0303-robustness.sh@@ -218,7 +218,7 @@ test_expect_success 'a submodule below the about path does not end the request' 		echo "repo.readme=master:file-1" 	} >aboutrc && 	CGIT_CONFIG="$PWD/aboutrc" QUERY_STRING="url=rob/about/sub" cgit >tmp 2>err &&-	grep "^Status:" tmp &&+	grep "^Status: 404" tmp && 	! grep "fatal" err ' @@ -525,6 +525,7 @@ test_expect_success 'a disk readme in the repository directory serves no sibling 	CGIT_CONFIG="$PWD/diskrc" QUERY_STRING="url=rob/about/" cgit >tmp && 	grep "hello" tmp && 	CGIT_CONFIG="$PWD/diskrc" QUERY_STRING="url=rob/about/config" cgit >tmp &&+	grep "^Status: 404" tmp && 	! grep "repositoryformatversion" tmp ' @@ -617,17 +618,27 @@ test_expect_success 'pages behind an auth filter are marked private' ' 	! grep "^Vary" tmp ' -# The about page redirects to its trailing-slash form so relative links-# resolve, and the branch asked for has to survive that hop, as does the-# hop back to the summary of a repository without a readme.-test_expect_success 'the about redirects keep the query string' '+# A repository without a readme sends its about page to the summary, and+# the branch asked for has to survive that hop.+test_expect_success 'the about redirect keeps the query string' ' 	git -C repos/rob branch other HEAD &&-	CGIT_CONFIG="$PWD/aboutrc" QUERY_STRING="url=rob/about&h=other&x=1" cgit >tmp &&-	grep "^Status: 301" tmp &&-	grep "^Location: /rob/about/?h=other&x=1$" tmp &&-	robq "url=rob/about/&h=other" >tmp &&+	robq "url=rob/about&h=other&x=1" >tmp && 	grep "^Status: 302" tmp &&-	grep "^Location: /rob/about/../?h=other$" tmp+	grep "^Location: /rob/?h=other&x=1$" tmp &&+	CGIT_CONFIG="$PWD/aboutrc" QUERY_STRING="url=rob/about&h=other" cgit >tmp &&+	grep "^Status: 200" tmp+'++# The about page shows the readme alone and its links lead into the tree,+# so a path below it names nothing, the site about page included.+test_expect_success 'a path below the about page answers 404' '+	for path in file-1 sub/ ../robrc+	do+		CGIT_CONFIG="$PWD/aboutrc" QUERY_STRING="url=rob/about/$path" cgit >tmp &&+		grep "^Status: 404" tmp || return 1+	done &&+	CGIT_CONFIG="$PWD/aboutrc" QUERY_STRING="p=about&path=x" cgit >tmp &&+	grep "^Status: 404" tmp '  test_expect_success 'a symlink whose target is a large blob is listed without it' '
diff --git a/tests/t0303-robustness.sh b/tests/t0303-robustness.sh
index 96c7897..d889a32 100755
--- a/tests/t0303-robustness.sh
+++ b/tests/t0303-robustness.sh
@@ -218,7 +218,7 @@ test_expect_success 'a submodule below the about path does not end the request'
 		echo "repo.readme=master:file-1"
 	} >aboutrc &&
 	CGIT_CONFIG="$PWD/aboutrc" QUERY_STRING="url=rob/about/sub" cgit >tmp 2>err &&
-	grep "^Status:" tmp &&
+	grep "^Status: 404" tmp &&
 	! grep "fatal" err
 '
 
@@ -525,6 +525,7 @@ test_expect_success 'a disk readme in the repository directory serves no sibling
 	CGIT_CONFIG="$PWD/diskrc" QUERY_STRING="url=rob/about/" cgit >tmp &&
 	grep "hello" tmp &&
 	CGIT_CONFIG="$PWD/diskrc" QUERY_STRING="url=rob/about/config" cgit >tmp &&
+	grep "^Status: 404" tmp &&
 	! grep "repositoryformatversion" tmp
 '
 
@@ -617,17 +618,27 @@ test_expect_success 'pages behind an auth filter are marked private' '
 	! grep "^Vary" tmp
 '
 
-# The about page redirects to its trailing-slash form so relative links
-# resolve, and the branch asked for has to survive that hop, as does the
-# hop back to the summary of a repository without a readme.
-test_expect_success 'the about redirects keep the query string' '
+# A repository without a readme sends its about page to the summary, and
+# the branch asked for has to survive that hop.
+test_expect_success 'the about redirect keeps the query string' '
 	git -C repos/rob branch other HEAD &&
-	CGIT_CONFIG="$PWD/aboutrc" QUERY_STRING="url=rob/about&h=other&x=1" cgit >tmp &&
-	grep "^Status: 301" tmp &&
-	grep "^Location: /rob/about/?h=other&x=1$" tmp &&
-	robq "url=rob/about/&h=other" >tmp &&
+	robq "url=rob/about&h=other&x=1" >tmp &&
 	grep "^Status: 302" tmp &&
-	grep "^Location: /rob/about/../?h=other$" tmp
+	grep "^Location: /rob/?h=other&x=1$" tmp &&
+	CGIT_CONFIG="$PWD/aboutrc" QUERY_STRING="url=rob/about&h=other" cgit >tmp &&
+	grep "^Status: 200" tmp
+'
+
+# The about page shows the readme alone and its links lead into the tree,
+# so a path below it names nothing, the site about page included.
+test_expect_success 'a path below the about page answers 404' '
+	for path in file-1 sub/ ../robrc
+	do
+		CGIT_CONFIG="$PWD/aboutrc" QUERY_STRING="url=rob/about/$path" cgit >tmp &&
+		grep "^Status: 404" tmp || return 1
+	done &&
+	CGIT_CONFIG="$PWD/aboutrc" QUERY_STRING="p=about&path=x" cgit >tmp &&
+	grep "^Status: 404" tmp
 '
 
 test_expect_success 'a symlink whose target is a large blob is listed without it' '