From 5b6b3f3fb3d69d3669bf65713d3df3ee8f09d27b Mon Sep 17 00:00:00 2001 From: Mai-Lapyst Date: Wed, 17 Apr 2024 12:44:17 +0200 Subject: [PATCH 1/3] Fix some edge cases; closes #3232 - Fixes wrong usage of AppURL - Fixes wrong rendering with extra path segments when AppSubURL is empty - Now also renders all links when 2+ permalinks are present --- modules/markup/file_preview.go | 8 +- modules/markup/html.go | 19 +-- modules/markup/html_test.go | 243 ++++++++++++++++++++++++--------- 3 files changed, 191 insertions(+), 79 deletions(-) diff --git a/modules/markup/file_preview.go b/modules/markup/file_preview.go index a37007d75b..75590b899b 100644 --- a/modules/markup/file_preview.go +++ b/modules/markup/file_preview.go @@ -56,11 +56,11 @@ func NewFilePreview(ctx *RenderContext, node *html.Node, locale translation.Loca urlFull := node.Data[m[0]:m[1]] // Ensure that we only use links to local repositories - if !strings.HasPrefix(urlFull, setting.AppURL+setting.AppSubURL) { + if !strings.HasPrefix(urlFull, setting.AppURL) { return nil } - projPath := strings.TrimSuffix(node.Data[m[2]:m[3]], "/") + projPath := strings.TrimPrefix(strings.TrimSuffix(node.Data[m[0]:m[3]], "/"), setting.AppURL) commitSha := node.Data[m[4]:m[5]] filePath := node.Data[m[6]:m[7]] @@ -70,6 +70,10 @@ func NewFilePreview(ctx *RenderContext, node *html.Node, locale translation.Loca preview.end = m[1] projPathSegments := strings.Split(projPath, "/") + if len(projPathSegments) != 2 { + return nil + } + ownerName := projPathSegments[len(projPathSegments)-2] repoName := projPathSegments[len(projPathSegments)-1] diff --git a/modules/markup/html.go b/modules/markup/html.go index d19eb37013..e177fa3d74 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -1063,19 +1063,20 @@ func filePreviewPatternProcessor(ctx *RenderContext, node *html.Node) { return } + locale := translation.NewLocale("en-US") + if ctx.Ctx != nil { + ctxLocale, ok := ctx.Ctx.Value(translation.ContextKey).(translation.Locale) + if ok { + locale = ctxLocale + } + } + next := node.NextSibling for node != nil && node != next { - locale := translation.NewLocale("en-US") - if ctx.Ctx != nil { - ctxLocale, ok := ctx.Ctx.Value(translation.ContextKey).(translation.Locale) - if ok { - locale = ctxLocale - } - } - preview := NewFilePreview(ctx, node, locale) if preview == nil { - return + node = node.NextSibling + continue } previewNode := preview.CreateHTML(locale) diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go index 2c5ed8c7b8..bbf4f7d9e1 100644 --- a/modules/markup/html_test.go +++ b/modules/markup/html_test.go @@ -17,6 +17,7 @@ import ( "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/markup/markdown" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/translation" "code.gitea.io/gitea/modules/util" @@ -680,9 +681,9 @@ func TestIssue18471(t *testing.T) { } func TestRender_FilePreview(t *testing.T) { - setting.StaticRootPath = "../../" - setting.Names = []string{"english"} - setting.Langs = []string{"en-US"} + defer test.MockVariableValue(&setting.StaticRootPath, "../../")() + defer test.MockVariableValue(&setting.Names, []string{"english"})() + defer test.MockVariableValue(&setting.Langs, []string{"en-US"})() translation.InitLocales(context.Background()) setting.AppURL = markup.TestAppURL @@ -705,7 +706,7 @@ func TestRender_FilePreview(t *testing.T) { sha := "190d9492934af498c3f669d6a2431dc5459e5b20" commitFilePreview := util.URLJoin(markup.TestRepoURL, "src", "commit", sha, "path", "to", "file.go") + "#L2-L3" - test := func(input, expected string, metas map[string]string) { + testRender := func(input, expected string, metas map[string]string) { buffer, err := markup.RenderString(&markup.RenderContext{ Ctx: git.DefaultContext, RelativePath: ".md", @@ -715,69 +716,175 @@ func TestRender_FilePreview(t *testing.T) { assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) } - test( - commitFilePreview, - `

`+ - `
`+ - `
`+ - `
`+ - `path/to/file.go`+ - `
`+ - ``+ - `Lines 2 to 3 in 190d949`+ - ``+ - `
`+ - `
`+ - ``+ - ``+ - ``+ - ``+ - ``+ - ``+ - ``+ - ``+ - ``+ - ``+ - ``+ - `
B`+"\n"+`
C`+"\n"+`
`+ - `
`+ - `
`+ - `

`, - localMetas, - ) + t.Run("single", func(t *testing.T) { + testRender( + commitFilePreview, + `

`+ + `
`+ + `
`+ + `
`+ + `path/to/file.go`+ + `
`+ + ``+ + `Lines 2 to 3 in 190d949`+ + ``+ + `
`+ + `
`+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + `
B`+"\n"+`
C`+"\n"+`
`+ + `
`+ + `
`+ + `

`, + localMetas, + ) + }) - test( - commitFilePreview, - `

`+ - `
`+ - `
`+ - `
`+ - `gogits/gogs – `+ - `path/to/file.go`+ - `
`+ - ``+ - `Lines 2 to 3 in gogits/gogs@190d949`+ - ``+ - `
`+ - `
`+ - ``+ - ``+ - ``+ - ``+ - ``+ - ``+ - ``+ - ``+ - ``+ - ``+ - ``+ - `
B`+"\n"+`
C`+"\n"+`
`+ - `
`+ - `
`+ - `

`, - map[string]string{ - "user": "gogits", - "repo": "gogs2", - }, - ) + t.Run("cross-repo", func(t *testing.T) { + testRender( + commitFilePreview, + `

`+ + `
`+ + `
`+ + `
`+ + `gogits/gogs – `+ + `path/to/file.go`+ + `
`+ + ``+ + `Lines 2 to 3 in gogits/gogs@190d949`+ + ``+ + `
`+ + `
`+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + `
B`+"\n"+`
C`+"\n"+`
`+ + `
`+ + `
`+ + `

`, + map[string]string{ + "user": "gogits", + "repo": "gogs2", + }, + ) + }) + + t.Run("AppSubURL", func(t *testing.T) { + urlWithSub := util.URLJoin(markup.TestAppURL, "sub", markup.TestOrgRepo, "src", "commit", sha, "path", "to", "file.go") + "#L2-L3" + + testRender( + urlWithSub, + `

190d949293/path/to/file.go (L2-L3)

`, + localMetas, + ) + + defer test.MockVariableValue(&setting.AppURL, markup.TestAppURL+"sub/")() + defer test.MockVariableValue(&setting.AppSubURL, "/sub")() + + testRender( + urlWithSub, + `

`+ + `
`+ + `
`+ + `
`+ + `path/to/file.go`+ + `
`+ + ``+ + `Lines 2 to 3 in 190d949`+ + ``+ + `
`+ + `
`+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + `
B`+"\n"+`
C`+"\n"+`
`+ + `
`+ + `
`+ + `

`, + localMetas, + ) + }) + + t.Run("multiples", func(t *testing.T) { + testRender( + "first "+commitFilePreview+" second "+commitFilePreview, + `

first

`+ + `
`+ + `
`+ + `
`+ + `path/to/file.go`+ + `
`+ + ``+ + `Lines 2 to 3 in 190d949`+ + ``+ + `
`+ + `
`+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + `
B`+"\n"+`
C`+"\n"+`
`+ + `
`+ + `
`+ + `

second

`+ + `
`+ + `
`+ + `
`+ + `path/to/file.go`+ + `
`+ + ``+ + `Lines 2 to 3 in 190d949`+ + ``+ + `
`+ + `
`+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + `
B`+"\n"+`
C`+"\n"+`
`+ + `
`+ + `
`+ + `

`, + localMetas, + ) + }) } From e9eacdecd20602887adc5ad254bcc4678ee1a94d Mon Sep 17 00:00:00 2001 From: Mai-Lapyst Date: Fri, 19 Apr 2024 18:21:21 +0200 Subject: [PATCH 2/3] Fix issue where rendering stops after the first invalid parmalink --- modules/markup/file_preview.go | 34 +++++++++++++++++++---------- modules/markup/html.go | 40 +++++++++++++++++++--------------- modules/markup/html_test.go | 31 ++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 28 deletions(-) diff --git a/modules/markup/file_preview.go b/modules/markup/file_preview.go index 75590b899b..993df717e1 100644 --- a/modules/markup/file_preview.go +++ b/modules/markup/file_preview.go @@ -35,24 +35,36 @@ type FilePreview struct { isTruncated bool } -func NewFilePreview(ctx *RenderContext, node *html.Node, locale translation.Locale) *FilePreview { +func NewFilePreviews(ctx *RenderContext, node *html.Node, locale translation.Locale) []*FilePreview { if setting.FilePreviewMaxLines == 0 { // Feature is disabled return nil } + mAll := filePreviewPattern.FindAllStringSubmatchIndex(node.Data, -1) + if mAll == nil { + return nil + } + + result := make([]*FilePreview, 0) + + for _, m := range mAll { + if slices.Contains(m, -1) { + continue + } + + preview := newFilePreview(ctx, node, locale, m) + if preview != nil { + result = append(result, preview) + } + } + + return result +} + +func newFilePreview(ctx *RenderContext, node *html.Node, locale translation.Locale, m []int) *FilePreview { preview := &FilePreview{} - m := filePreviewPattern.FindStringSubmatchIndex(node.Data) - if m == nil { - return nil - } - - // Ensure that every group has a match - if slices.Contains(m, -1) { - return nil - } - urlFull := node.Data[m[0]:m[1]] // Ensure that we only use links to local repositories diff --git a/modules/markup/html.go b/modules/markup/html.go index e177fa3d74..8851558286 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -1073,28 +1073,34 @@ func filePreviewPatternProcessor(ctx *RenderContext, node *html.Node) { next := node.NextSibling for node != nil && node != next { - preview := NewFilePreview(ctx, node, locale) - if preview == nil { + previews := NewFilePreviews(ctx, node, locale) + if previews == nil { node = node.NextSibling continue } - previewNode := preview.CreateHTML(locale) + offset := 0 + for _, preview := range previews { + previewNode := preview.CreateHTML(locale) - // Specialized version of replaceContent, so the parent paragraph element is not destroyed from our div - before := node.Data[:preview.start] - after := node.Data[preview.end:] - node.Data = before - nextSibling := node.NextSibling - node.Parent.InsertBefore(&html.Node{ - Type: html.RawNode, - Data: "

", - }, nextSibling) - node.Parent.InsertBefore(previewNode, nextSibling) - node.Parent.InsertBefore(&html.Node{ - Type: html.RawNode, - Data: "

" + after, - }, nextSibling) + // Specialized version of replaceContent, so the parent paragraph element is not destroyed from our div + before := node.Data[:(preview.start - offset)] + after := node.Data[(preview.end - offset):] + offset += preview.end - 3 + node.Data = before + nextSibling := node.NextSibling + node.Parent.InsertBefore(&html.Node{ + Type: html.RawNode, + Data: "

", + }, nextSibling) + node.Parent.InsertBefore(previewNode, nextSibling) + afterNode := &html.Node{ + Type: html.RawNode, + Data: "

" + after, + } + node.Parent.InsertBefore(afterNode, nextSibling) + node = afterNode + } node = node.NextSibling } diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go index bbf4f7d9e1..1b8a92e51f 100644 --- a/modules/markup/html_test.go +++ b/modules/markup/html_test.go @@ -828,6 +828,37 @@ func TestRender_FilePreview(t *testing.T) { `

`, localMetas, ) + + testRender( + "first without sub "+commitFilePreview+" second "+urlWithSub, + `

first without sub 190d949293/path/to/file.go (L2-L3) second

`+ + `
`+ + `
`+ + `
`+ + `path/to/file.go`+ + `
`+ + ``+ + `Lines 2 to 3 in 190d949`+ + ``+ + `
`+ + `
`+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + `
B`+"\n"+`
C`+"\n"+`
`+ + `
`+ + `
`+ + `

`, + localMetas, + ) }) t.Run("multiples", func(t *testing.T) { From acfae43253d4f0bb637552bfab7d6fbc57013ed9 Mon Sep 17 00:00:00 2001 From: Mai-Lapyst Date: Fri, 19 Apr 2024 23:54:46 +0200 Subject: [PATCH 3/3] Fix panic where now a third link breaks everything --- modules/markup/html.go | 5 ++- modules/markup/html_test.go | 81 +++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/modules/markup/html.go b/modules/markup/html.go index 8851558286..b4fd008aa7 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -1086,7 +1086,8 @@ func filePreviewPatternProcessor(ctx *RenderContext, node *html.Node) { // Specialized version of replaceContent, so the parent paragraph element is not destroyed from our div before := node.Data[:(preview.start - offset)] after := node.Data[(preview.end - offset):] - offset += preview.end - 3 + afterPrefix := "

" + offset = preview.end - len(afterPrefix) node.Data = before nextSibling := node.NextSibling node.Parent.InsertBefore(&html.Node{ @@ -1096,7 +1097,7 @@ func filePreviewPatternProcessor(ctx *RenderContext, node *html.Node) { node.Parent.InsertBefore(previewNode, nextSibling) afterNode := &html.Node{ Type: html.RawNode, - Data: "

" + after, + Data: afterPrefix + after, } node.Parent.InsertBefore(afterNode, nextSibling) node = afterNode diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go index 1b8a92e51f..a1a99c1a7f 100644 --- a/modules/markup/html_test.go +++ b/modules/markup/html_test.go @@ -917,5 +917,86 @@ func TestRender_FilePreview(t *testing.T) { `

`, localMetas, ) + + testRender( + "first "+commitFilePreview+" second "+commitFilePreview+" third "+commitFilePreview, + `

first

`+ + `
`+ + `
`+ + `
`+ + `path/to/file.go`+ + `
`+ + ``+ + `Lines 2 to 3 in 190d949`+ + ``+ + `
`+ + `
`+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + `
B`+"\n"+`
C`+"\n"+`
`+ + `
`+ + `
`+ + `

second

`+ + `
`+ + `
`+ + `
`+ + `path/to/file.go`+ + `
`+ + ``+ + `Lines 2 to 3 in 190d949`+ + ``+ + `
`+ + `
`+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + `
B`+"\n"+`
C`+"\n"+`
`+ + `
`+ + `
`+ + `

third

`+ + `
`+ + `
`+ + `
`+ + `path/to/file.go`+ + `
`+ + ``+ + `Lines 2 to 3 in 190d949`+ + ``+ + `
`+ + `
`+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + ``+ + `
B`+"\n"+`
C`+"\n"+`
`+ + `
`+ + `
`+ + `

`, + localMetas, + ) }) }