Lead a readme's relative links into the tree

A browser resolved them against the about page, where nothing lives
now that the page answers 404 below itself. They resolve the way
GitHub resolves a readme's links instead, from the readme's directory
and on the branch it was read from.

4 files changed, 114 insertions, 11 deletions

diff --git a/custom/extensions/about-render.lua b/custom/extensions/about-render.luaindex ecb2abe..a3932af 100644--- a/custom/extensions/about-render.lua+++ b/custom/extensions/about-render.lua@@ -30,7 +30,7 @@ local max_bytes = 512 * 1024 -- markers cannot drive the recursive parser into a stack overflow. local max_depth = 24 -local filename, chunks = "", {}+local filename, tree_base, tree_query, chunks = "", "", "", {}   -- Lua patterns backtrack, so "%s+$" costs the square of a run of spaces in@@ -108,11 +108,56 @@ local function safe_url(url) 	return url end +-- A readme percent-encodes what a link path cannot carry, and the path is+-- encoded again on its way out, so it is decoded first. An encoded NUL stays+-- encoded, since cgit's sinks would stop at a real one.+local function unescape_path(path)+	return (path:gsub("%%(%x%x)", function(hex)+		local byte = tonumber(hex, 16)+		if byte > 0 then+			return string.char(byte)+		end+	end))+end++-- The path in the tree that a relative link names. Like GitHub, it resolves+-- against the readme's own directory, or against the root when it starts+-- with a slash, and dot segments fold without climbing above the root.+local function tree_path(path)+	if path:sub(1, 1) ~= "/" then+		path = (filename:match("^(.*/)") or "") .. path+	end+	local segments = {}+	for segment in path:gmatch("[^/]+") do+		if segment == ".." then+			segments[#segments] = nil+		elseif segment ~= "." then+			segments[#segments + 1] = segment+		end+	end+	return table.concat(segments, "/")+end++-- Writes a link target that safe_url passed. A relative one leads into the+-- tree at the ref the readme came from, keeping its fragment but not a+-- query, which the tree page would read as its own. A readme with no tree+-- behind it keeps its links as written.+local function emit_href(url)+	if tree_base == "" or url:find("^#") or url:find("^%a[%w%+%.%-]*:") then+		html_attr(url)+		return+	end+	html_attr(tree_base)+	html_url_path(tree_path(unescape_path(url:match("^[^?#]*"))))+	html_attr(tree_query)+	html_attr(url:match("#.*$") or "")+end+  -- The page renders untrusted repository content. Every run of text goes out--- through cgit's html_txt, every attribute value through html_attr, and--- every link or image target through safe_url before html_attr. html() only--- ever gets literal tag scaffolding.+-- through cgit's html_txt, every attribute value through html_attr or, for a+-- path in the tree, html_url_path, and every link or image target through+-- safe_url first. html() only ever gets literal tag scaffolding.  local emit_inline @@ -130,7 +175,7 @@ emit_inline = function(list) 		elseif kind == "link" then 			local target = safe_url(node.url) 			if target then-				html("<a href='"); html_attr(target); html("'>")+				html("<a href='"); emit_href(target); html("'>") 				emit_inline(node.kids) 				html("</a>") 			else@@ -632,8 +677,12 @@ for _, ext in ipairs({ "1", "2", "3", "4", "5", "6", "7", "8", "9", "man" }) do 	handlers[ext] = render_man end -function filter_open(name)+-- A readme tracked in the repository comes with the url of the tree page at+-- its ref and the query that follows a path there, both escaped for a url.+function filter_open(name, base, query) 	filename = name or ""+	tree_base = base or ""+	tree_query = query or "" 	chunks = {} end 
diff --git a/custom/extensions/about-render.lua b/custom/extensions/about-render.lua
index ecb2abe..a3932af 100644
--- a/custom/extensions/about-render.lua
+++ b/custom/extensions/about-render.lua
@@ -30,7 +30,7 @@ local max_bytes = 512 * 1024
 -- markers cannot drive the recursive parser into a stack overflow.
 local max_depth = 24
 
-local filename, chunks = "", {}
+local filename, tree_base, tree_query, chunks = "", "", "", {}
 
 
 -- Lua patterns backtrack, so "%s+$" costs the square of a run of spaces in
@@ -108,11 +108,56 @@ local function safe_url(url)
 	return url
 end
 
+-- A readme percent-encodes what a link path cannot carry, and the path is
+-- encoded again on its way out, so it is decoded first. An encoded NUL stays
+-- encoded, since cgit's sinks would stop at a real one.
+local function unescape_path(path)
+	return (path:gsub("%%(%x%x)", function(hex)
+		local byte = tonumber(hex, 16)
+		if byte > 0 then
+			return string.char(byte)
+		end
+	end))
+end
+
+-- The path in the tree that a relative link names. Like GitHub, it resolves
+-- against the readme's own directory, or against the root when it starts
+-- with a slash, and dot segments fold without climbing above the root.
+local function tree_path(path)
+	if path:sub(1, 1) ~= "/" then
+		path = (filename:match("^(.*/)") or "") .. path
+	end
+	local segments = {}
+	for segment in path:gmatch("[^/]+") do
+		if segment == ".." then
+			segments[#segments] = nil
+		elseif segment ~= "." then
+			segments[#segments + 1] = segment
+		end
+	end
+	return table.concat(segments, "/")
+end
+
+-- Writes a link target that safe_url passed. A relative one leads into the
+-- tree at the ref the readme came from, keeping its fragment but not a
+-- query, which the tree page would read as its own. A readme with no tree
+-- behind it keeps its links as written.
+local function emit_href(url)
+	if tree_base == "" or url:find("^#") or url:find("^%a[%w%+%.%-]*:") then
+		html_attr(url)
+		return
+	end
+	html_attr(tree_base)
+	html_url_path(tree_path(unescape_path(url:match("^[^?#]*"))))
+	html_attr(tree_query)
+	html_attr(url:match("#.*$") or "")
+end
+
 
 -- The page renders untrusted repository content. Every run of text goes out
--- through cgit's html_txt, every attribute value through html_attr, and
--- every link or image target through safe_url before html_attr. html() only
--- ever gets literal tag scaffolding.
+-- through cgit's html_txt, every attribute value through html_attr or, for a
+-- path in the tree, html_url_path, and every link or image target through
+-- safe_url first. html() only ever gets literal tag scaffolding.
 
 local emit_inline
 
@@ -130,7 +175,7 @@ emit_inline = function(list)
 		elseif kind == "link" then
 			local target = safe_url(node.url)
 			if target then
-				html("<a href='"); html_attr(target); html("'>")
+				html("<a href='"); emit_href(target); html("'>")
 				emit_inline(node.kids)
 				html("</a>")
 			else
@@ -632,8 +677,12 @@ for _, ext in ipairs({ "1", "2", "3", "4", "5", "6", "7", "8", "9", "man" }) do
 	handlers[ext] = render_man
 end
 
-function filter_open(name)
+-- A readme tracked in the repository comes with the url of the tree page at
+-- its ref and the query that follows a path there, both escaped for a url.
+function filter_open(name, base, query)
 	filename = name or ""
+	tree_base = base or ""
+	tree_query = query or ""
 	chunks = {}
 end
 
diff --git a/tests/extensions/harness.lua b/tests/extensions/harness.luaindex d9318ee..fc83a73 100644--- a/tests/extensions/harness.lua+++ b/tests/extensions/harness.lua@@ -46,16 +46,23 @@ html_attr = sink(function(s) 	return (s:gsub("[&<>'\"]", attr_map)) end) +-- The bytes url_escape_table encodes, which are those a url path cannot+-- carry plainly and those an attribute or a query would misread.+html_url_path = sink(function(s)+	return (s:gsub("[%c \"#%%&'+<=>?\\^`{|}\128-\255]", function(c)+		return string.format("%%%02X", c:byte())+	end))+end)+ -- No shipped extension calls the remaining sinks, so rather than risk an -- unfaithful copy quietly passing a test, using one fails loudly until its--- escaping is mirrored from source/html.c the way the three above are.+-- escaping is mirrored from source/html.c the way the ones above are. local function unmirrored(name) 	return function() 		error(name .. " is not mirrored by the test harness yet") 	end end -html_url_path = unmirrored("html_url_path") html_url_arg = unmirrored("html_url_arg") html_include = unmirrored("html_include") 
diff --git a/tests/extensions/harness.lua b/tests/extensions/harness.lua
index d9318ee..fc83a73 100644
--- a/tests/extensions/harness.lua
+++ b/tests/extensions/harness.lua
@@ -46,16 +46,23 @@ html_attr = sink(function(s)
 	return (s:gsub("[&<>'\"]", attr_map))
 end)
 
+-- The bytes url_escape_table encodes, which are those a url path cannot
+-- carry plainly and those an attribute or a query would misread.
+html_url_path = sink(function(s)
+	return (s:gsub("[%c \"#%%&'+<=>?\\^`{|}\128-\255]", function(c)
+		return string.format("%%%02X", c:byte())
+	end))
+end)
+
 -- No shipped extension calls the remaining sinks, so rather than risk an
 -- unfaithful copy quietly passing a test, using one fails loudly until its
--- escaping is mirrored from source/html.c the way the three above are.
+-- escaping is mirrored from source/html.c the way the ones above are.
 local function unmirrored(name)
 	return function()
 		error(name .. " is not mirrored by the test harness yet")
 	end
 end
 
-html_url_path = unmirrored("html_url_path")
 html_url_arg = unmirrored("html_url_arg")
 html_include = unmirrored("html_include")
 
diff --git a/tests/extensions/test-about-render.lua b/tests/extensions/test-about-render.luaindex b0379b7..4f8b532 100644--- a/tests/extensions/test-about-render.lua+++ b/tests/extensions/test-about-render.lua@@ -28,6 +28,11 @@ local function render(filename, text) 	return h.run({ filename }, { text }) end +-- A readme tracked in a repository comes with the url of its tree.+local function render_tree(filename, query, text)+	return h.run({ filename, "/r/tree/", query }, { text })+end+ -- Dispatch and the plain text rendering need nothing but the script itself, -- so they run whatever became of lpeg. @@ -107,7 +112,36 @@ out = render("README.md", "[docs](https://example.com/docs)") h.contains("an https link renders as an anchor", out, "<a href='https://example.com/docs'>docs</a>")  out = render("README.md", "[docs](docs/page.html)")-h.contains("a relative link renders as an anchor", out, "<a href='docs/page.html'>docs</a>")+h.contains("with no tree a relative link stays as written", out, "<a href='docs/page.html'>docs</a>")++out = render_tree("README.md", "", "[road](ROADMAP.md)")+h.contains("a relative link leads into the tree", out, "<a href='/r/tree/ROADMAP.md'>road</a>")++out = render_tree("docs/README.md", "", "[a](guide.md#setup) [b](../NEWS) [c](/src/x.c)")+h.contains("a link resolves against the readme's directory", out, "href='/r/tree/docs/guide.md#setup'")+h.contains("dot segments fold", out, "href='/r/tree/NEWS'")+h.contains("a leading slash names the root", out, "href='/r/tree/src/x.c'")++out = render_tree("docs/README.md", "", "[up](../../../etc/passwd)")+h.contains("no link climbs above the root", out, "href='/r/tree/etc/passwd'")++out = render_tree("README.md", "?h=next", "[a](NEWS#top)")+h.contains("the ref's query comes before the fragment", out, "href='/r/tree/NEWS?h=next#top'")++out = render_tree("README.md", "&h=next", "[a](NEWS)")+h.contains("the query is escaped for the attribute", out, "href='/r/tree/NEWS&amp;h=next'")++out = render_tree("README.md", "", "[a](my%20file.md) [b](a&b.md) [c](f.md?plain=1)")+h.contains("an encoded path is encoded only once", out, "href='/r/tree/my%20file.md'")+h.contains("a path is escaped for the url", out, "href='/r/tree/a%26b.md'")+h.contains("a query in a link is dropped", out, "href='/r/tree/f.md'")++out = render_tree("README.md", "", "[a](x%00y)")+h.contains("an encoded nul stays encoded", out, "href='/r/tree/x%2500y'")++out = render_tree("README.md", "", "[a](#usage) [b](https://example.com/x)")+h.contains("a fragment stays on the page", out, "href='#usage'")+h.contains("an absolute link is left alone", out, "href='https://example.com/x'")  out = render("README.md", "[mail](mailto:a@example.com)") h.contains("a mailto link renders as an anchor", out, "<a href='mailto:a@example.com'>mail</a>")
diff --git a/tests/extensions/test-about-render.lua b/tests/extensions/test-about-render.lua
index b0379b7..4f8b532 100644
--- a/tests/extensions/test-about-render.lua
+++ b/tests/extensions/test-about-render.lua
@@ -28,6 +28,11 @@ local function render(filename, text)
 	return h.run({ filename }, { text })
 end
 
+-- A readme tracked in a repository comes with the url of its tree.
+local function render_tree(filename, query, text)
+	return h.run({ filename, "/r/tree/", query }, { text })
+end
+
 -- Dispatch and the plain text rendering need nothing but the script itself,
 -- so they run whatever became of lpeg.
 
@@ -107,7 +112,36 @@ out = render("README.md", "[docs](https://example.com/docs)")
 h.contains("an https link renders as an anchor", out, "<a href='https://example.com/docs'>docs</a>")
 
 out = render("README.md", "[docs](docs/page.html)")
-h.contains("a relative link renders as an anchor", out, "<a href='docs/page.html'>docs</a>")
+h.contains("with no tree a relative link stays as written", out, "<a href='docs/page.html'>docs</a>")
+
+out = render_tree("README.md", "", "[road](ROADMAP.md)")
+h.contains("a relative link leads into the tree", out, "<a href='/r/tree/ROADMAP.md'>road</a>")
+
+out = render_tree("docs/README.md", "", "[a](guide.md#setup) [b](../NEWS) [c](/src/x.c)")
+h.contains("a link resolves against the readme's directory", out, "href='/r/tree/docs/guide.md#setup'")
+h.contains("dot segments fold", out, "href='/r/tree/NEWS'")
+h.contains("a leading slash names the root", out, "href='/r/tree/src/x.c'")
+
+out = render_tree("docs/README.md", "", "[up](../../../etc/passwd)")
+h.contains("no link climbs above the root", out, "href='/r/tree/etc/passwd'")
+
+out = render_tree("README.md", "?h=next", "[a](NEWS#top)")
+h.contains("the ref's query comes before the fragment", out, "href='/r/tree/NEWS?h=next#top'")
+
+out = render_tree("README.md", "&h=next", "[a](NEWS)")
+h.contains("the query is escaped for the attribute", out, "href='/r/tree/NEWS&amp;h=next'")
+
+out = render_tree("README.md", "", "[a](my%20file.md) [b](a&b.md) [c](f.md?plain=1)")
+h.contains("an encoded path is encoded only once", out, "href='/r/tree/my%20file.md'")
+h.contains("a path is escaped for the url", out, "href='/r/tree/a%26b.md'")
+h.contains("a query in a link is dropped", out, "href='/r/tree/f.md'")
+
+out = render_tree("README.md", "", "[a](x%00y)")
+h.contains("an encoded nul stays encoded", out, "href='/r/tree/x%2500y'")
+
+out = render_tree("README.md", "", "[a](#usage) [b](https://example.com/x)")
+h.contains("a fragment stays on the page", out, "href='#usage'")
+h.contains("an absolute link is left alone", out, "href='https://example.com/x'")
 
 out = render("README.md", "[mail](mailto:a@example.com)")
 h.contains("a mailto link renders as an anchor", out, "<a href='mailto:a@example.com'>mail</a>")
diff --git a/tests/t0501-about-render.sh b/tests/t0501-about-render.shindex f870c0e..b47c11d 100755--- a/tests/t0501-about-render.sh+++ b/tests/t0501-about-render.sh@@ -36,9 +36,12 @@ test_expect_success 'create a repository with a markdown readme' ' 		# The Title  		A **bold** claim and a <script>alert(1)</script> attempt.++		See the [roadmap](ROADMAP.md#plan). 		EOF 		git add README.md &&-		git commit -m "add readme"+		git commit -m "add readme" &&+		git branch next 	) ' @@ -48,7 +51,7 @@ test_expect_success 'point cgit at it through the about filter' ' 	cache-size=0 	repo.url=md 	repo.path=$PWD/repos/md/.git-	repo.readme=master:README.md+	repo.readme=:README.md 	repo.about-filter=lua:$EXTENSIONS_DIRECTORY/about-render.lua 	EOF '@@ -78,6 +81,16 @@ test_expect_success CGIT_LUA_LPEG 'readme markup reaches the page escaped' ' 	! grep "<script>alert(1)" tmp ' +test_expect_success CGIT_LUA_LPEG 'a relative link leads into the tree' '+	cgit_url "md/about/" >tmp &&+	grep "<a href=./md/tree/ROADMAP.md#plan.>roadmap</a>" tmp+'++test_expect_success CGIT_LUA_LPEG 'the link stays on the branch shown' '+	cgit_query "url=md/about/&h=next" >tmp &&+	grep "<a href=./md/tree/ROADMAP.md?h=next#plan.>roadmap</a>" tmp+'+ test_expect_success CGIT_LUA_NOLPEG 'without lpeg the readme is served escaped' ' 	cgit_url "md/about/" >tmp && 	grep "<pre class=.plaintext.>" tmp &&
diff --git a/tests/t0501-about-render.sh b/tests/t0501-about-render.sh
index f870c0e..b47c11d 100755
--- a/tests/t0501-about-render.sh
+++ b/tests/t0501-about-render.sh
@@ -36,9 +36,12 @@ test_expect_success 'create a repository with a markdown readme' '
 		# The Title
 
 		A **bold** claim and a <script>alert(1)</script> attempt.
+
+		See the [roadmap](ROADMAP.md#plan).
 		EOF
 		git add README.md &&
-		git commit -m "add readme"
+		git commit -m "add readme" &&
+		git branch next
 	)
 '
 
@@ -48,7 +51,7 @@ test_expect_success 'point cgit at it through the about filter' '
 	cache-size=0
 	repo.url=md
 	repo.path=$PWD/repos/md/.git
-	repo.readme=master:README.md
+	repo.readme=:README.md
 	repo.about-filter=lua:$EXTENSIONS_DIRECTORY/about-render.lua
 	EOF
 '
@@ -78,6 +81,16 @@ test_expect_success CGIT_LUA_LPEG 'readme markup reaches the page escaped' '
 	! grep "<script>alert(1)" tmp
 '
 
+test_expect_success CGIT_LUA_LPEG 'a relative link leads into the tree' '
+	cgit_url "md/about/" >tmp &&
+	grep "<a href=./md/tree/ROADMAP.md#plan.>roadmap</a>" tmp
+'
+
+test_expect_success CGIT_LUA_LPEG 'the link stays on the branch shown' '
+	cgit_query "url=md/about/&h=next" >tmp &&
+	grep "<a href=./md/tree/ROADMAP.md?h=next#plan.>roadmap</a>" tmp
+'
+
 test_expect_success CGIT_LUA_NOLPEG 'without lpeg the readme is served escaped' '
 	cgit_url "md/about/" >tmp &&
 	grep "<pre class=.plaintext.>" tmp &&