markup: address review feedback for goldmark migration

- Use lazyregexp for linkifyURLRegexp to avoid compile-at-init overhead.
- Replace stdlib log with clog/v2 and return HTML-escaped body on
  conversion error instead of nil.
- Handle absolute URL prefixes in linkTransformer using net/url to
  preserve scheme and host.
- Remove goldmarkhtml.WithUnsafe() from RawMarkdown renderer options.
- Use exact assertions in autolink tests instead of Contains.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

internal/markup/markdown.go

@@ -4,10 +4,9 @@ import (
 	"bytes"
 	"fmt"
 	"html"
-	"log"
+	"net/url"
 	"path"
 	"path/filepath"
-	"regexp"
 	"strings"
 
 	"github.com/yuin/goldmark"
@@ -18,6 +17,7 @@ import (
 	goldmarkhtml "github.com/yuin/goldmark/renderer/html"
 	"github.com/yuin/goldmark/text"
 	"github.com/yuin/goldmark/util"
+	log "unknwon.dev/clog/v2"
 
 	"gogs.io/gogs/internal/conf"
 	"gogs.io/gogs/internal/lazyregexp"
@@ -37,7 +37,7 @@ func IsMarkdownFile(name string) bool {
 
 var (
 	validLinksPattern = lazyregexp.New(`^[a-z][\w-]+://|^mailto:`)
-	linkifyURLRegexp  = regexp.MustCompile(`^(?:http|https|ftp)://[-a-zA-Z0-9@:%._+~#=]{1,256}(?:\.[a-z]+)?(?::\d+)?(?:[/#?][-a-zA-Z0-9@:%_+.~#$!?&/=();,'\^{}\[\]` + "`" + `]*)?`)
+	linkifyURLRegexp  = lazyregexp.New(`^(?:http|https|ftp)://[-a-zA-Z0-9@:%._+~#=]{1,256}(?:\.[a-z]+)?(?::\d+)?(?:[/#?][-a-zA-Z0-9@:%_+.~#$!?&/=();,'\^{}\[\]` + "`" + `]*)?`)
 )
 
 func isLink(link []byte) bool {
@@ -56,13 +56,25 @@ func (t *linkTransformer) Transform(node *ast.Document, reader text.Reader, _ pa
 		if link, ok := n.(*ast.Link); ok {
 			dest := link.Destination
 			if len(dest) > 0 && !isLink(dest) && dest[0] != '#' {
-				link.Destination = []byte(path.Join(t.urlPrefix, string(dest)))
+				link.Destination = []byte(joinURLPath(t.urlPrefix, string(dest)))
 			}
 		}
 		return ast.WalkContinue, nil
 	})
 }
 
+// joinURLPath joins a base URL prefix with a relative path. It uses net/url for
+// absolute URL prefixes to preserve the scheme and host, and path.Join for
+// path-only prefixes.
+func joinURLPath(prefix, relPath string) string {
+	u, err := url.Parse(prefix)
+	if err != nil || u.Scheme == "" {
+		return path.Join(prefix, relPath)
+	}
+	u.Path = path.Join(u.Path, relPath)
+	return u.String()
+}
+
 type gogsRenderer struct {
 	urlPrefix string
 }
@@ -135,7 +147,7 @@ func RawMarkdown(body []byte, urlPrefix string) []byte {
 		extension.Table,
 		extension.Strikethrough,
 		extension.TaskList,
-		extension.NewLinkify(extension.WithLinkifyURLRegexp(linkifyURLRegexp)),
+		extension.NewLinkify(extension.WithLinkifyURLRegexp(linkifyURLRegexp.Regexp())),
 	}
 
 	if conf.Smartypants.Enabled {
@@ -143,7 +155,6 @@ func RawMarkdown(body []byte, urlPrefix string) []byte {
 	}
 
 	rendererOpts := []renderer.Option{
-		goldmarkhtml.WithUnsafe(),
 		renderer.WithNodeRenderers(
 			util.Prioritized(&gogsRenderer{urlPrefix: urlPrefix}, 0),
 		),
@@ -165,8 +176,8 @@ func RawMarkdown(body []byte, urlPrefix string) []byte {
 
 	var buf bytes.Buffer
 	if err := md.Convert(body, &buf); err != nil {
-		log.Printf("markup: failed to convert Markdown: %v", err)
-		return nil
+		log.Error("Failed to convert Markdown: %v", err)
+		return []byte(html.EscapeString(string(body)))
 	}
 	return buf.Bytes()
 }

internal/markup/markdown_test.go

@@ -5,7 +5,6 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 
 	"gogs.io/gogs/internal/conf"
 	. "gogs.io/gogs/internal/markup"
@@ -47,39 +46,38 @@ func Test_RawMarkdown_AutoLink(t *testing.T) {
 		{
 			name:  "issue URL from same instance",
 			input: "http://localhost:3000/user/repo/issues/3333",
-			want:  `<a href="http://localhost:3000/user/repo/issues/3333">#3333</a>`,
+			want:  "<p><a href=\"http://localhost:3000/user/repo/issues/3333\">#3333</a></p>\n",
 		},
 		{
 			name:  "non-matching issue-like URL",
 			input: "http://1111/2222/ssss-issues/3333?param=blah&blahh=333",
-			want:  `<a href="http://1111/2222/ssss-issues/3333?param=blah&amp;blahh=333">http://1111/2222/ssss-issues/3333?param=blah&amp;blahh=333</a>`,
+			want:  "<p><a href=\"http://1111/2222/ssss-issues/3333?param=blah&amp;blahh=333\">http://1111/2222/ssss-issues/3333?param=blah&amp;blahh=333</a></p>\n",
 		},
 		{
 			name:  "external issue URL",
 			input: "http://test.com/issues/33333",
-			want:  `http://test.com/issues/33333`,
+			want:  "<p><a href=\"http://test.com/issues/33333\">http://test.com/issues/33333</a></p>\n",
 		},
 		{
 			name:  "commit URL from same instance",
 			input: "http://localhost:3000/user/project/commit/d8a994ef243349f321568f9e36d5c3f444b99cae",
-			want:  `<code><a href="http://localhost:3000/user/project/commit/d8a994ef243349f321568f9e36d5c3f444b99cae">d8a994ef24</a></code>`,
+			want:  "<p> <code><a href=\"http://localhost:3000/user/project/commit/d8a994ef243349f321568f9e36d5c3f444b99cae\">d8a994ef24</a></code></p>\n",
 		},
 		{
 			name:  "commit URL with fragment from same instance",
 			input: "http://localhost:3000/user/project/commit/d8a994ef243349f321568f9e36d5c3f444b99cae#diff-2",
-			want:  `<code><a href="http://localhost:3000/user/project/commit/d8a994ef243349f321568f9e36d5c3f444b99cae#diff-2">d8a994ef24</a></code>`,
+			want:  "<p> <code><a href=\"http://localhost:3000/user/project/commit/d8a994ef243349f321568f9e36d5c3f444b99cae#diff-2\">d8a994ef24</a></code></p>\n",
 		},
 		{
 			name:  "external commit URL",
 			input: "https://external-link.gogs.io/gogs/gogs/commit/d8a994ef243349f321568f9e36d5c3f444b99cae#diff-2",
-			want:  `https://external-link.gogs.io/gogs/gogs/commit/d8a994ef243349f321568f9e36d5c3f444b99cae#diff-2`,
+			want:  "<p><a href=\"https://external-link.gogs.io/gogs/gogs/commit/d8a994ef243349f321568f9e36d5c3f444b99cae#diff-2\">https://external-link.gogs.io/gogs/gogs/commit/d8a994ef243349f321568f9e36d5c3f444b99cae#diff-2</a></p>\n",
 		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			result := string(RawMarkdown([]byte(test.input), ""))
-			require.NotEmpty(t, result)
-			assert.Contains(t, result, test.want)
+			got := string(RawMarkdown([]byte(test.input), ""))
+			assert.Equal(t, test.want, got)
 		})
 	}
 }