From 53d7d3f053cd0c163dc55531e571e7db0b343bf6 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 13 Aug 2026 03:30:26 +0800 Subject: [PATCH] refactor: external render (#38885) make the "command variable replacement" more accurate and OS-independent, add a test for it. --- modules/markup/external/external.go | 55 ++++++++++++++---------- modules/markup/external/external_test.go | 24 +++++++++++ routers/common/markup.go | 1 + routers/web/org/home.go | 3 +- routers/web/user/profile.go | 4 +- 5 files changed, 60 insertions(+), 27 deletions(-) create mode 100644 modules/markup/external/external_test.go diff --git a/modules/markup/external/external.go b/modules/markup/external/external.go index dc6633dff62..0ff010c6ad5 100644 --- a/modules/markup/external/external.go +++ b/modules/markup/external/external.go @@ -5,11 +5,11 @@ package external import ( "bytes" + "errors" "fmt" "io" "os" "os/exec" - "runtime" "strings" "gitea.dev/modules/markup" @@ -91,52 +91,63 @@ func (p *Renderer) GetExternalRendererOptions() (ret markup.ExternalRendererOpti return ret } -func envMark(envName string) string { - if runtime.GOOS == "windows" { - return "%" + envName + "%" +func (p *Renderer) prepareExternalCommand(vars map[string]string) (string, []string, error) { + fields, err := shellquote.Split(strings.TrimSpace(p.Command)) + if err != nil { + return "", nil, err } - return "$" + envName + if len(fields) == 0 { + return "", nil, errors.New("no command") + } + var replacements []string + for k, v := range vars { + replacements = append(replacements, "$"+k, v) + replacements = append(replacements, "%"+k+"%", v) // for legacy Windows-style support + } + r := strings.NewReplacer(replacements...) + for i := range fields { + fields[i] = r.Replace(fields[i]) + } + return fields[0], fields[1:], nil } // Render renders the data of the document to HTML via the external tool. func (p *Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error { baseLinkSrc := ctx.RenderHelper.ResolveLink("", markup.LinkTypeDefault) baseLinkRaw := ctx.RenderHelper.ResolveLink("", markup.LinkTypeRaw) - command := strings.NewReplacer( - envMark("GITEA_PREFIX_SRC"), baseLinkSrc, - envMark("GITEA_PREFIX_RAW"), baseLinkRaw, - ).Replace(p.Command) - commands, err := shellquote.Split(command) - if err != nil || len(commands) == 0 { - return fmt.Errorf("%s invalid command %q: %w", p.Name(), p.Command, err) + cmdVars := map[string]string{ + "GITEA_PREFIX_SRC": baseLinkSrc, + "GITEA_PREFIX_RAW": baseLinkRaw, + } + cmdProg, cmdArgs, err := p.prepareExternalCommand(cmdVars) + if err != nil { + return fmt.Errorf("invalid external render (%s) command %q: %w", p.Name(), p.Command, err) } - args := commands[1:] - if p.IsInputFile { // write to temp file - f, cleanup, err := setting.AppDataTempDir("git-repo-content").CreateTempFileRandom("gitea_input") + tmpFile, cleanup, err := setting.AppDataTempDir("git-repo-content").CreateTempFileRandom("gitea_input") if err != nil { return fmt.Errorf("%s create temp file when rendering %s failed: %w", p.Name(), p.Command, err) } defer cleanup() - _, err = io.Copy(f, input) + _, err = io.Copy(tmpFile, input) if err != nil { - _ = f.Close() + _ = tmpFile.Close() return fmt.Errorf("%s write data to temp file when rendering %s failed: %w", p.Name(), p.Command, err) } - err = f.Close() + err = tmpFile.Close() if err != nil { return fmt.Errorf("%s close temp file when rendering %s failed: %w", p.Name(), p.Command, err) } - args = append(args, f.Name()) + cmdArgs = append(cmdArgs, tmpFile.Name()) } - processCtx, _, finished := process.GetManager().AddContext(ctx, fmt.Sprintf("Render [%s] for %s", commands[0], baseLinkSrc)) + processCtx, _, finished := process.GetManager().AddContext(ctx, fmt.Sprintf("Render [%s] for %s", cmdProg, baseLinkSrc)) defer finished() - cmd := exec.CommandContext(processCtx, commands[0], args...) + cmd := exec.CommandContext(processCtx, cmdProg, cmdArgs...) cmd.Env = append( os.Environ(), "GITEA_PREFIX_SRC="+baseLinkSrc, @@ -151,7 +162,7 @@ func (p *Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io. process.SetSysProcAttribute(cmd) if err := cmd.Run(); err != nil { - return fmt.Errorf("%s render run command %s %v failed: %w\nStderr: %s", p.Name(), commands[0], args, err, stderr.String()) + return fmt.Errorf("%s render run command %s %v failed: %w\nStderr: %s", p.Name(), cmdProg, shellquote.Join(cmdArgs...), err, stderr.String()) } return nil } diff --git a/modules/markup/external/external_test.go b/modules/markup/external/external_test.go new file mode 100644 index 00000000000..19b1bf378cc --- /dev/null +++ b/modules/markup/external/external_test.go @@ -0,0 +1,24 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package external + +import ( + "testing" + + "gitea.dev/modules/setting" + + "github.com/stretchr/testify/assert" +) + +func TestPrepareExternalCommand(t *testing.T) { + r := &Renderer{MarkupRenderer: &setting.MarkupRenderer{Command: ""}} + _, _, err := r.prepareExternalCommand(map[string]string{"KEY": "val"}) + assert.ErrorContains(t, err, "no command") + + r = &Renderer{MarkupRenderer: &setting.MarkupRenderer{Command: `"/foo bar/bin" --opt $KEY "$KEY" %KEY% other`}} + prog, args, err := r.prepareExternalCommand(map[string]string{"KEY": `a"b`}) + assert.NoError(t, err) + assert.Equal(t, "/foo bar/bin", prog) + assert.Equal(t, []string{"--opt", `a"b`, `a"b`, `a"b`, "other"}, args) +} diff --git a/routers/common/markup.go b/routers/common/markup.go index a379deda3ea..cf3c799e901 100644 --- a/routers/common/markup.go +++ b/routers/common/markup.go @@ -64,6 +64,7 @@ func RenderMarkup(ctx *context.Base, ctxRepo *context.Repository, mode, text, ur treePath = 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, "/"+treePath) // now we get the correct branch path: "branch/features/feat-12" + refPath = util.PathEscapeSegments(refPath) } else if fields = strings.SplitN(repoLinkPath, "/", 3); len(fields) == 2 { repoOwnerName, repoName = fields[0], fields[1] } diff --git a/routers/web/org/home.go b/routers/web/org/home.go index 19b80fa0933..b4fdf28fb9b 100644 --- a/routers/web/org/home.go +++ b/routers/web/org/home.go @@ -5,7 +5,6 @@ package org import ( "net/http" - "path" "strings" "gitea.dev/models/db" @@ -201,7 +200,7 @@ func prepareOrgProfileReadme(ctx *context.Context, prepareResult *shared_user.Pr } rctx := renderhelper.NewRenderContextRepoFile(ctx, profileRepo, renderhelper.RepoFileOptions{ - CurrentRefSubURL: path.Join("branch", util.PathEscapeSegments(profileRepo.DefaultBranch)), + CurrentRefSubURL: git.RefNameFromBranch(profileRepo.DefaultBranch).RefWebLinkPath(), }) ctx.Data["ProfileReadmeContent"], err = markdown.RenderString(rctx, readmeBytes) if err != nil { diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go index 8efb5e59459..7357bf571c3 100644 --- a/routers/web/user/profile.go +++ b/routers/web/user/profile.go @@ -7,7 +7,6 @@ package user import ( "fmt" "net/http" - "path" "strings" activities_model "gitea.dev/models/activities" @@ -22,7 +21,6 @@ import ( "gitea.dev/modules/optional" "gitea.dev/modules/setting" "gitea.dev/modules/templates" - "gitea.dev/modules/util" "gitea.dev/routers/web/feed" "gitea.dev/routers/web/org" shared_user "gitea.dev/routers/web/shared/user" @@ -255,7 +253,7 @@ func prepareUserProfileTabData(ctx *context.Context, profileDbRepo *repo_model.R log.Error("failed to GetBlobContent: %v", err) } else { rctx := renderhelper.NewRenderContextRepoFile(ctx, profileDbRepo, renderhelper.RepoFileOptions{ - CurrentRefSubURL: path.Join("branch", util.PathEscapeSegments(profileDbRepo.DefaultBranch)), + CurrentRefSubURL: git.RefNameFromBranch(profileDbRepo.DefaultBranch).RefWebLinkPath(), }) if profileContent, err := markdown.RenderString(rctx, bytes); err != nil { log.Error("failed to RenderString: %v", err)