diff options
author | Lunny Xiao <xiaolunwen@gmail.com> | 2022-06-16 11:33:23 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-06-16 11:33:23 +0800 |
commit | b01dce2a6e98c25915a8e98afb741a1c34d05aba (patch) | |
tree | b391ae8dff3ed5270b9721900bdc667b05fd555c /modules | |
parent | 7d1770cd714416bd80f114681d19e3076a0b0966 (diff) | |
download | gitea-b01dce2a6e98c25915a8e98afb741a1c34d05aba.tar.gz gitea-b01dce2a6e98c25915a8e98afb741a1c34d05aba.zip |
Allow render HTML with css/js external links (#19017)
* Allow render HTML with css/js external links
* Fix bug because of filename escape chars
* Fix lint
* Update docs about new configuration item
* Fix bug of render HTML in sub directory
* Add CSP head for displaying iframe in rendering file
* Fix test
* Apply suggestions from code review
Co-authored-by: delvh <dev.lh@web.de>
* Some improvements
* some improvement
* revert change in SanitizerDisabled of external renderer
* Add sandbox for iframe and support allow-scripts and allow-same-origin
* refactor
* fix
* fix lint
* fine tune
* use single option RENDER_CONTENT_MODE, use sandbox=allow-scripts
* fine tune CSP
* Apply suggestions from code review
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Diffstat (limited to 'modules')
-rw-r--r-- | modules/csv/csv.go | 2 | ||||
-rw-r--r-- | modules/csv/csv_test.go | 2 | ||||
-rw-r--r-- | modules/markup/console/console.go | 8 | ||||
-rw-r--r-- | modules/markup/csv/csv.go | 8 | ||||
-rw-r--r-- | modules/markup/external/external.go | 12 | ||||
-rw-r--r-- | modules/markup/html_test.go | 26 | ||||
-rw-r--r-- | modules/markup/markdown/markdown.go | 9 | ||||
-rw-r--r-- | modules/markup/orgmode/orgmode.go | 9 | ||||
-rw-r--r-- | modules/markup/renderer.go | 81 | ||||
-rw-r--r-- | modules/setting/markup.go | 37 |
10 files changed, 125 insertions, 69 deletions
diff --git a/modules/csv/csv.go b/modules/csv/csv.go index 0dd54271f1..fe0c350960 100644 --- a/modules/csv/csv.go +++ b/modules/csv/csv.go @@ -54,7 +54,7 @@ func CreateReaderAndDetermineDelimiter(ctx *markup.RenderContext, rd io.Reader) func determineDelimiter(ctx *markup.RenderContext, data []byte) rune { extension := ".csv" if ctx != nil { - extension = strings.ToLower(filepath.Ext(ctx.Filename)) + extension = strings.ToLower(filepath.Ext(ctx.RelativePath)) } var delimiter rune diff --git a/modules/csv/csv_test.go b/modules/csv/csv_test.go index b1e928ae99..9d0848ae5b 100644 --- a/modules/csv/csv_test.go +++ b/modules/csv/csv_test.go @@ -230,7 +230,7 @@ John Doe john@doe.com This,note,had,a,lot,of,commas,to,test,delimiters`, } for n, c := range cases { - delimiter := determineDelimiter(&markup.RenderContext{Filename: c.filename}, []byte(decodeSlashes(t, c.csv))) + delimiter := determineDelimiter(&markup.RenderContext{RelativePath: c.filename}, []byte(decodeSlashes(t, c.csv))) assert.EqualValues(t, c.expectedDelimiter, delimiter, "case %d: delimiter should be equal, expected '%c' got '%c'", n, c.expectedDelimiter, delimiter) } } diff --git a/modules/markup/console/console.go b/modules/markup/console/console.go index b59594acb7..597593eee1 100644 --- a/modules/markup/console/console.go +++ b/modules/markup/console/console.go @@ -33,9 +33,6 @@ func (Renderer) Name() string { return MarkupName } -// NeedPostProcess implements markup.Renderer -func (Renderer) NeedPostProcess() bool { return false } - // Extensions implements markup.Renderer func (Renderer) Extensions() []string { return []string{".sh-session"} @@ -48,11 +45,6 @@ func (Renderer) SanitizerRules() []setting.MarkupSanitizerRule { } } -// SanitizerDisabled disabled sanitize if return true -func (Renderer) SanitizerDisabled() bool { - return false -} - // CanRender implements markup.RendererContentDetector func (Renderer) CanRender(filename string, input io.Reader) bool { buf, err := io.ReadAll(input) diff --git a/modules/markup/csv/csv.go b/modules/markup/csv/csv.go index 17c3fe6f4f..5095b85465 100644 --- a/modules/markup/csv/csv.go +++ b/modules/markup/csv/csv.go @@ -29,9 +29,6 @@ func (Renderer) Name() string { return "csv" } -// NeedPostProcess implements markup.Renderer -func (Renderer) NeedPostProcess() bool { return false } - // Extensions implements markup.Renderer func (Renderer) Extensions() []string { return []string{".csv", ".tsv"} @@ -46,11 +43,6 @@ func (Renderer) SanitizerRules() []setting.MarkupSanitizerRule { } } -// SanitizerDisabled disabled sanitize if return true -func (Renderer) SanitizerDisabled() bool { - return false -} - func writeField(w io.Writer, element, class, field string) error { if _, err := io.WriteString(w, "<"); err != nil { return err diff --git a/modules/markup/external/external.go b/modules/markup/external/external.go index a587abcc3b..23dd45ba0a 100644 --- a/modules/markup/external/external.go +++ b/modules/markup/external/external.go @@ -34,6 +34,11 @@ type Renderer struct { *setting.MarkupRenderer } +var ( + _ markup.PostProcessRenderer = (*Renderer)(nil) + _ markup.ExternalRenderer = (*Renderer)(nil) +) + // Name returns the external tool name func (p *Renderer) Name() string { return p.MarkupName @@ -56,7 +61,12 @@ func (p *Renderer) SanitizerRules() []setting.MarkupSanitizerRule { // SanitizerDisabled disabled sanitize if return true func (p *Renderer) SanitizerDisabled() bool { - return p.DisableSanitizer + return p.RenderContentMode == setting.RenderContentModeNoSanitizer || p.RenderContentMode == setting.RenderContentModeIframe +} + +// DisplayInIFrame represents whether render the content with an iframe +func (p *Renderer) DisplayInIFrame() bool { + return p.RenderContentMode == setting.RenderContentModeIframe } func envMark(envName string) string { diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go index f6aabc6272..f494998c59 100644 --- a/modules/markup/html_test.go +++ b/modules/markup/html_test.go @@ -29,10 +29,10 @@ func TestRender_Commits(t *testing.T) { setting.AppURL = TestAppURL test := func(input, expected string) { buffer, err := RenderString(&RenderContext{ - Ctx: git.DefaultContext, - Filename: ".md", - URLPrefix: TestRepoURL, - Metas: localMetas, + Ctx: git.DefaultContext, + RelativePath: ".md", + URLPrefix: TestRepoURL, + Metas: localMetas, }, input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) @@ -80,9 +80,9 @@ func TestRender_CrossReferences(t *testing.T) { test := func(input, expected string) { buffer, err := RenderString(&RenderContext{ - Filename: "a.md", - URLPrefix: setting.AppSubURL, - Metas: localMetas, + RelativePath: "a.md", + URLPrefix: setting.AppSubURL, + Metas: localMetas, }, input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) @@ -124,8 +124,8 @@ func TestRender_links(t *testing.T) { test := func(input, expected string) { buffer, err := RenderString(&RenderContext{ - Filename: "a.md", - URLPrefix: TestRepoURL, + RelativePath: "a.md", + URLPrefix: TestRepoURL, }, input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) @@ -223,8 +223,8 @@ func TestRender_email(t *testing.T) { test := func(input, expected string) { res, err := RenderString(&RenderContext{ - Filename: "a.md", - URLPrefix: TestRepoURL, + RelativePath: "a.md", + URLPrefix: TestRepoURL, }, input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(res)) @@ -281,8 +281,8 @@ func TestRender_emoji(t *testing.T) { test := func(input, expected string) { expected = strings.ReplaceAll(expected, "&", "&") buffer, err := RenderString(&RenderContext{ - Filename: "a.md", - URLPrefix: TestRepoURL, + RelativePath: "a.md", + URLPrefix: TestRepoURL, }, input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) diff --git a/modules/markup/markdown/markdown.go b/modules/markup/markdown/markdown.go index 7ebdfea6c4..37e11e606f 100644 --- a/modules/markup/markdown/markdown.go +++ b/modules/markup/markdown/markdown.go @@ -205,12 +205,14 @@ func init() { // Renderer implements markup.Renderer type Renderer struct{} +var _ markup.PostProcessRenderer = (*Renderer)(nil) + // Name implements markup.Renderer func (Renderer) Name() string { return MarkupName } -// NeedPostProcess implements markup.Renderer +// NeedPostProcess implements markup.PostProcessRenderer func (Renderer) NeedPostProcess() bool { return true } // Extensions implements markup.Renderer @@ -223,11 +225,6 @@ func (Renderer) SanitizerRules() []setting.MarkupSanitizerRule { return []setting.MarkupSanitizerRule{} } -// SanitizerDisabled disabled sanitize if return true -func (Renderer) SanitizerDisabled() bool { - return false -} - // Render implements markup.Renderer func (Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error { return render(ctx, input, output) diff --git a/modules/markup/orgmode/orgmode.go b/modules/markup/orgmode/orgmode.go index 2f394b992b..8c9f3b3da7 100644 --- a/modules/markup/orgmode/orgmode.go +++ b/modules/markup/orgmode/orgmode.go @@ -29,12 +29,14 @@ func init() { // Renderer implements markup.Renderer for orgmode type Renderer struct{} +var _ markup.PostProcessRenderer = (*Renderer)(nil) + // Name implements markup.Renderer func (Renderer) Name() string { return "orgmode" } -// NeedPostProcess implements markup.Renderer +// NeedPostProcess implements markup.PostProcessRenderer func (Renderer) NeedPostProcess() bool { return true } // Extensions implements markup.Renderer @@ -47,11 +49,6 @@ func (Renderer) SanitizerRules() []setting.MarkupSanitizerRule { return []setting.MarkupSanitizerRule{} } -// SanitizerDisabled disabled sanitize if return true -func (Renderer) SanitizerDisabled() bool { - return false -} - // Render renders orgmode rawbytes to HTML func Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error { htmlWriter := org.NewHTMLWriter() diff --git a/modules/markup/renderer.go b/modules/markup/renderer.go index 6e4ae4e08c..e88fa31187 100644 --- a/modules/markup/renderer.go +++ b/modules/markup/renderer.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "io" + "net/url" "path/filepath" "strings" "sync" @@ -43,17 +44,18 @@ type Header struct { // RenderContext represents a render context type RenderContext struct { - Ctx context.Context - Filename string - Type string - IsWiki bool - URLPrefix string - Metas map[string]string - DefaultLink string - GitRepo *git.Repository - ShaExistCache map[string]bool - cancelFn func() - TableOfContents []Header + Ctx context.Context + RelativePath string // relative path from tree root of the branch + Type string + IsWiki bool + URLPrefix string + Metas map[string]string + DefaultLink string + GitRepo *git.Repository + ShaExistCache map[string]bool + cancelFn func() + TableOfContents []Header + InStandalonePage bool // used by external render. the router "/org/repo/render/..." will output the rendered content in a standalone page } // Cancel runs any cleanup functions that have been registered for this Ctx @@ -88,12 +90,24 @@ func (ctx *RenderContext) AddCancel(fn func()) { type Renderer interface { Name() string // markup format name Extensions() []string - NeedPostProcess() bool SanitizerRules() []setting.MarkupSanitizerRule - SanitizerDisabled() bool Render(ctx *RenderContext, input io.Reader, output io.Writer) error } +// PostProcessRenderer defines an interface for renderers who need post process +type PostProcessRenderer interface { + NeedPostProcess() bool +} + +// PostProcessRenderer defines an interface for external renderers +type ExternalRenderer interface { + // SanitizerDisabled disabled sanitize if return true + SanitizerDisabled() bool + + // DisplayInIFrame represents whether render the content with an iframe + DisplayInIFrame() bool +} + // RendererContentDetector detects if the content can be rendered // by specified renderer type RendererContentDetector interface { @@ -142,7 +156,7 @@ func DetectRendererType(filename string, input io.Reader) string { func Render(ctx *RenderContext, input io.Reader, output io.Writer) error { if ctx.Type != "" { return renderByType(ctx, input, output) - } else if ctx.Filename != "" { + } else if ctx.RelativePath != "" { return renderFile(ctx, input, output) } return errors.New("Render options both filename and type missing") @@ -163,6 +177,27 @@ type nopCloser struct { func (nopCloser) Close() error { return nil } +func renderIFrame(ctx *RenderContext, output io.Writer) error { + // set height="0" ahead, otherwise the scrollHeight would be max(150, realHeight) + // at the moment, only "allow-scripts" is allowed for sandbox mode. + // "allow-same-origin" should never be used, it leads to XSS attack, and it makes the JS in iframe can access parent window's config and CSRF token + // TODO: when using dark theme, if the rendered content doesn't have proper style, the default text color is black, which is not easy to read + _, err := io.WriteString(output, fmt.Sprintf(` +<iframe src="%s/%s/%s/render/%s/%s" +name="giteaExternalRender" +onload="this.height=giteaExternalRender.document.documentElement.scrollHeight" +width="100%%" height="0" scrolling="no" frameborder="0" style="overflow: hidden" +sandbox="allow-scripts" +></iframe>`, + setting.AppSubURL, + url.PathEscape(ctx.Metas["user"]), + url.PathEscape(ctx.Metas["repo"]), + ctx.Metas["BranchNameSubURL"], + url.PathEscape(ctx.RelativePath), + )) + return err +} + func render(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Writer) error { var wg sync.WaitGroup var err error @@ -175,7 +210,12 @@ func render(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Wr var pr2 io.ReadCloser var pw2 io.WriteCloser - if !renderer.SanitizerDisabled() { + var sanitizerDisabled bool + if r, ok := renderer.(ExternalRenderer); ok { + sanitizerDisabled = r.SanitizerDisabled() + } + + if !sanitizerDisabled { pr2, pw2 = io.Pipe() defer func() { _ = pr2.Close() @@ -194,7 +234,7 @@ func render(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Wr wg.Add(1) go func() { - if renderer.NeedPostProcess() { + if r, ok := renderer.(PostProcessRenderer); ok && r.NeedPostProcess() { err = PostProcess(ctx, pr, pw2) } else { _, err = io.Copy(pw2, pr) @@ -239,8 +279,15 @@ func (err ErrUnsupportedRenderExtension) Error() string { } func renderFile(ctx *RenderContext, input io.Reader, output io.Writer) error { - extension := strings.ToLower(filepath.Ext(ctx.Filename)) + extension := strings.ToLower(filepath.Ext(ctx.RelativePath)) if renderer, ok := extRenderers[extension]; ok { + if r, ok := renderer.(ExternalRenderer); ok && r.DisplayInIFrame() { + if !ctx.InStandalonePage { + // for an external render, it could only output its content in a standalone page + // otherwise, a <iframe> should be outputted to embed the external rendered page + return renderIFrame(ctx, output) + } + } return render(ctx, renderer, input, output) } return ErrUnsupportedRenderExtension{extension} diff --git a/modules/setting/markup.go b/modules/setting/markup.go index 5fb6af6838..fd41bdd7cc 100644 --- a/modules/setting/markup.go +++ b/modules/setting/markup.go @@ -20,6 +20,12 @@ var ( MermaidMaxSourceCharacters int ) +const ( + RenderContentModeSanitized = "sanitized" + RenderContentModeNoSanitizer = "no-sanitizer" + RenderContentModeIframe = "iframe" +) + // MarkupRenderer defines the external parser configured in ini type MarkupRenderer struct { Enabled bool @@ -29,7 +35,7 @@ type MarkupRenderer struct { IsInputFile bool NeedPostProcess bool MarkupSanitizerRules []MarkupSanitizerRule - DisableSanitizer bool + RenderContentMode string } // MarkupSanitizerRule defines the policy for whitelisting attributes on @@ -144,13 +150,28 @@ func newMarkupRenderer(name string, sec *ini.Section) { return } + if sec.HasKey("DISABLE_SANITIZER") { + log.Error("Deprecated setting `[markup.*]` `DISABLE_SANITIZER` present. This fallback will be removed in v1.18.0") + } + + renderContentMode := sec.Key("RENDER_CONTENT_MODE").MustString(RenderContentModeSanitized) + if !sec.HasKey("RENDER_CONTENT_MODE") && sec.Key("DISABLE_SANITIZER").MustBool(false) { + renderContentMode = RenderContentModeNoSanitizer // if only the legacy DISABLE_SANITIZER exists, use it + } + if renderContentMode != RenderContentModeSanitized && + renderContentMode != RenderContentModeNoSanitizer && + renderContentMode != RenderContentModeIframe { + log.Error("invalid RENDER_CONTENT_MODE: %q, default to %q", renderContentMode, RenderContentModeSanitized) + renderContentMode = RenderContentModeSanitized + } + ExternalMarkupRenderers = append(ExternalMarkupRenderers, &MarkupRenderer{ - Enabled: sec.Key("ENABLED").MustBool(false), - MarkupName: name, - FileExtensions: exts, - Command: command, - IsInputFile: sec.Key("IS_INPUT_FILE").MustBool(false), - NeedPostProcess: sec.Key("NEED_POSTPROCESS").MustBool(true), - DisableSanitizer: sec.Key("DISABLE_SANITIZER").MustBool(false), + Enabled: sec.Key("ENABLED").MustBool(false), + MarkupName: name, + FileExtensions: exts, + Command: command, + IsInputFile: sec.Key("IS_INPUT_FILE").MustBool(false), + NeedPostProcess: sec.Key("NEED_POSTPROCESS").MustBool(true), + RenderContentMode: renderContentMode, }) } |