From 597d1da96b92b181c106813ce26149334b2b44e5 Mon Sep 17 00:00:00 2001 From: Brecht Van Lommel Date: Mon, 17 Jun 2024 08:16:14 +0200 Subject: [PATCH] Fix missing images in editor preview due to wrong links (#31299) Parse base path and tree path so that media links can be correctly created with /media/. Resolves #31294 --------- Co-authored-by: wxiaoguang --- modules/markup/renderer.go | 8 +-- modules/structs/miscellaneous.go | 6 +- routers/api/v1/misc/markup_test.go | 100 ++++++++++++++++++----------- routers/common/markup.go | 63 +++++++++--------- templates/swagger/v1_json.tmpl | 4 +- 5 files changed, 103 insertions(+), 78 deletions(-) diff --git a/modules/markup/renderer.go b/modules/markup/renderer.go index 66e8cf611..5eb568cb1 100644 --- a/modules/markup/renderer.go +++ b/modules/markup/renderer.go @@ -86,10 +86,10 @@ type RenderContext struct { } type Links struct { - AbsolutePrefix bool - Base string - BranchPath string - TreePath string + AbsolutePrefix bool // add absolute URL prefix to auto-resolved links like "#issue", but not for pre-provided links and medias + Base string // base prefix for pre-provided links and medias (images, videos) + BranchPath string // actually it is the ref path, eg: "branch/features/feat-12", "tag/v1.0" + TreePath string // the dir of the file, eg: "doc" if the file "doc/CHANGE.md" is being rendered } func (l *Links) Prefix() string { diff --git a/modules/structs/miscellaneous.go b/modules/structs/miscellaneous.go index bff10f95b..3b206c1dd 100644 --- a/modules/structs/miscellaneous.go +++ b/modules/structs/miscellaneous.go @@ -25,7 +25,8 @@ type MarkupOption struct { // // in: body Mode string - // Context to render + // URL path for rendering issue, media and file links + // Expected format: /subpath/{user}/{repo}/src/{branch, commit, tag}/{identifier/path}/{file/dir} // // in: body Context string @@ -53,7 +54,8 @@ type MarkdownOption struct { // // in: body Mode string - // Context to render + // URL path for rendering issue, media and file links + // Expected format: /subpath/{user}/{repo}/src/{branch, commit, tag}/{identifier/path}/{file/dir} // // in: body Context string diff --git a/routers/api/v1/misc/markup_test.go b/routers/api/v1/misc/markup_test.go index 5236fd06a..e2ab7141b 100644 --- a/routers/api/v1/misc/markup_test.go +++ b/routers/api/v1/misc/markup_test.go @@ -7,6 +7,7 @@ import ( go_context "context" "io" "net/http" + "path" "strings" "testing" @@ -19,36 +20,40 @@ import ( "github.com/stretchr/testify/assert" ) -const ( - AppURL = "http://localhost:3000/" - Repo = "gogits/gogs" - FullURL = AppURL + Repo + "/" -) +const AppURL = "http://localhost:3000/" -func testRenderMarkup(t *testing.T, mode, filePath, text, responseBody string, responseCode int) { +func testRenderMarkup(t *testing.T, mode string, wiki bool, filePath, text, expectedBody string, expectedCode int) { setting.AppURL = AppURL + context := "/gogits/gogs" + if !wiki { + context += path.Join("/src/branch/main", path.Dir(filePath)) + } options := api.MarkupOption{ Mode: mode, Text: text, - Context: Repo, - Wiki: true, + Context: context, + Wiki: wiki, FilePath: filePath, } ctx, resp := contexttest.MockAPIContext(t, "POST /api/v1/markup") web.SetForm(ctx, &options) Markup(ctx) - assert.Equal(t, responseBody, resp.Body.String()) - assert.Equal(t, responseCode, resp.Code) + assert.Equal(t, expectedBody, resp.Body.String()) + assert.Equal(t, expectedCode, resp.Code) resp.Body.Reset() } -func testRenderMarkdown(t *testing.T, mode, text, responseBody string, responseCode int) { +func testRenderMarkdown(t *testing.T, mode string, wiki bool, text, responseBody string, responseCode int) { setting.AppURL = AppURL + context := "/gogits/gogs" + if !wiki { + context += "/src/branch/main" + } options := api.MarkdownOption{ Mode: mode, Text: text, - Context: Repo, - Wiki: true, + Context: context, + Wiki: wiki, } ctx, resp := contexttest.MockAPIContext(t, "POST /api/v1/markdown") web.SetForm(ctx, &options) @@ -65,7 +70,7 @@ func TestAPI_RenderGFM(t *testing.T) { }, }) - testCasesCommon := []string{ + testCasesWiki := []string{ // dear imgui wiki markdown extract: special wiki syntax `Wiki! Enjoy :) - [[Links, Language bindings, Engine bindings|Links]] @@ -74,20 +79,20 @@ func TestAPI_RenderGFM(t *testing.T) { // rendered `

Wiki! Enjoy :)

`, // Guard wiki sidebar: special syntax `[[Guardfile-DSL / Configuring-Guard|Guardfile-DSL---Configuring-Guard]]`, // rendered - `

Guardfile-DSL / Configuring-Guard

+ `

Guardfile-DSL / Configuring-Guard

`, // special syntax `[[Name|Link]]`, // rendered - `

Name

+ `

Name

`, // empty ``, @@ -95,7 +100,7 @@ func TestAPI_RenderGFM(t *testing.T) { ``, } - testCasesDocument := []string{ + testCasesWikiDocument := []string{ // wine-staging wiki home extract: special wiki syntax, images `## What is Wine Staging? **Wine Staging** on website [wine-staging.com](http://wine-staging.com). @@ -111,31 +116,48 @@ Here are some links to the most important topics. You can find the full list of

Wine Staging on website wine-staging.com.

Quick Links

Here are some links to the most important topics. You can find the full list of pages at the sidebar.

-

Configuration -images/icon-bug.png

+

Configuration +images/icon-bug.png

`, } - for i := 0; i < len(testCasesCommon); i += 2 { - text := testCasesCommon[i] - response := testCasesCommon[i+1] - testRenderMarkdown(t, "gfm", text, response, http.StatusOK) - testRenderMarkup(t, "gfm", "", text, response, http.StatusOK) - testRenderMarkdown(t, "comment", text, response, http.StatusOK) - testRenderMarkup(t, "comment", "", text, response, http.StatusOK) - testRenderMarkup(t, "file", "path/test.md", text, response, http.StatusOK) + for i := 0; i < len(testCasesWiki); i += 2 { + text := testCasesWiki[i] + response := testCasesWiki[i+1] + testRenderMarkdown(t, "gfm", true, text, response, http.StatusOK) + testRenderMarkup(t, "gfm", true, "", text, response, http.StatusOK) + testRenderMarkdown(t, "comment", true, text, response, http.StatusOK) + testRenderMarkup(t, "comment", true, "", text, response, http.StatusOK) + testRenderMarkup(t, "file", true, "path/test.md", text, response, http.StatusOK) } - for i := 0; i < len(testCasesDocument); i += 2 { - text := testCasesDocument[i] - response := testCasesDocument[i+1] - testRenderMarkdown(t, "gfm", text, response, http.StatusOK) - testRenderMarkup(t, "gfm", "", text, response, http.StatusOK) - testRenderMarkup(t, "file", "path/test.md", text, response, http.StatusOK) + for i := 0; i < len(testCasesWikiDocument); i += 2 { + text := testCasesWikiDocument[i] + response := testCasesWikiDocument[i+1] + testRenderMarkdown(t, "gfm", true, text, response, http.StatusOK) + testRenderMarkup(t, "gfm", true, "", text, response, http.StatusOK) + testRenderMarkup(t, "file", true, "path/test.md", text, response, http.StatusOK) } - testRenderMarkup(t, "file", "path/test.unknown", "## Test", "Unsupported render extension: .unknown\n", http.StatusUnprocessableEntity) - testRenderMarkup(t, "unknown", "", "## Test", "Unknown mode: unknown\n", http.StatusUnprocessableEntity) + input := "[Link](test.md)\n![Image](image.png)" + testRenderMarkdown(t, "gfm", false, input, `

Link +Image

+`, http.StatusOK) + + testRenderMarkdown(t, "gfm", false, input, `

Link +Image

+`, http.StatusOK) + + testRenderMarkup(t, "gfm", false, "", input, `

Link +Image

+`, http.StatusOK) + + testRenderMarkup(t, "file", false, "path/new-file.md", input, `

Link +Image

+`, http.StatusOK) + + testRenderMarkup(t, "file", true, "path/test.unknown", "## Test", "Unsupported render extension: .unknown\n", http.StatusUnprocessableEntity) + testRenderMarkup(t, "unknown", true, "", "## Test", "Unknown mode: unknown\n", http.StatusUnprocessableEntity) } var simpleCases = []string{ @@ -160,7 +182,7 @@ func TestAPI_RenderSimple(t *testing.T) { options := api.MarkdownOption{ Mode: "markdown", Text: "", - Context: Repo, + Context: "/gogits/gogs", } ctx, resp := contexttest.MockAPIContext(t, "POST /api/v1/markdown") for i := 0; i < len(simpleCases); i += 2 { diff --git a/routers/common/markup.go b/routers/common/markup.go index f7d096008..0a00eac7d 100644 --- a/routers/common/markup.go +++ b/routers/common/markup.go @@ -7,63 +7,67 @@ package common import ( "fmt" "net/http" + "path" "strings" repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/modules/httplib" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/markup/markdown" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/services/context" - - "mvdan.cc/xurls/v2" ) // RenderMarkup renders markup text for the /markup and /markdown endpoints -func RenderMarkup(ctx *context.Base, repo *context.Repository, mode, text, urlPrefix, filePath string, wiki bool) { - var markupType string - relativePath := "" +func RenderMarkup(ctx *context.Base, repo *context.Repository, mode, text, urlPathContext, filePath string, wiki bool) { + // urlPathContext format is "/subpath/{user}/{repo}/src/{branch, commit, tag}/{identifier/path}/{file/dir}" + // filePath is the path of the file to render if the end user is trying to preview a repo file (mode == "file") + // filePath will be used as RenderContext.RelativePath - if len(text) == 0 { - _, _ = ctx.Write([]byte("")) - return + // for example, when previewing file "/gitea/owner/repo/src/branch/features/feat-123/doc/CHANGE.md", then filePath is "doc/CHANGE.md" + // and the urlPathContext is "/gitea/owner/repo/src/branch/features/feat-123/doc" + + var markupType, relativePath string + + links := markup.Links{AbsolutePrefix: true} + if urlPathContext != "" { + links.Base = fmt.Sprintf("%s%s", httplib.GuessCurrentHostURL(ctx), urlPathContext) } switch mode { case "markdown": // Raw markdown if err := markdown.RenderRaw(&markup.RenderContext{ - Ctx: ctx, - Links: markup.Links{ - AbsolutePrefix: true, - Base: urlPrefix, - }, + Ctx: ctx, + Links: links, }, strings.NewReader(text), ctx.Resp); err != nil { ctx.Error(http.StatusInternalServerError, err.Error()) } return case "comment": - // Comment as markdown + // Issue & comment content markupType = markdown.MarkupName case "gfm": - // Github Flavored Markdown as document + // GitHub Flavored Markdown markupType = markdown.MarkupName case "file": - // File as document based on file extension - markupType = "" + markupType = "" // render the repo file content by its extension relativePath = filePath default: ctx.Error(http.StatusUnprocessableEntity, fmt.Sprintf("Unknown mode: %s", mode)) return } - if !strings.HasPrefix(setting.AppSubURL+"/", urlPrefix) { - // check if urlPrefix is already set to a URL - linkRegex, _ := xurls.StrictMatchingScheme("https?://") - m := linkRegex.FindStringIndex(urlPrefix) - if m == nil { - urlPrefix = util.URLJoin(setting.AppURL, urlPrefix) - } + fields := strings.SplitN(strings.TrimPrefix(urlPathContext, setting.AppSubURL+"/"), "/", 5) + if len(fields) == 5 && fields[2] == "src" && (fields[3] == "branch" || fields[3] == "commit" || fields[3] == "tag") { + // absolute base prefix is something like "https://host/subpath/{user}/{repo}" + absoluteBasePrefix := fmt.Sprintf("%s%s/%s", httplib.GuessCurrentAppURL(ctx), fields[0], fields[1]) + + fileDir := path.Dir(filePath) // it is "doc" if filePath is "doc/CHANGE.md" + refPath := strings.Join(fields[3:], "/") // it is "branch/features/feat-12/doc" + refPath = strings.TrimSuffix(refPath, "/"+fileDir) // now we get the correct branch path: "branch/features/feat-12" + + links = markup.Links{AbsolutePrefix: true, Base: absoluteBasePrefix, BranchPath: refPath, TreePath: fileDir} } meta := map[string]string{} @@ -81,12 +85,9 @@ func RenderMarkup(ctx *context.Base, repo *context.Repository, mode, text, urlPr } if err := markup.Render(&markup.RenderContext{ - Ctx: ctx, - Repo: repoCtx, - Links: markup.Links{ - AbsolutePrefix: true, - Base: urlPrefix, - }, + Ctx: ctx, + Repo: repoCtx, + Links: links, Metas: meta, IsWiki: wiki, Type: markupType, diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index ebfdcb6a8..4aa64c537 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -22404,7 +22404,7 @@ "type": "object", "properties": { "Context": { - "description": "Context to render\n\nin: body", + "description": "URL path for rendering issue, media and file links\nExpected format: /subpath/{user}/{repo}/src/{branch, commit, tag}/{identifier/path}/{file/dir}\n\nin: body", "type": "string" }, "Mode": { @@ -22427,7 +22427,7 @@ "type": "object", "properties": { "Context": { - "description": "Context to render\n\nin: body", + "description": "URL path for rendering issue, media and file links\nExpected format: /subpath/{user}/{repo}/src/{branch, commit, tag}/{identifier/path}/{file/dir}\n\nin: body", "type": "string" }, "FilePath": {