Fix orgmode link resolving (go-gitea#29024)

Fix go-gitea#28974 Add some new tests and fix some legacy unclear tests. # Conflicts: # modules/markup/orgmode/orgmode_test.go
wxiaoguang · Feb 7, 2024 · c2ddaec · c2ddaec
1 parent 283cfe0
commit c2ddaec
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 41 deletions.
diff --git a/modules/markup/orgmode/orgmode.go b/modules/markup/orgmode/orgmode.go
@@ -133,18 +133,18 @@ type Writer struct {
 	Ctx *markup.RenderContext
 }
 
-const mailto = "mailto:"
-
-func (r *Writer) resolveLink(l org.RegularLink) string {
-	link := html.EscapeString(l.URL)
-	if l.Protocol == "file" {
-		link = link[len("file:"):]
-	}
-	if len(link) > 0 && !markup.IsLinkStr(link) &&
-		link[0] != '#' && !strings.HasPrefix(link, mailto) {
+func (r *Writer) resolveLink(kind, link string) string {
+	link = strings.TrimPrefix(link, "file:")
+	if !strings.HasPrefix(link, "#") && // not a URL fragment
+		!markup.IsLinkStr(link) && // not an absolute URL
+		!strings.HasPrefix(link, "mailto:") {
+		if kind == "regular" {
+			// orgmode reports the link kind as "regular" for "[[ImageLink.svg][The Image Desc]]"
+			// so we need to try to guess the link kind again here
+			kind = org.RegularLink{URL: link}.Kind()
+		}
 		base := r.Ctx.Links.Base
-		switch l.Kind() {
-		case "image", "video":
+		if kind == "image" || kind == "video" {
 			base = r.Ctx.Links.ResolveMediaLink(r.Ctx.IsWiki)
 		}
 		link = util.URLJoin(base, link)
@@ -154,29 +154,29 @@ func (r *Writer) resolveLink(l org.RegularLink) string {
 
 // WriteRegularLink renders images, links or videos
 func (r *Writer) WriteRegularLink(l org.RegularLink) {
-	link := r.resolveLink(l)
+	link := r.resolveLink(l.Kind(), l.URL)
 
 	// Inspired by https://github.com/niklasfasching/go-org/blob/6eb20dbda93cb88c3503f7508dc78cbbc639378f/org/html_writer.go#L406-L427
 	switch l.Kind() {
 	case "image":
 		if l.Description == nil {
-			fmt.Fprintf(r, `<img src="%s" alt="%s" />`, link, link)
+			_, _ = fmt.Fprintf(r, `<img src="%s" alt="%s" />`, link, link)
 		} else {
-			imageSrc := r.resolveLink(l.Description[0].(org.RegularLink))
-			fmt.Fprintf(r, `<a href="%s"><img src="%s" alt="%s" /></a>`, link, imageSrc, imageSrc)
+			imageSrc := r.resolveLink(l.Kind(), org.String(l.Description...))
+			_, _ = fmt.Fprintf(r, `<a href="%s"><img src="%s" alt="%s" /></a>`, link, imageSrc, imageSrc)
 		}
 	case "video":
 		if l.Description == nil {
-			fmt.Fprintf(r, `<video src="%s">%s</video>`, link, link)
+			_, _ = fmt.Fprintf(r, `<video src="%s">%s</video>`, link, link)
 		} else {
-			videoSrc := r.resolveLink(l.Description[0].(org.RegularLink))
-			fmt.Fprintf(r, `<a href="%s"><video src="%s">%s</video></a>`, link, videoSrc, videoSrc)
+			videoSrc := r.resolveLink(l.Kind(), org.String(l.Description...))
+			_, _ = fmt.Fprintf(r, `<a href="%s"><video src="%s">%s</video></a>`, link, videoSrc, videoSrc)
 		}
 	default:
 		description := link
 		if l.Description != nil {
 			description = r.WriteNodesAsString(l.Description...)
 		}
-		fmt.Fprintf(r, `<a href="%s">%s</a>`, link, description)
+		_, _ = fmt.Fprintf(r, `<a href="%s">%s</a>`, link, description)
 	}
 }
diff --git a/modules/markup/orgmode/orgmode_test.go b/modules/markup/orgmode/orgmode_test.go
@@ -10,60 +10,53 @@ import (
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/markup"
 	"code.gitea.io/gitea/modules/setting"
-	"code.gitea.io/gitea/modules/util"
 
 	"github.com/stretchr/testify/assert"
 )
 
-const (
-	AppURL    = "http://localhost:3000/"
-	Repo      = "gogits/gogs"
-	AppSubURL = AppURL + Repo + "/"
-)
+const AppURL = "http://localhost:3000/"
 
 func TestRender_StandardLinks(t *testing.T) {
 	setting.AppURL = AppURL
-	setting.AppSubURL = AppSubURL
 
 	test := func(input, expected string) {
 		buffer, err := RenderString(&markup.RenderContext{
 			Ctx: git.DefaultContext,
 			Links: markup.Links{
-				Base: setting.AppSubURL,
+				Base:       "/relative-path",
+				BranchPath: "branch/main",
 			},
 		}, input)
 		assert.NoError(t, err)
 		assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer))
 	}
 
-	googleRendered := "<p><a href=\"https://google.com/\">https://google.com/</a></p>"
-	test("[[https://google.com/]]", googleRendered)
-
-	lnk := util.URLJoin(AppSubURL, "WikiPage")
-	test("[[WikiPage][WikiPage]]",
-		"<p><a href=\""+lnk+"\">WikiPage</a></p>")
+	test("[[https://google.com/]]",
+		`<p><a href="https://google.com/">https://google.com/</a></p>`)
+	test("[[WikiPage][The WikiPage Desc]]",
+		`<p><a href="/relative-path/WikiPage">The WikiPage Desc</a></p>`)
+	test("[[ImageLink.svg][The Image Desc]]",
+		`<p><a href="/relative-path/media/branch/main/ImageLink.svg">The Image Desc</a></p>`)
 }
 
 func TestRender_Media(t *testing.T) {
 	setting.AppURL = AppURL
-	setting.AppSubURL = AppSubURL
 
 	test := func(input, expected string) {
 		buffer, err := RenderString(&markup.RenderContext{
 			Ctx: git.DefaultContext,
 			Links: markup.Links{
-				Base: setting.AppSubURL,
+				Base: "./relative-path",
 			},
 		}, input)
 		assert.NoError(t, err)
 		assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer))
 	}
 
-	url := "../../.images/src/02/train.jpg"
-	result := util.URLJoin(AppSubURL, url)
-
-	test("[[file:"+url+"]]",
-		"<p><img src=\""+result+"\" alt=\""+result+"\" /></p>")
+	test("[[file:../../.images/src/02/train.jpg]]",
+		`<p><img src=".images/src/02/train.jpg" alt=".images/src/02/train.jpg" /></p>`)
+	test("[[file:train.jpg]]",
+		`<p><img src="relative-path/train.jpg" alt="relative-path/train.jpg" /></p>`)
 
 	// With description.
 	test("[[https://example.com][https://example.com/example.svg]]",
@@ -76,11 +69,20 @@ func TestRender_Media(t *testing.T) {
 		`<p><img src="https://example.com/example.svg" alt="https://example.com/example.svg" /></p>`)
 	test("[[https://example.com/example.mp4]]",
 		`<p><video src="https://example.com/example.mp4">https://example.com/example.mp4</video></p>`)
+
+	// test [[LINK][DESCRIPTION]] syntax with "file:" prefix
+	test(`[[https://example.com/][file:https://example.com/foo%20bar.svg]]`,
+		`<p><a href="https://example.com/"><img src="https://example.com/foo%20bar.svg" alt="https://example.com/foo%20bar.svg" /></a></p>`)
+	test(`[[file:https://example.com/foo%20bar.svg][Goto Image]]`,
+		`<p><a href="https://example.com/foo%20bar.svg">Goto Image</a></p>`)
+	test(`[[file:https://example.com/link][https://example.com/image.jpg]]`,
+		`<p><a href="https://example.com/link"><img src="https://example.com/image.jpg" alt="https://example.com/image.jpg" /></a></p>`)
+	test(`[[file:https://example.com/link][file:https://example.com/image.jpg]]`,
+		`<p><a href="https://example.com/link"><img src="https://example.com/image.jpg" alt="https://example.com/image.jpg" /></a></p>`)
 }
 
 func TestRender_Source(t *testing.T) {
 	setting.AppURL = AppURL
-	setting.AppSubURL = AppSubURL
 
 	test := func(input, expected string) {
 		buffer, err := RenderString(&markup.RenderContext{