Skip to content

Commit 50ed07e

Browse files
wxiaoguangwxiaoguang
committed
Fix external render (go-gitea#35727)
Fix go-gitea#35725 --------- Signed-off-by: wxiaoguang <[email protected]>
1 parent 0925089 commit 50ed07e

File tree

7 files changed

+124
-56
lines changed

7 files changed

+124
-56
lines changed

‎custom/conf/app.example.ini‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2541,6 +2541,12 @@ LEVEL = Info
25412541
;; * no-sanitizer: Disable the sanitizer and render the content inside current page. It's **insecure** and may lead to XSS attack if the content contains malicious code.
25422542
;; * iframe: Render the content in a separate standalone page and embed it into current page by iframe. The iframe is in sandbox mode with same-origin disabled, and the JS code are safely isolated from parent page.
25432543
;RENDER_CONTENT_MODE=sanitized
2544+
;;
2545+
;; Whether post-process the rendered HTML content, including:
2546+
;; resolve relative links and image sources, recognizing issue/commit references, escaping invisible characters,
2547+
;; mentioning users, rendering permlink code blocks, replacing emoji shorthands, etc.
2548+
;; By default, this is true when RENDER_CONTENT_MODE is `sanitized`, otherwise false.
2549+
;NEED_POST_PROCESS=false
25442550

25452551
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
25462552
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

‎modules/markup/external/external.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import (
1515
"code.gitea.io/gitea/modules/markup"
1616
"code.gitea.io/gitea/modules/process"
1717
"code.gitea.io/gitea/modules/setting"
18+
19+
"github.com/kballard/go-shellquote"
1820
)
1921

2022
// RegisterRenderers registers all supported third part renderers according settings
@@ -81,7 +83,10 @@ func (p *Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.
8183
envMark("GITEA_PREFIX_SRC"), baseLinkSrc,
8284
envMark("GITEA_PREFIX_RAW"), baseLinkRaw,
8385
).Replace(p.Command)
84-
commands := strings.Fields(command)
86+
commands, err := shellquote.Split(command)
87+
if err != nil || len(commands) == 0 {
88+
return fmt.Errorf("%s invalid command %q: %w", p.Name(), p.Command, err)
89+
}
8590
args := commands[1:]
8691

8792
if p.IsInputFile {

‎modules/markup/render.go‎

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -120,31 +120,38 @@ func (ctx *RenderContext) WithHelper(helper RenderHelper) *RenderContext {
120120
return ctx
121121
}
122122

123-
// Render renders markup file to HTML with all specific handling stuff.
124-
func Render(ctx *RenderContext, input io.Reader, output io.Writer) error {
123+
// FindRendererByContext finds renderer by RenderContext
124+
// TODO: it should be merged with other similar functions like GetRendererByFileName, DetectMarkupTypeByFileName, etc
125+
func FindRendererByContext(ctx *RenderContext) (Renderer, error) {
125126
if ctx.RenderOptions.MarkupType == "" && ctx.RenderOptions.RelativePath != "" {
126127
ctx.RenderOptions.MarkupType = DetectMarkupTypeByFileName(ctx.RenderOptions.RelativePath)
127128
if ctx.RenderOptions.MarkupType == "" {
128-
return util.NewInvalidArgumentErrorf("unsupported file to render: %q", ctx.RenderOptions.RelativePath)
129+
return nil, util.NewInvalidArgumentErrorf("unsupported file to render: %q", ctx.RenderOptions.RelativePath)
129130
}
130131
}
131132

132133
renderer := renderers[ctx.RenderOptions.MarkupType]
133134
if renderer == nil {
134-
return util.NewInvalidArgumentErrorf("unsupported markup type: %q", ctx.RenderOptions.MarkupType)
135+
return nil, util.NewNotExistErrorf("unsupported markup type: %q", ctx.RenderOptions.MarkupType)
135136
}
136137

137-
if ctx.RenderOptions.RelativePath != "" {
138-
if externalRender, ok := renderer.(ExternalRenderer); ok && externalRender.DisplayInIFrame() {
139-
if !ctx.RenderOptions.InStandalonePage {
140-
// for an external "DisplayInIFrame" render, it could only output its content in a standalone page
141-
// otherwise, a <iframe> should be outputted to embed the external rendered page
142-
return renderIFrame(ctx, output)
143-
}
144-
}
138+
return renderer, nil
139+
}
140+
141+
func RendererNeedPostProcess(renderer Renderer) bool {
142+
if r, ok := renderer.(PostProcessRenderer); ok && r.NeedPostProcess() {
143+
return true
145144
}
145+
return false
146+
}
146147

147-
return render(ctx, renderer, input, output)
148+
// Render renders markup file to HTML with all specific handling stuff.
149+
func Render(ctx *RenderContext, input io.Reader, output io.Writer) error {
150+
renderer, err := FindRendererByContext(ctx)
151+
if err != nil {
152+
return err
153+
}
154+
return RenderWithRenderer(ctx, renderer, input, output)
148155
}
149156

150157
// RenderString renders Markup string to HTML with all specific handling stuff and return string
@@ -185,7 +192,16 @@ func pipes() (io.ReadCloser, io.WriteCloser, func()) {
185192
}
186193
}
187194

188-
func render(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Writer) error {
195+
func RenderWithRenderer(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Writer) error {
196+
if externalRender, ok := renderer.(ExternalRenderer); ok && externalRender.DisplayInIFrame() {
197+
if !ctx.RenderOptions.InStandalonePage {
198+
// for an external "DisplayInIFrame" render, it could only output its content in a standalone page
199+
// otherwise, a <iframe> should be outputted to embed the external rendered page
200+
return renderIFrame(ctx, output)
201+
}
202+
// else: this is a standalone page, fallthrough to the real rendering
203+
}
204+
189205
ctx.usedByRender = true
190206
if ctx.RenderHelper != nil {
191207
defer ctx.RenderHelper.CleanUp()
@@ -214,7 +230,7 @@ func render(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Wr
214230
}
215231

216232
eg.Go(func() (err error) {
217-
if r, ok := renderer.(PostProcessRenderer); ok && r.NeedPostProcess() {
233+
if RendererNeedPostProcess(renderer) {
218234
err = PostProcessDefault(ctx, pr1, pw2)
219235
} else {
220236
_, err = io.Copy(pw2, pr1)

‎modules/setting/markup.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,9 @@ func newMarkupRenderer(name string, sec ConfigSection) {
259259
FileExtensions: exts,
260260
Command: command,
261261
IsInputFile: sec.Key("IS_INPUT_FILE").MustBool(false),
262-
NeedPostProcess: sec.Key("NEED_POSTPROCESS").MustBool(true),
263262
RenderContentMode: renderContentMode,
263+
264+
// if no sanitizer is needed, no post process is needed
265+
NeedPostProcess: sec.Key("NEED_POST_PROCESS").MustBool(renderContentMode == RenderContentModeSanitized),
264266
})
265267
}

‎routers/web/repo/view.go‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,17 +151,28 @@ func loadLatestCommitData(ctx *context.Context, latestCommit *git.Commit) bool {
151151
}
152152

153153
func markupRender(ctx *context.Context, renderCtx *markup.RenderContext, input io.Reader) (escaped *charset.EscapeStatus, output template.HTML, err error) {
154+
renderer, err := markup.FindRendererByContext(renderCtx)
155+
if err != nil {
156+
return nil, "", err
157+
}
158+
154159
markupRd, markupWr := io.Pipe()
155160
defer markupWr.Close()
161+
156162
done := make(chan struct{})
157163
go func() {
158164
sb := &strings.Builder{}
159-
// We allow NBSP here this is rendered
160-
escaped, _ = charset.EscapeControlReader(markupRd, sb, ctx.Locale, charset.RuneNBSP)
165+
if markup.RendererNeedPostProcess(renderer) {
166+
escaped, _ = charset.EscapeControlReader(markupRd, sb, ctx.Locale, charset.RuneNBSP) // We allow NBSP here this is rendered
167+
} else {
168+
escaped = &charset.EscapeStatus{}
169+
_, _ = io.Copy(sb, markupRd)
170+
}
161171
output = template.HTML(sb.String())
162172
close(done)
163173
}()
164-
err = markup.Render(renderCtx, input, markupWr)
174+
175+
err = markup.RenderWithRenderer(renderCtx, renderer, input, markupWr)
165176
_ = markupWr.CloseWithError(err)
166177
<-done
167178
return escaped, output, err

‎tests/integration/markup_external_test.go‎

Lines changed: 54 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,23 @@
44
package integration
55

66
import (
7-
"bytes"
87
"io"
98
"net/http"
9+
"net/url"
1010
"strings"
1111
"testing"
1212

13+
repo_model "code.gitea.io/gitea/models/repo"
14+
"code.gitea.io/gitea/models/unittest"
15+
user_model "code.gitea.io/gitea/models/user"
1316
"code.gitea.io/gitea/modules/markup"
1417
"code.gitea.io/gitea/modules/markup/external"
1518
"code.gitea.io/gitea/modules/setting"
19+
"code.gitea.io/gitea/modules/test"
1620
"code.gitea.io/gitea/tests"
1721

1822
"github.com/stretchr/testify/assert"
23+
"github.com/stretchr/testify/require"
1924
)
2025

2126
func TestExternalMarkupRenderer(t *testing.T) {
@@ -25,36 +30,52 @@ func TestExternalMarkupRenderer(t *testing.T) {
2530
return
2631
}
2732

28-
req := NewRequest(t, "GET", "/user30/renderer/src/branch/master/README.html")
29-
resp := MakeRequest(t, req, http.StatusOK)
30-
assert.Equal(t, "text/html; charset=utf-8", resp.Header().Get("Content-Type"))
31-
32-
bs, err := io.ReadAll(resp.Body)
33-
assert.NoError(t, err)
34-
35-
doc := NewHTMLParser(t, bytes.NewBuffer(bs))
36-
div := doc.Find("div.file-view")
37-
data, err := div.Html()
38-
assert.NoError(t, err)
39-
assert.Equal(t, "<div>\n\ttest external renderer\n</div>", strings.TrimSpace(data))
40-
41-
r := markup.GetRendererByFileName("a.html").(*external.Renderer)
42-
r.RenderContentMode = setting.RenderContentModeIframe
43-
44-
req = NewRequest(t, "GET", "/user30/renderer/src/branch/master/README.html")
45-
resp = MakeRequest(t, req, http.StatusOK)
46-
assert.Equal(t, "text/html; charset=utf-8", resp.Header().Get("Content-Type"))
47-
bs, err = io.ReadAll(resp.Body)
48-
assert.NoError(t, err)
49-
doc = NewHTMLParser(t, bytes.NewBuffer(bs))
50-
iframe := doc.Find("iframe")
51-
assert.Equal(t, "/user30/renderer/render/branch/master/README.html", iframe.AttrOr("src", ""))
52-
53-
req = NewRequest(t, "GET", "/user30/renderer/render/branch/master/README.html")
54-
resp = MakeRequest(t, req, http.StatusOK)
55-
assert.Equal(t, "text/html; charset=utf-8", resp.Header().Get("Content-Type"))
56-
bs, err = io.ReadAll(resp.Body)
57-
assert.NoError(t, err)
58-
assert.Equal(t, "frame-src 'self'; sandbox allow-scripts", resp.Header().Get("Content-Security-Policy"))
59-
assert.Equal(t, "<div>\n\ttest external renderer\n</div>\n", string(bs))
33+
onGiteaRun(t, func(t *testing.T, _ *url.URL) {
34+
t.Run("RenderNoSanitizer", func(t *testing.T) {
35+
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
36+
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
37+
_, err := createFile(user2, repo1, "file.no-sanitizer", "master", `any content`)
38+
require.NoError(t, err)
39+
40+
req := NewRequest(t, "GET", "/user2/repo1/src/branch/master/file.no-sanitizer")
41+
resp := MakeRequest(t, req, http.StatusOK)
42+
doc := NewHTMLParser(t, resp.Body)
43+
div := doc.Find("div.file-view")
44+
data, err := div.Html()
45+
assert.NoError(t, err)
46+
assert.Equal(t, `<script>window.alert("hi")</script>`, strings.TrimSpace(data))
47+
})
48+
})
49+
50+
t.Run("RenderContentDirectly", func(t *testing.T) {
51+
req := NewRequest(t, "GET", "/user30/renderer/src/branch/master/README.html")
52+
resp := MakeRequest(t, req, http.StatusOK)
53+
assert.Equal(t, "text/html; charset=utf-8", resp.Header().Get("Content-Type"))
54+
55+
doc := NewHTMLParser(t, resp.Body)
56+
div := doc.Find("div.file-view")
57+
data, err := div.Html()
58+
assert.NoError(t, err)
59+
assert.Equal(t, "<div>\n\ttest external renderer\n</div>", strings.TrimSpace(data))
60+
})
61+
62+
r := markup.GetRendererByFileName("any-file.html").(*external.Renderer)
63+
defer test.MockVariableValue(&r.RenderContentMode, setting.RenderContentModeIframe)()
64+
65+
t.Run("RenderContentInIFrame", func(t *testing.T) {
66+
req := NewRequest(t, "GET", "/user30/renderer/src/branch/master/README.html")
67+
resp := MakeRequest(t, req, http.StatusOK)
68+
assert.Equal(t, "text/html; charset=utf-8", resp.Header().Get("Content-Type"))
69+
doc := NewHTMLParser(t, resp.Body)
70+
iframe := doc.Find("iframe")
71+
assert.Equal(t, "/user30/renderer/render/branch/master/README.html", iframe.AttrOr("src", ""))
72+
73+
req = NewRequest(t, "GET", "/user30/renderer/render/branch/master/README.html")
74+
resp = MakeRequest(t, req, http.StatusOK)
75+
assert.Equal(t, "text/html; charset=utf-8", resp.Header().Get("Content-Type"))
76+
bs, err := io.ReadAll(resp.Body)
77+
assert.NoError(t, err)
78+
assert.Equal(t, "frame-src 'self'; sandbox allow-scripts", resp.Header().Get("Content-Security-Policy"))
79+
assert.Equal(t, "<div>\n\ttest external renderer\n</div>\n", string(bs))
80+
})
6081
}

‎tests/sqlite.ini.tmpl‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,16 @@ ENABLED = true
114114
[markup.html]
115115
ENABLED = true
116116
FILE_EXTENSIONS = .html
117-
RENDER_COMMAND = `go run build/test-echo.go`
118-
IS_INPUT_FILE = false
119-
RENDER_CONTENT_MODE=sanitized
117+
RENDER_COMMAND = go run build/test-echo.go
118+
;RENDER_COMMAND = cat
119+
;IS_INPUT_FILE = true
120+
RENDER_CONTENT_MODE = sanitized
121+
122+
[markup.no-sanitizer]
123+
ENABLED = true
124+
FILE_EXTENSIONS = .no-sanitizer
125+
RENDER_COMMAND = echo '<script>window.alert("hi")</script>'
126+
RENDER_CONTENT_MODE = no-sanitizer
120127

121128
[actions]
122129
ENABLED = true

0 commit comments

Comments
 (0)