Skip to content

Commit 022552d

Browse files
KN4CK3RKN4CK3R
authored
Rework markup link rendering (#26745) (#28803)
Backport #26745 Fixes #26548 This PR refactors the rendering of markup links. The old code uses `strings.Replace` to change some urls while the new code uses more context to decide which link should be generated. The added tests should ensure the same output for the old and new behaviour (besides the bug). We may need to refactor the rendering a bit more to make it clear how the different helper methods render the input string. There are lots of options (resolve links / images / mentions / git hashes / emojis / ...) but you don't really know what helper uses which options. For example, we currently support images in the user description which should not be allowed I think: <details> <summary>Profile</summary> https://try.gitea.io/KN4CK3R ![grafik](https://github.com/go-gitea/gitea/assets/1666336/109ae422-496d-4200-b52e-b3a528f553e5) </details>
1 parent 376fa0d commit 022552d

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+966
-388
lines changed

‎models/issues/comment_code.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,11 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu
109109

110110
var err error
111111
if comment.RenderedContent, err = markdown.RenderString(&markup.RenderContext{
112-
Ctx: ctx,
113-
URLPrefix: issue.Repo.Link(),
114-
Metas: issue.Repo.ComposeMetas(),
112+
Ctx: ctx,
113+
Links: markup.Links{
114+
Base: issue.Repo.Link(),
115+
},
116+
Metas: issue.Repo.ComposeMetas(),
115117
}, comment.Content); err != nil {
116118
return nil, err
117119
}

‎models/repo/repo.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -578,8 +578,7 @@ func (repo *Repository) CanEnableEditor() bool {
578578
// DescriptionHTML does special handles to description and return HTML string.
579579
func (repo *Repository) DescriptionHTML(ctx context.Context) template.HTML {
580580
desc, err := markup.RenderDescriptionHTML(&markup.RenderContext{
581-
Ctx: ctx,
582-
URLPrefix: repo.HTMLURL(),
581+
Ctx: ctx,
583582
// Don't use Metas to speedup requests
584583
}, repo.Description)
585584
if err != nil {

‎modules/markup/external/external.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,10 @@ func envMark(envName string) string {
7979
// Render renders the data of the document to HTML via the external tool.
8080
func (p *Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error {
8181
var (
82-
urlRawPrefix = strings.Replace(ctx.URLPrefix, "/src/", "/raw/", 1)
83-
command = strings.NewReplacer(envMark("GITEA_PREFIX_SRC"), ctx.URLPrefix,
84-
envMark("GITEA_PREFIX_RAW"), urlRawPrefix).Replace(p.Command)
82+
command = strings.NewReplacer(
83+
envMark("GITEA_PREFIX_SRC"), ctx.Links.SrcLink(),
84+
envMark("GITEA_PREFIX_RAW"), ctx.Links.RawLink(),
85+
).Replace(p.Command)
8586
commands = strings.Fields(command)
8687
args = commands[1:]
8788
)
@@ -121,14 +122,14 @@ func (p *Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.
121122
ctx.Ctx = graceful.GetManager().ShutdownContext()
122123
}
123124

124-
processCtx, _, finished := process.GetManager().AddContext(ctx.Ctx, fmt.Sprintf("Render [%s] for %s", commands[0], ctx.URLPrefix))
125+
processCtx, _, finished := process.GetManager().AddContext(ctx.Ctx, fmt.Sprintf("Render [%s] for %s", commands[0], ctx.Links.SrcLink()))
125126
defer finished()
126127

127128
cmd := exec.CommandContext(processCtx, commands[0], args...)
128129
cmd.Env = append(
129130
os.Environ(),
130-
"GITEA_PREFIX_SRC="+ctx.URLPrefix,
131-
"GITEA_PREFIX_RAW="+urlRawPrefix,
131+
"GITEA_PREFIX_SRC="+ctx.Links.SrcLink(),
132+
"GITEA_PREFIX_RAW="+ctx.Links.RawLink(),
132133
)
133134
if !p.IsInputFile {
134135
cmd.Stdin = input

‎modules/markup/html.go

Lines changed: 16 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -80,15 +80,10 @@ const keywordClass = "issue-keyword"
8080

8181
// IsLink reports whether link fits valid format.
8282
func IsLink(link []byte) bool {
83-
return isLink(link)
84-
}
85-
86-
// isLink reports whether link fits valid format.
87-
func isLink(link []byte) bool {
8883
return validLinksPattern.Match(link)
8984
}
9085

91-
func isLinkStr(link string) bool {
86+
func IsLinkStr(link string) bool {
9287
return validLinksPattern.MatchString(link)
9388
}
9489

@@ -344,7 +339,7 @@ func postProcess(ctx *RenderContext, procs []processor, input io.Reader, output
344339
node = node.FirstChild
345340
}
346341

347-
visitNode(ctx, procs, procs, node)
342+
visitNode(ctx, procs, node)
348343

349344
newNodes := make([]*html.Node, 0, 5)
350345

@@ -375,7 +370,7 @@ func postProcess(ctx *RenderContext, procs []processor, input io.Reader, output
375370
return nil
376371
}
377372

378-
func visitNode(ctx *RenderContext, procs, textProcs []processor, node *html.Node) {
373+
func visitNode(ctx *RenderContext, procs []processor, node *html.Node) {
379374
// Add user-content- to IDs and "#" links if they don't already have them
380375
for idx, attr := range node.Attr {
381376
val := strings.TrimPrefix(attr.Val, "#")
@@ -390,35 +385,29 @@ func visitNode(ctx *RenderContext, procs, textProcs []processor, node *html.Node
390385
}
391386

392387
if attr.Key == "class" && attr.Val == "emoji" {
393-
textProcs = nil
388+
procs = nil
394389
}
395390
}
396391

397392
// We ignore code and pre.
398393
switch node.Type {
399394
case html.TextNode:
400-
textNode(ctx, textProcs, node)
395+
textNode(ctx, procs, node)
401396
case html.ElementNode:
402397
if node.Data == "img" {
403398
for i, attr := range node.Attr {
404399
if attr.Key != "src" {
405400
continue
406401
}
407-
if len(attr.Val) > 0 && !isLinkStr(attr.Val) && !strings.HasPrefix(attr.Val, "data:image/") {
408-
prefix := ctx.URLPrefix
409-
if ctx.IsWiki {
410-
prefix = util.URLJoin(prefix, "wiki", "raw")
411-
}
412-
prefix = strings.Replace(prefix, "/src/", "/media/", 1)
413-
414-
attr.Val = util.URLJoin(prefix, attr.Val)
402+
if len(attr.Val) > 0 && !IsLinkStr(attr.Val) && !strings.HasPrefix(attr.Val, "data:image/") {
403+
attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsWiki), attr.Val)
415404
}
416405
attr.Val = camoHandleLink(attr.Val)
417406
node.Attr[i] = attr
418407
}
419408
} else if node.Data == "a" {
420409
// Restrict text in links to emojis
421-
textProcs = emojiProcessors
410+
procs = emojiProcessors
422411
} else if node.Data == "code" || node.Data == "pre" {
423412
return
424413
} else if node.Data == "i" {
@@ -444,7 +433,7 @@ func visitNode(ctx *RenderContext, procs, textProcs []processor, node *html.Node
444433
}
445434
}
446435
for n := node.FirstChild; n != nil; n = n.NextSibling {
447-
visitNode(ctx, procs, textProcs, n)
436+
visitNode(ctx, procs, n)
448437
}
449438
}
450439
// ignore everything else
@@ -641,10 +630,6 @@ func mentionProcessor(ctx *RenderContext, node *html.Node) {
641630
}
642631

643632
func shortLinkProcessor(ctx *RenderContext, node *html.Node) {
644-
shortLinkProcessorFull(ctx, node, false)
645-
}
646-
647-
func shortLinkProcessorFull(ctx *RenderContext, node *html.Node, noLink bool) {
648633
next := node.NextSibling
649634
for node != nil && node != next {
650635
m := shortLinkPattern.FindStringSubmatchIndex(node.Data)
@@ -665,7 +650,7 @@ func shortLinkProcessorFull(ctx *RenderContext, node *html.Node, noLink bool) {
665650
if equalPos := strings.IndexByte(v, '='); equalPos == -1 {
666651
// There is no equal in this argument; this is a mandatory arg
667652
if props["name"] == "" {
668-
if isLinkStr(v) {
653+
if IsLinkStr(v) {
669654
// If we clearly see it is a link, we save it so
670655

671656
// But first we need to ensure, that if both mandatory args provided
@@ -740,7 +725,7 @@ func shortLinkProcessorFull(ctx *RenderContext, node *html.Node, noLink bool) {
740725
DataAtom: atom.A,
741726
}
742727
childNode.Parent = linkNode
743-
absoluteLink := isLinkStr(link)
728+
absoluteLink := IsLinkStr(link)
744729
if !absoluteLink {
745730
if image {
746731
link = strings.ReplaceAll(link, " ", "+")
@@ -751,16 +736,9 @@ func shortLinkProcessorFull(ctx *RenderContext, node *html.Node, noLink bool) {
751736
link = url.PathEscape(link)
752737
}
753738
}
754-
urlPrefix := ctx.URLPrefix
755739
if image {
756740
if !absoluteLink {
757-
if IsSameDomain(urlPrefix) {
758-
urlPrefix = strings.Replace(urlPrefix, "/src/", "/raw/", 1)
759-
}
760-
if ctx.IsWiki {
761-
link = util.URLJoin("wiki", "raw", link)
762-
}
763-
link = util.URLJoin(urlPrefix, link)
741+
link = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsWiki), link)
764742
}
765743
title := props["title"]
766744
if title == "" {
@@ -789,18 +767,15 @@ func shortLinkProcessorFull(ctx *RenderContext, node *html.Node, noLink bool) {
789767
} else {
790768
if !absoluteLink {
791769
if ctx.IsWiki {
792-
link = util.URLJoin("wiki", link)
770+
link = util.URLJoin(ctx.Links.WikiLink(), link)
771+
} else {
772+
link = util.URLJoin(ctx.Links.SrcLink(), link)
793773
}
794-
link = util.URLJoin(urlPrefix, link)
795774
}
796775
childNode.Type = html.TextNode
797776
childNode.Data = name
798777
}
799-
if noLink {
800-
linkNode = childNode
801-
} else {
802-
linkNode.Attr = []html.Attribute{{Key: "href", Val: link}}
803-
}
778+
linkNode.Attr = []html.Attribute{{Key: "href", Val: link}}
804779
replaceContent(node, m[0], m[1], linkNode)
805780
node = node.NextSibling.NextSibling
806781
}

‎modules/markup/html_internal_test.go

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -287,8 +287,8 @@ func TestRender_IssueIndexPattern_Document(t *testing.T) {
287287
}
288288

289289
func testRenderIssueIndexPattern(t *testing.T, input, expected string, ctx *RenderContext) {
290-
if ctx.URLPrefix == "" {
291-
ctx.URLPrefix = TestAppURL
290+
if ctx.Links.Base == "" {
291+
ctx.Links.Base = TestRepoURL
292292
}
293293

294294
var buf strings.Builder
@@ -303,19 +303,23 @@ func TestRender_AutoLink(t *testing.T) {
303303
test := func(input, expected string) {
304304
var buffer strings.Builder
305305
err := PostProcess(&RenderContext{
306-
Ctx: git.DefaultContext,
307-
URLPrefix: TestRepoURL,
308-
Metas: localMetas,
306+
Ctx: git.DefaultContext,
307+
Links: Links{
308+
Base: TestRepoURL,
309+
},
310+
Metas: localMetas,
309311
}, strings.NewReader(input), &buffer)
310312
assert.Equal(t, err, nil)
311313
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer.String()))
312314

313315
buffer.Reset()
314316
err = PostProcess(&RenderContext{
315-
Ctx: git.DefaultContext,
316-
URLPrefix: TestRepoURL,
317-
Metas: localMetas,
318-
IsWiki: true,
317+
Ctx: git.DefaultContext,
318+
Links: Links{
319+
Base: TestRepoURL,
320+
},
321+
Metas: localMetas,
322+
IsWiki: true,
319323
}, strings.NewReader(input), &buffer)
320324
assert.Equal(t, err, nil)
321325
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer.String()))
@@ -342,9 +346,11 @@ func TestRender_FullIssueURLs(t *testing.T) {
342346
test := func(input, expected string) {
343347
var result strings.Builder
344348
err := postProcess(&RenderContext{
345-
Ctx: git.DefaultContext,
346-
URLPrefix: TestRepoURL,
347-
Metas: localMetas,
349+
Ctx: git.DefaultContext,
350+
Links: Links{
351+
Base: TestRepoURL,
352+
},
353+
Metas: localMetas,
348354
}, []processor{fullIssuePatternProcessor}, strings.NewReader(input), &result)
349355
assert.NoError(t, err)
350356
assert.Equal(t, expected, result.String())

0 commit comments

Comments
 (0)