From ab60f40b68b13523ff1687e84027879fee0f3d23 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 7 Jan 2025 23:30:44 +0800 Subject: [PATCH 1/5] fix --- modules/htmlutil/html.go | 4 ++-- modules/markup/internal/renderinternal.go | 2 +- modules/markup/markdown/math/block_renderer.go | 4 +++- modules/markup/orgmode/orgmode.go | 2 +- modules/templates/helper.go | 16 +++++++++++++++- modules/templates/helper_test.go | 3 +-- 6 files changed, 23 insertions(+), 8 deletions(-) diff --git a/modules/htmlutil/html.go b/modules/htmlutil/html.go index 9b5f5a92d89a8..0ab0e71689d3d 100644 --- a/modules/htmlutil/html.go +++ b/modules/htmlutil/html.go @@ -30,7 +30,7 @@ func ParseSizeAndClass(defaultSize int, defaultClass string, others ...any) (int return size, class } -func HTMLFormat(s string, rawArgs ...any) template.HTML { +func HTMLFormat(s template.HTML, rawArgs ...any) template.HTML { args := slices.Clone(rawArgs) for i, v := range args { switch v := v.(type) { @@ -44,5 +44,5 @@ func HTMLFormat(s string, rawArgs ...any) template.HTML { args[i] = template.HTMLEscapeString(fmt.Sprint(v)) } } - return template.HTML(fmt.Sprintf(s, args...)) + return template.HTML(fmt.Sprintf(string(s), args...)) } diff --git a/modules/markup/internal/renderinternal.go b/modules/markup/internal/renderinternal.go index 4d58f160a9288..7a3e37b120f82 100644 --- a/modules/markup/internal/renderinternal.go +++ b/modules/markup/internal/renderinternal.go @@ -76,7 +76,7 @@ func (r *RenderInternal) ProtectSafeAttrs(content template.HTML) template.HTML { return template.HTML(reAttrClass().ReplaceAllString(string(content), `$1 data-attr-class="`+r.secureIDPrefix+`$2"$3`)) } -func (r *RenderInternal) FormatWithSafeAttrs(w io.Writer, fmt string, a ...any) error { +func (r *RenderInternal) FormatWithSafeAttrs(w io.Writer, fmt template.HTML, a ...any) error { _, err := w.Write([]byte(r.ProtectSafeAttrs(htmlutil.HTMLFormat(fmt, a...)))) return err } diff --git a/modules/markup/markdown/math/block_renderer.go b/modules/markup/markdown/math/block_renderer.go index c29f061882180..412e4d0dee6c2 100644 --- a/modules/markup/markdown/math/block_renderer.go +++ b/modules/markup/markdown/math/block_renderer.go @@ -4,6 +4,8 @@ package math import ( + "html/template" + "code.gitea.io/gitea/modules/markup/internal" giteaUtil "code.gitea.io/gitea/modules/util" @@ -50,7 +52,7 @@ func (r *BlockRenderer) renderBlock(w util.BufWriter, source []byte, node gast.N n := node.(*Block) if entering { code := giteaUtil.Iif(n.Inline, "", `
`) + ``
-		_ = r.renderInternal.FormatWithSafeAttrs(w, code)
+		_ = r.renderInternal.FormatWithSafeAttrs(w, template.HTML(code))
 		r.writeLines(w, source, n)
 	} else {
 		_, _ = w.WriteString(`` + giteaUtil.Iif(n.Inline, "", `
`) + "\n") diff --git a/modules/markup/orgmode/orgmode.go b/modules/markup/orgmode/orgmode.go index c6cc3340000f6..70d02c132161f 100644 --- a/modules/markup/orgmode/orgmode.go +++ b/modules/markup/orgmode/orgmode.go @@ -147,7 +147,7 @@ func (r *orgWriter) resolveLink(kind, link string) string { func (r *orgWriter) WriteRegularLink(l org.RegularLink) { link := r.resolveLink(l.Kind(), l.URL) - printHTML := func(html string, a ...any) { + printHTML := func(html template.HTML, a ...any) { _, _ = fmt.Fprint(r, htmlutil.HTMLFormat(html, a...)) } // Inspired by https://github.com/niklasfasching/go-org/blob/6eb20dbda93cb88c3503f7508dc78cbbc639378f/org/html_writer.go#L406-L427 diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 48d3a8ff89aed..609407d36b539 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -38,7 +38,7 @@ func NewFuncMap() template.FuncMap { "Iif": iif, "Eval": evalTokens, "SafeHTML": safeHTML, - "HTMLFormat": htmlutil.HTMLFormat, + "HTMLFormat": htmlFormat, "HTMLEscape": htmlEscape, "QueryEscape": queryEscape, "QueryBuild": QueryBuild, @@ -207,6 +207,20 @@ func htmlEscape(s any) template.HTML { panic(fmt.Sprintf("unexpected type %T", s)) } +func htmlFormat(s any, args ...any) template.HTML { + if len(args) == 0 { + // to prevent developers from calling "HTMLFormat $userInput" by mistake which will lead to XSS + panic("missing arguments for HTMLFormat") + } + switch v := s.(type) { + case string: + return htmlutil.HTMLFormat(template.HTML(v), args...) + case template.HTML: + return htmlutil.HTMLFormat(v, args...) + } + panic(fmt.Sprintf("unexpected type %T", s)) +} + func jsEscapeSafe(s string) template.HTML { return template.HTML(template.JSEscapeString(s)) } diff --git a/modules/templates/helper_test.go b/modules/templates/helper_test.go index e35e8a28f86ed..5d7bc93622be2 100644 --- a/modules/templates/helper_test.go +++ b/modules/templates/helper_test.go @@ -8,7 +8,6 @@ import ( "strings" "testing" - "code.gitea.io/gitea/modules/htmlutil" "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" @@ -88,7 +87,7 @@ func TestTemplateIif(t *testing.T) { func TestTemplateEscape(t *testing.T) { execTmpl := func(code string) string { tmpl := template.New("test") - tmpl.Funcs(template.FuncMap{"QueryBuild": QueryBuild, "HTMLFormat": htmlutil.HTMLFormat}) + tmpl.Funcs(template.FuncMap{"QueryBuild": QueryBuild, "HTMLFormat": htmlFormat}) template.Must(tmpl.Parse(code)) w := &strings.Builder{} assert.NoError(t, tmpl.Execute(w, nil)) From 730ef3e5c29f4dd01f2dc571aacd80b5e8879741 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 8 Jan 2025 08:45:40 +0800 Subject: [PATCH 2/5] update dependency --- go.mod | 2 +- go.sum | 2 ++ modules/markup/orgmode/orgmode_test.go | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 084b2946091f1..b77e35203d92a 100644 --- a/go.mod +++ b/go.mod @@ -27,7 +27,7 @@ require ( github.com/ProtonMail/go-crypto v1.0.0 github.com/PuerkitoBio/goquery v1.10.0 github.com/SaveTheRbtz/zstd-seekable-format-go/pkg v0.7.3 - github.com/alecthomas/chroma/v2 v2.14.0 + github.com/alecthomas/chroma/v2 v2.15.0 github.com/aws/aws-sdk-go-v2/credentials v1.17.42 github.com/aws/aws-sdk-go-v2/service/codecommit v1.27.3 github.com/blakesmith/ar v0.0.0-20190502131153-809d4375e1fb diff --git a/go.sum b/go.sum index 40add64289dce..bcf936e29d4db 100644 --- a/go.sum +++ b/go.sum @@ -86,6 +86,8 @@ github.com/alecthomas/assert/v2 v2.7.0/go.mod h1:Bze95FyfUr7x34QZrjL+XP+0qgp/zg8 github.com/alecthomas/chroma/v2 v2.2.0/go.mod h1:vf4zrexSH54oEjJ7EdB65tGNHmH3pGZmVkgTP5RHvAs= github.com/alecthomas/chroma/v2 v2.14.0 h1:R3+wzpnUArGcQz7fCETQBzO5n9IMNi13iIs46aU4V9E= github.com/alecthomas/chroma/v2 v2.14.0/go.mod h1:QolEbTfmUHIMVpBqxeDnNBj2uoeI4EbYP4i6n68SG4I= +github.com/alecthomas/chroma/v2 v2.15.0 h1:LxXTQHFoYrstG2nnV9y2X5O94sOBzf0CIUpSTbpxvMc= +github.com/alecthomas/chroma/v2 v2.15.0/go.mod h1:gUhVLrPDXPtp/f+L1jo9xepo9gL4eLwRuGAunSZMkio= github.com/alecthomas/repr v0.0.0-20220113201626-b1b626ac65ae/go.mod h1:2kn6fqh/zIyPLmm3ugklbEi5hg5wS435eygvNfaDQL8= github.com/alecthomas/repr v0.4.0 h1:GhI2A8MACjfegCPVq9f1FLvIBS+DrQ2KQBFZP1iFzXc= github.com/alecthomas/repr v0.4.0/go.mod h1:Fr0507jx4eOXV7AlPV6AVZLYrLIuIeSOWtW57eE/O/4= diff --git a/modules/markup/orgmode/orgmode_test.go b/modules/markup/orgmode/orgmode_test.go index e3cc05b4f022c..de39bafebeabe 100644 --- a/modules/markup/orgmode/orgmode_test.go +++ b/modules/markup/orgmode/orgmode_test.go @@ -103,8 +103,8 @@ func HelloWorld() { } #+end_src `, `
-
// HelloWorld prints "Hello World"
-func HelloWorld() {
+
// HelloWorld prints "Hello World"
+func HelloWorld() {
 	fmt.Println("Hello World")
 }
`) From e6dbf988eede1b081535745d0c101931d7a5772f Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 8 Jan 2025 08:49:42 +0800 Subject: [PATCH 3/5] fix js error --- web_src/js/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web_src/js/utils.ts b/web_src/js/utils.ts index efa144c3c762f..2a2bdc60f9577 100644 --- a/web_src/js/utils.ts +++ b/web_src/js/utils.ts @@ -40,7 +40,7 @@ export function parseIssueHref(href: string): IssuePathInfo { export function parseIssueNewHref(href: string): IssuePathInfo { const path = (href || '').replace(/[#?].*$/, ''); const [_, ownerName, repoName, pathTypeField] = /([^/]+)\/([^/]+)\/(issues\/new|compare\/.+\.\.\.)/.exec(path) || []; - const pathType = pathTypeField.startsWith('issues/new') ? 'issues' : 'pulls'; + const pathType = pathTypeField ? (pathTypeField.startsWith('issues/new') ? 'issues' : 'pulls') : undefined; return {ownerName, repoName, pathType}; } From 409354f0db2423ff68980507b645eb3878ffd4ed Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 8 Jan 2025 09:39:17 +0800 Subject: [PATCH 4/5] fix test --- web_src/js/utils.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/web_src/js/utils.test.ts b/web_src/js/utils.test.ts index bbe328f658df0..b527111533ff4 100644 --- a/web_src/js/utils.test.ts +++ b/web_src/js/utils.test.ts @@ -50,6 +50,7 @@ test('parseIssueNewHref', () => { expect(parseIssueNewHref('/owner/repo/issues/new?query')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues'}); expect(parseIssueNewHref('/sub/owner/repo/issues/new#hash')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues'}); expect(parseIssueNewHref('/sub/owner/repo/compare/feature/branch-1...fix/branch-2')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'pulls'}); + expect(parseIssueNewHref('/other')).toEqual({}); }); test('parseUrl', () => { From bed0c19ff0ac497774f9315ec56b089ddee2c01d Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 8 Jan 2025 09:46:31 +0800 Subject: [PATCH 5/5] improve comments --- go.sum | 6 ++---- web_src/js/features/common-button.ts | 3 ++- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/go.sum b/go.sum index f17c16fcfae85..d2383601c23b9 100644 --- a/go.sum +++ b/go.sum @@ -81,11 +81,9 @@ github.com/RoaringBitmap/roaring v1.9.4 h1:yhEIoH4YezLYT04s1nHehNO64EKFTop/wBhxv github.com/RoaringBitmap/roaring v1.9.4/go.mod h1:6AXUsoIEzDTFFQCe1RbGA6uFONMhvejWj5rqITANK90= github.com/SaveTheRbtz/zstd-seekable-format-go/pkg v0.7.3 h1:BP0HiyNT3AQEYi+if3wkRcIdQFHtsw6xX3Kx0glckgA= github.com/SaveTheRbtz/zstd-seekable-format-go/pkg v0.7.3/go.mod h1:hMNtySovKkn2gdDuLqnqveP+mfhUSaBdoBcr2I7Zt0E= -github.com/alecthomas/assert/v2 v2.7.0 h1:QtqSACNS3tF7oasA8CU6A6sXZSBDqnm7RfpLl9bZqbE= -github.com/alecthomas/assert/v2 v2.7.0/go.mod h1:Bze95FyfUr7x34QZrjL+XP+0qgp/zg8yS+TtBj1WA3k= +github.com/alecthomas/assert/v2 v2.11.0 h1:2Q9r3ki8+JYXvGsDyBXwH3LcJ+WK5D0gc5E8vS6K3D0= +github.com/alecthomas/assert/v2 v2.11.0/go.mod h1:Bze95FyfUr7x34QZrjL+XP+0qgp/zg8yS+TtBj1WA3k= github.com/alecthomas/chroma/v2 v2.2.0/go.mod h1:vf4zrexSH54oEjJ7EdB65tGNHmH3pGZmVkgTP5RHvAs= -github.com/alecthomas/chroma/v2 v2.14.0 h1:R3+wzpnUArGcQz7fCETQBzO5n9IMNi13iIs46aU4V9E= -github.com/alecthomas/chroma/v2 v2.14.0/go.mod h1:QolEbTfmUHIMVpBqxeDnNBj2uoeI4EbYP4i6n68SG4I= github.com/alecthomas/chroma/v2 v2.15.0 h1:LxXTQHFoYrstG2nnV9y2X5O94sOBzf0CIUpSTbpxvMc= github.com/alecthomas/chroma/v2 v2.15.0/go.mod h1:gUhVLrPDXPtp/f+L1jo9xepo9gL4eLwRuGAunSZMkio= github.com/alecthomas/repr v0.0.0-20220113201626-b1b626ac65ae/go.mod h1:2kn6fqh/zIyPLmm3ugklbEi5hg5wS435eygvNfaDQL8= diff --git a/web_src/js/features/common-button.ts b/web_src/js/features/common-button.ts index acce992b902d3..3162557b9b21f 100644 --- a/web_src/js/features/common-button.ts +++ b/web_src/js/features/common-button.ts @@ -17,7 +17,8 @@ export function initGlobalDeleteButton(): void { // Some model/form elements will be filled by `data-id` / `data-name` / `data-data-xxx` attributes. // If there is a form defined by `data-form`, then the form will be submitted as-is (without any modification). // If there is no form, then the data will be posted to `data-url`. - // TODO: it's not encouraged to use this method. `show-modal` does far better than this. + // TODO: do not use this method in new code. `show-modal` / `link-action(data-modal-confirm)` does far better than this. + // FIXME: all legacy `delete-button` should be refactored to use `show-modal` or `link-action` for (const btn of document.querySelectorAll('.delete-button')) { btn.addEventListener('click', (e) => { e.preventDefault();