diff options
Diffstat (limited to 'modules')
-rw-r--r-- | modules/htmlutil/html.go | 4 | ||||
-rw-r--r-- | modules/markup/html_node.go | 10 | ||||
-rw-r--r-- | modules/markup/markdown/markdown_test.go | 19 | ||||
-rw-r--r-- | modules/markup/markdown/math/block_renderer.go | 4 | ||||
-rw-r--r-- | modules/markup/markdown/math/inline_renderer.go | 2 | ||||
-rw-r--r-- | modules/markup/sanitizer_default.go | 9 | ||||
-rw-r--r-- | modules/markup/sanitizer_default_test.go | 2 | ||||
-rw-r--r-- | modules/structs/repo_file.go | 2 | ||||
-rw-r--r-- | modules/templates/helper.go | 28 | ||||
-rw-r--r-- | modules/web/router_path.go | 36 | ||||
-rw-r--r-- | modules/web/router_test.go | 82 |
11 files changed, 112 insertions, 86 deletions
diff --git a/modules/htmlutil/html.go b/modules/htmlutil/html.go index 0ab0e71689..194135ba18 100644 --- a/modules/htmlutil/html.go +++ b/modules/htmlutil/html.go @@ -7,6 +7,7 @@ import ( "fmt" "html/template" "slices" + "strings" ) // ParseSizeAndClass get size and class from string with default values @@ -31,6 +32,9 @@ func ParseSizeAndClass(defaultSize int, defaultClass string, others ...any) (int } func HTMLFormat(s template.HTML, rawArgs ...any) template.HTML { + if !strings.Contains(string(s), "%") || len(rawArgs) == 0 { + panic("HTMLFormat requires one or more arguments") + } args := slices.Clone(rawArgs) for i, v := range args { switch v := v.(type) { diff --git a/modules/markup/html_node.go b/modules/markup/html_node.go index f67437465c..4eb78fdd2b 100644 --- a/modules/markup/html_node.go +++ b/modules/markup/html_node.go @@ -63,8 +63,11 @@ func processNodeA(ctx *RenderContext, node *html.Node) { func visitNodeImg(ctx *RenderContext, img *html.Node) (next *html.Node) { next = img.NextSibling + attrSrc, hasLazy := "", false for i, imgAttr := range img.Attr { + hasLazy = hasLazy || imgAttr.Key == "loading" && imgAttr.Val == "lazy" if imgAttr.Key != "src" { + attrSrc = imgAttr.Val continue } @@ -72,8 +75,8 @@ func visitNodeImg(ctx *RenderContext, img *html.Node) (next *html.Node) { isLinkable := imgSrcOrigin != "" && !strings.HasPrefix(imgSrcOrigin, "data:") // By default, the "<img>" tag should also be clickable, - // because frontend use `<img>` to paste the re-scaled image into the markdown, - // so it must match the default markdown image behavior. + // because frontend uses `<img>` to paste the re-scaled image into the Markdown, + // so it must match the default Markdown image behavior. cnt := 0 for p := img.Parent; isLinkable && p != nil && cnt < 2; p = p.Parent { if hasParentAnchor := p.Type == html.ElementNode && p.Data == "a"; hasParentAnchor { @@ -98,6 +101,9 @@ func visitNodeImg(ctx *RenderContext, img *html.Node) (next *html.Node) { imgAttr.Val = camoHandleLink(imgAttr.Val) img.Attr[i] = imgAttr } + if !RenderBehaviorForTesting.DisableAdditionalAttributes && !hasLazy && !strings.HasPrefix(attrSrc, "data:") { + img.Attr = append(img.Attr, html.Attribute{Key: "loading", Val: "lazy"}) + } return next } diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go index 76434ac8b3..4eb01bcc2d 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/modules/markup/markdown/markdown_test.go @@ -47,7 +47,7 @@ func TestRender_StandardLinks(t *testing.T) { func TestRender_Images(t *testing.T) { setting.AppURL = AppURL - test := func(input, expected string) { + render := func(input, expected string) { buffer, err := markdown.RenderString(markup.NewTestRenderContext(FullURL), input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(buffer))) @@ -59,27 +59,32 @@ func TestRender_Images(t *testing.T) { result := util.URLJoin(FullURL, url) // hint: With Markdown v2.5.2, there is a new syntax: [link](URL){:target="_blank"} , but we do not support it now - test( + render( "", `<p><a href="`+result+`" target="_blank" rel="nofollow noopener"><img src="`+result+`" alt="`+title+`"/></a></p>`) - test( + render( "[["+title+"|"+url+"]]", `<p><a href="`+result+`" rel="nofollow"><img src="`+result+`" title="`+title+`" alt="`+title+`"/></a></p>`) - test( + render( "[]("+href+")", `<p><a href="`+href+`" rel="nofollow"><img src="`+result+`" alt="`+title+`"/></a></p>`) - test( + render( "", `<p><a href="`+result+`" target="_blank" rel="nofollow noopener"><img src="`+result+`" alt="`+title+`"/></a></p>`) - test( + render( "[["+title+"|"+url+"]]", `<p><a href="`+result+`" rel="nofollow"><img src="`+result+`" title="`+title+`" alt="`+title+`"/></a></p>`) - test( + render( "[]("+href+")", `<p><a href="`+href+`" rel="nofollow"><img src="`+result+`" alt="`+title+`"/></a></p>`) + + defer test.MockVariableValue(&markup.RenderBehaviorForTesting.DisableAdditionalAttributes, false)() + render( + "<a><img src='a.jpg'></a>", // by the way, empty "a" tag will be removed + `<p dir="auto"><img src="http://localhost:3000/user13/repo11/a.jpg" loading="lazy"/></p>`) } func TestTotal_RenderString(t *testing.T) { diff --git a/modules/markup/markdown/math/block_renderer.go b/modules/markup/markdown/math/block_renderer.go index 427ed842ec..95a336a02c 100644 --- a/modules/markup/markdown/math/block_renderer.go +++ b/modules/markup/markdown/math/block_renderer.go @@ -51,8 +51,8 @@ func (r *BlockRenderer) writeLines(w util.BufWriter, source []byte, n gast.Node) func (r *BlockRenderer) renderBlock(w util.BufWriter, source []byte, node gast.Node, entering bool) (gast.WalkStatus, error) { n := node.(*Block) if entering { - code := giteaUtil.Iif(n.Inline, "", `<pre class="code-block is-loading">`) + `<code class="language-math display">` - _ = r.renderInternal.FormatWithSafeAttrs(w, template.HTML(code)) + codeHTML := giteaUtil.Iif[template.HTML](n.Inline, "", `<pre class="code-block is-loading">`) + `<code class="language-math display">` + _, _ = w.WriteString(string(r.renderInternal.ProtectSafeAttrs(codeHTML))) r.writeLines(w, source, n) } else { _, _ = w.WriteString(`</code>` + giteaUtil.Iif(n.Inline, "", `</pre>`) + "\n") diff --git a/modules/markup/markdown/math/inline_renderer.go b/modules/markup/markdown/math/inline_renderer.go index d000a7b317..eeeb60cc7e 100644 --- a/modules/markup/markdown/math/inline_renderer.go +++ b/modules/markup/markdown/math/inline_renderer.go @@ -28,7 +28,7 @@ func NewInlineRenderer(renderInternal *internal.RenderInternal) renderer.NodeRen func (r *InlineRenderer) renderInline(w util.BufWriter, source []byte, n ast.Node, entering bool) (ast.WalkStatus, error) { if entering { - _ = r.renderInternal.FormatWithSafeAttrs(w, `<code class="language-math">`) + _, _ = w.WriteString(string(r.renderInternal.ProtectSafeAttrs(`<code class="language-math">`))) for c := n.FirstChild(); c != nil; c = c.NextSibling() { segment := c.(*ast.Text).Segment value := util.EscapeHTML(segment.Value(source)) diff --git a/modules/markup/sanitizer_default.go b/modules/markup/sanitizer_default.go index 14161eb533..0fbf0f0b24 100644 --- a/modules/markup/sanitizer_default.go +++ b/modules/markup/sanitizer_default.go @@ -4,6 +4,7 @@ package markup import ( + "html/template" "io" "net/url" "regexp" @@ -52,6 +53,8 @@ func (st *Sanitizer) createDefaultPolicy() *bluemonday.Policy { policy.AllowAttrs("src", "autoplay", "controls").OnElements("video") + policy.AllowAttrs("loading").OnElements("img") + // Allow generally safe attributes (reference: https://github.com/jch/html-pipeline) generalSafeAttrs := []string{ "abbr", "accept", "accept-charset", @@ -90,9 +93,9 @@ func (st *Sanitizer) createDefaultPolicy() *bluemonday.Policy { return policy } -// Sanitize takes a string that contains a HTML fragment or document and applies policy whitelist. -func Sanitize(s string) string { - return GetDefaultSanitizer().defaultPolicy.Sanitize(s) +// Sanitize use default sanitizer policy to sanitize a string +func Sanitize(s string) template.HTML { + return template.HTML(GetDefaultSanitizer().defaultPolicy.Sanitize(s)) } // SanitizeReader sanitizes a Reader diff --git a/modules/markup/sanitizer_default_test.go b/modules/markup/sanitizer_default_test.go index 5282916944..e5ba018e1b 100644 --- a/modules/markup/sanitizer_default_test.go +++ b/modules/markup/sanitizer_default_test.go @@ -69,6 +69,6 @@ func TestSanitizer(t *testing.T) { } for i := 0; i < len(testCases); i += 2 { - assert.Equal(t, testCases[i+1], Sanitize(testCases[i])) + assert.Equal(t, testCases[i+1], string(Sanitize(testCases[i]))) } } diff --git a/modules/structs/repo_file.go b/modules/structs/repo_file.go index b0e0bd979e..a281620a3b 100644 --- a/modules/structs/repo_file.go +++ b/modules/structs/repo_file.go @@ -66,6 +66,8 @@ func (o *UpdateFileOptions) Branch() string { return o.FileOptions.BranchName } +// FIXME: ChangeFileOperation.SHA is NOT required for update or delete if last commit is provided in the options. + // ChangeFileOperation for creating, updating or deleting a file type ChangeFileOperation struct { // indicates what to do with the file diff --git a/modules/templates/helper.go b/modules/templates/helper.go index d55d4f87c5..ff3f7cfda1 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -6,7 +6,6 @@ package templates import ( "fmt" - "html" "html/template" "net/url" "strconv" @@ -38,9 +37,7 @@ func NewFuncMap() template.FuncMap { "dict": dict, // it's lowercase because this name has been widely used. Our other functions should have uppercase names. "Iif": iif, "Eval": evalTokens, - "SafeHTML": safeHTML, "HTMLFormat": htmlFormat, - "HTMLEscape": htmlEscape, "QueryEscape": queryEscape, "QueryBuild": QueryBuild, "JSEscape": jsEscapeSafe, @@ -165,30 +162,9 @@ func NewFuncMap() template.FuncMap { } } -// safeHTML render raw as HTML -func safeHTML(s any) template.HTML { - switch v := s.(type) { - case string: - return template.HTML(v) - case template.HTML: - return v - } - panic(fmt.Sprintf("unexpected type %T", s)) -} - -// SanitizeHTML sanitizes the input by pre-defined markdown rules +// SanitizeHTML sanitizes the input by default sanitization rules. func SanitizeHTML(s string) template.HTML { - return template.HTML(markup.Sanitize(s)) -} - -func htmlEscape(s any) template.HTML { - switch v := s.(type) { - case string: - return template.HTML(html.EscapeString(v)) - case template.HTML: - return v - } - panic(fmt.Sprintf("unexpected type %T", s)) + return markup.Sanitize(s) } func htmlFormat(s any, args ...any) template.HTML { diff --git a/modules/web/router_path.go b/modules/web/router_path.go index 1531ccd01c..ce041eedab 100644 --- a/modules/web/router_path.go +++ b/modules/web/router_path.go @@ -6,6 +6,7 @@ package web import ( "net/http" "regexp" + "slices" "strings" "code.gitea.io/gitea/modules/container" @@ -36,11 +37,21 @@ func (g *RouterPathGroup) ServeHTTP(resp http.ResponseWriter, req *http.Request) g.r.chiRouter.NotFoundHandler().ServeHTTP(resp, req) } +type RouterPathGroupPattern struct { + re *regexp.Regexp + params []routerPathParam + middlewares []any +} + // MatchPath matches the request method, and uses regexp to match the path. -// The pattern uses "<...>" to define path parameters, for example: "/<name>" (different from chi router) -// It is only designed to resolve some special cases which chi router can't handle. +// The pattern uses "<...>" to define path parameters, for example, "/<name>" (different from chi router) +// It is only designed to resolve some special cases that chi router can't handle. // For most cases, it shouldn't be used because it needs to iterate all rules to find the matched one (inefficient). func (g *RouterPathGroup) MatchPath(methods, pattern string, h ...any) { + g.MatchPattern(methods, g.PatternRegexp(pattern), h...) +} + +func (g *RouterPathGroup) MatchPattern(methods string, pattern *RouterPathGroupPattern, h ...any) { g.matchers = append(g.matchers, newRouterPathMatcher(methods, pattern, h...)) } @@ -96,8 +107,8 @@ func isValidMethod(name string) bool { return false } -func newRouterPathMatcher(methods, pattern string, h ...any) *routerPathMatcher { - middlewares, handlerFunc := wrapMiddlewareAndHandler(nil, h) +func newRouterPathMatcher(methods string, patternRegexp *RouterPathGroupPattern, h ...any) *routerPathMatcher { + middlewares, handlerFunc := wrapMiddlewareAndHandler(patternRegexp.middlewares, h) p := &routerPathMatcher{methods: make(container.Set[string]), middlewares: middlewares, handlerFunc: handlerFunc} for method := range strings.SplitSeq(methods, ",") { method = strings.TrimSpace(method) @@ -106,19 +117,25 @@ func newRouterPathMatcher(methods, pattern string, h ...any) *routerPathMatcher } p.methods.Add(method) } + p.re, p.params = patternRegexp.re, patternRegexp.params + return p +} + +func patternRegexp(pattern string, h ...any) *RouterPathGroupPattern { + p := &RouterPathGroupPattern{middlewares: slices.Clone(h)} re := []byte{'^'} lastEnd := 0 for lastEnd < len(pattern) { start := strings.IndexByte(pattern[lastEnd:], '<') if start == -1 { - re = append(re, pattern[lastEnd:]...) + re = append(re, regexp.QuoteMeta(pattern[lastEnd:])...) break } end := strings.IndexByte(pattern[lastEnd+start:], '>') if end == -1 { panic("invalid pattern: " + pattern) } - re = append(re, pattern[lastEnd:lastEnd+start]...) + re = append(re, regexp.QuoteMeta(pattern[lastEnd:lastEnd+start])...) partName, partExp, _ := strings.Cut(pattern[lastEnd+start+1:lastEnd+start+end], ":") lastEnd += start + end + 1 @@ -140,7 +157,10 @@ func newRouterPathMatcher(methods, pattern string, h ...any) *routerPathMatcher p.params = append(p.params, param) } re = append(re, '$') - reStr := string(re) - p.re = regexp.MustCompile(reStr) + p.re = regexp.MustCompile(string(re)) return p } + +func (g *RouterPathGroup) PatternRegexp(pattern string, h ...any) *RouterPathGroupPattern { + return patternRegexp(pattern, h...) +} diff --git a/modules/web/router_test.go b/modules/web/router_test.go index 21619012ea..1cee2b879b 100644 --- a/modules/web/router_test.go +++ b/modules/web/router_test.go @@ -34,7 +34,7 @@ func TestPathProcessor(t *testing.T) { testProcess := func(pattern, uri string, expectedPathParams map[string]string) { chiCtx := chi.NewRouteContext() chiCtx.RouteMethod = "GET" - p := newRouterPathMatcher("GET", pattern, http.NotFound) + p := newRouterPathMatcher("GET", patternRegexp(pattern), http.NotFound) assert.True(t, p.matchPath(chiCtx, uri), "use pattern %s to process uri %s", pattern, uri) assert.Equal(t, expectedPathParams, chiURLParamsToMap(chiCtx), "use pattern %s to process uri %s", pattern, uri) } @@ -56,18 +56,20 @@ func TestRouter(t *testing.T) { recorder.Body = buff type resultStruct struct { - method string - pathParams map[string]string - handlerMark string + method string + pathParams map[string]string + handlerMarks []string } - var res resultStruct + var res resultStruct h := func(optMark ...string) func(resp http.ResponseWriter, req *http.Request) { mark := util.OptionalArg(optMark, "") return func(resp http.ResponseWriter, req *http.Request) { res.method = req.Method res.pathParams = chiURLParamsToMap(chi.RouteContext(req.Context())) - res.handlerMark = mark + if mark != "" { + res.handlerMarks = append(res.handlerMarks, mark) + } } } @@ -77,6 +79,8 @@ func TestRouter(t *testing.T) { if stop := req.FormValue("stop"); stop != "" && (mark == "" || mark == stop) { h(stop)(resp, req) resp.WriteHeader(http.StatusOK) + } else if mark != "" { + res.handlerMarks = append(res.handlerMarks, mark) } } } @@ -108,7 +112,7 @@ func TestRouter(t *testing.T) { m.Delete("", h()) }) m.PathGroup("/*", func(g *RouterPathGroup) { - g.MatchPath("GET", `/<dir:*>/<file:[a-z]{1,2}>`, stopMark("s2"), h("match-path")) + g.MatchPattern("GET", g.PatternRegexp(`/<dir:*>/<file:[a-z]{1,2}>`, stopMark("s2")), stopMark("s3"), h("match-path")) }, stopMark("s1")) }) }) @@ -126,31 +130,31 @@ func TestRouter(t *testing.T) { } t.Run("RootRouter", func(t *testing.T) { - testRoute(t, "GET /the-user/the-repo/other", resultStruct{method: "GET", handlerMark: "not-found:/"}) + testRoute(t, "GET /the-user/the-repo/other", resultStruct{method: "GET", handlerMarks: []string{"not-found:/"}}) testRoute(t, "GET /the-user/the-repo/pulls", resultStruct{ - method: "GET", - pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "type": "pulls"}, - handlerMark: "list-issues-b", + method: "GET", + pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "type": "pulls"}, + handlerMarks: []string{"list-issues-b"}, }) testRoute(t, "GET /the-user/the-repo/issues/123", resultStruct{ - method: "GET", - pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "type": "issues", "index": "123"}, - handlerMark: "view-issue", + method: "GET", + pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "type": "issues", "index": "123"}, + handlerMarks: []string{"view-issue"}, }) testRoute(t, "GET /the-user/the-repo/issues/123?stop=hijack", resultStruct{ - method: "GET", - pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "type": "issues", "index": "123"}, - handlerMark: "hijack", + method: "GET", + pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "type": "issues", "index": "123"}, + handlerMarks: []string{"hijack"}, }) testRoute(t, "POST /the-user/the-repo/issues/123/update", resultStruct{ - method: "POST", - pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "index": "123"}, - handlerMark: "update-issue", + method: "POST", + pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "index": "123"}, + handlerMarks: []string{"update-issue"}, }) }) t.Run("Sub Router", func(t *testing.T) { - testRoute(t, "GET /api/v1/other", resultStruct{method: "GET", handlerMark: "not-found:/api/v1"}) + testRoute(t, "GET /api/v1/other", resultStruct{method: "GET", handlerMarks: []string{"not-found:/api/v1"}}) testRoute(t, "GET /api/v1/repos/the-user/the-repo/branches", resultStruct{ method: "GET", pathParams: map[string]string{"username": "the-user", "reponame": "the-repo"}, @@ -179,31 +183,37 @@ func TestRouter(t *testing.T) { t.Run("MatchPath", func(t *testing.T) { testRoute(t, "GET /api/v1/repos/the-user/the-repo/branches/d1/d2/fn", resultStruct{ - method: "GET", - pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "*": "d1/d2/fn", "dir": "d1/d2", "file": "fn"}, - handlerMark: "match-path", + method: "GET", + pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "*": "d1/d2/fn", "dir": "d1/d2", "file": "fn"}, + handlerMarks: []string{"s1", "s2", "s3", "match-path"}, }) testRoute(t, "GET /api/v1/repos/the-user/the-repo/branches/d1%2fd2/fn", resultStruct{ - method: "GET", - pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "*": "d1%2fd2/fn", "dir": "d1%2fd2", "file": "fn"}, - handlerMark: "match-path", + method: "GET", + pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "*": "d1%2fd2/fn", "dir": "d1%2fd2", "file": "fn"}, + handlerMarks: []string{"s1", "s2", "s3", "match-path"}, }) testRoute(t, "GET /api/v1/repos/the-user/the-repo/branches/d1/d2/000", resultStruct{ - method: "GET", - pathParams: map[string]string{"reponame": "the-repo", "username": "the-user", "*": "d1/d2/000"}, - handlerMark: "not-found:/api/v1", + method: "GET", + pathParams: map[string]string{"reponame": "the-repo", "username": "the-user", "*": "d1/d2/000"}, + handlerMarks: []string{"s1", "not-found:/api/v1"}, }) testRoute(t, "GET /api/v1/repos/the-user/the-repo/branches/d1/d2/fn?stop=s1", resultStruct{ - method: "GET", - pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "*": "d1/d2/fn"}, - handlerMark: "s1", + method: "GET", + pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "*": "d1/d2/fn"}, + handlerMarks: []string{"s1"}, }) testRoute(t, "GET /api/v1/repos/the-user/the-repo/branches/d1/d2/fn?stop=s2", resultStruct{ - method: "GET", - pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "*": "d1/d2/fn", "dir": "d1/d2", "file": "fn"}, - handlerMark: "s2", + method: "GET", + pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "*": "d1/d2/fn", "dir": "d1/d2", "file": "fn"}, + handlerMarks: []string{"s1", "s2"}, + }) + + testRoute(t, "GET /api/v1/repos/the-user/the-repo/branches/d1/d2/fn?stop=s3", resultStruct{ + method: "GET", + pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "*": "d1/d2/fn", "dir": "d1/d2", "file": "fn"}, + handlerMarks: []string{"s1", "s2", "s3"}, }) }) } |