diff --git a/Dockerfile b/Dockerfile index f71b13e8f3c..9922cee9c41 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,14 +3,14 @@ FROM --platform=$BUILDPLATFORM docker.io/library/golang:1.26-alpine3.23 AS frontend-build RUN apk --no-cache add build-base git nodejs pnpm WORKDIR /src +COPY package.json pnpm-lock.yaml .npmrc ./ +RUN --mount=type=cache,target=/root/.local/share/pnpm/store pnpm install --frozen-lockfile COPY --exclude=.git/ . . -RUN --mount=type=cache,target=/root/.local/share/pnpm/store make frontend +RUN make frontend # Build backend for each target platform FROM docker.io/library/golang:1.26-alpine3.23 AS build-env -ARG GOPROXY=direct - ARG GITEA_VERSION ARG TAGS="sqlite sqlite_unlock_notify" ENV TAGS="bindata timetzdata $TAGS" @@ -22,14 +22,15 @@ RUN apk --no-cache add \ git WORKDIR ${GOPATH}/src/code.gitea.io/gitea +COPY go.mod go.sum ./ +RUN go mod download # Use COPY instead of bind mount as read-only one breaks makefile state tracking and read-write one needs binary to be moved as it's discarded. # ".git" directory is mounted separately later only for version data extraction. COPY --exclude=.git/ . . COPY --from=frontend-build /src/public/assets public/assets # Build gitea, .git mount is required for version data -RUN --mount=type=cache,target=/go/pkg/mod \ - --mount=type=cache,target="/root/.cache/go-build" \ +RUN --mount=type=cache,target="/root/.cache/go-build" \ --mount=type=bind,source=".git/",target=".git/" \ make backend diff --git a/Dockerfile.rootless b/Dockerfile.rootless index bc210132c53..a1742e3d51f 100644 --- a/Dockerfile.rootless +++ b/Dockerfile.rootless @@ -3,14 +3,14 @@ FROM --platform=$BUILDPLATFORM docker.io/library/golang:1.26-alpine3.23 AS frontend-build RUN apk --no-cache add build-base git nodejs pnpm WORKDIR /src +COPY package.json pnpm-lock.yaml .npmrc ./ +RUN --mount=type=cache,target=/root/.local/share/pnpm/store pnpm install --frozen-lockfile COPY --exclude=.git/ . . -RUN --mount=type=cache,target=/root/.local/share/pnpm/store make frontend +RUN make frontend # Build backend for each target platform FROM docker.io/library/golang:1.26-alpine3.23 AS build-env -ARG GOPROXY=direct - ARG GITEA_VERSION ARG TAGS="sqlite sqlite_unlock_notify" ENV TAGS="bindata timetzdata $TAGS" @@ -22,13 +22,14 @@ RUN apk --no-cache add \ git WORKDIR ${GOPATH}/src/code.gitea.io/gitea +COPY go.mod go.sum ./ +RUN go mod download # See the comments in Dockerfile COPY --exclude=.git/ . . COPY --from=frontend-build /src/public/assets public/assets # Build gitea, .git mount is required for version data -RUN --mount=type=cache,target=/go/pkg/mod \ - --mount=type=cache,target="/root/.cache/go-build" \ +RUN --mount=type=cache,target="/root/.cache/go-build" \ --mount=type=bind,source=".git/",target=".git/" \ make backend diff --git a/modules/charset/escape_stream.go b/modules/charset/escape_stream.go index 29943eb8580..22e7f14f39f 100644 --- a/modules/charset/escape_stream.go +++ b/modules/charset/escape_stream.go @@ -61,12 +61,14 @@ func (e *escapeStreamer) Text(data string) error { until = len(data) next = until } else { - until, next = nextIdxs[0]+pos, nextIdxs[1]+pos + until = min(nextIdxs[0]+pos, len(data)) + next = min(nextIdxs[1]+pos, len(data)) } // from pos until we know that the runes are not \r\t\n or even ' ' - runes := make([]rune, 0, next-until) - positions := make([]int, 0, next-until+1) + n := next - until + runes := make([]rune, 0, n) + positions := make([]int, 0, n+1) for pos < until { r, sz := utf8.DecodeRune(dataBytes[pos:]) diff --git a/modules/indexer/code/gitgrep/gitgrep.go b/modules/indexer/code/gitgrep/gitgrep.go index 6f6e0b47b9e..5fbd7201ef0 100644 --- a/modules/indexer/code/gitgrep/gitgrep.go +++ b/modules/indexer/code/gitgrep/gitgrep.go @@ -24,7 +24,7 @@ func indexSettingToGitGrepPathspecList() (list []string) { return list } -func PerformSearch(ctx context.Context, page int, repoID int64, gitRepo *git.Repository, ref git.RefName, keyword string, searchMode indexer.SearchModeType) (searchResults []*code_indexer.Result, total int, err error) { +func PerformSearch(ctx context.Context, page int, repoID int64, gitRepo *git.Repository, ref git.RefName, keyword string, searchMode indexer.SearchModeType) (searchResults []*code_indexer.Result, total int64, err error) { grepMode := git.GrepModeWords switch searchMode { case indexer.SearchModeExact: @@ -47,7 +47,7 @@ func PerformSearch(ctx context.Context, page int, repoID int64, gitRepo *git.Rep return nil, 0, fmt.Errorf("gitRepo.GetRefCommitID: %w", err) } - total = len(res) + total = int64(len(res)) pageStart := min((page-1)*setting.UI.RepoSearchPagingNum, len(res)) pageEnd := min(page*setting.UI.RepoSearchPagingNum, len(res)) res = res[pageStart:pageEnd] diff --git a/modules/indexer/code/search.go b/modules/indexer/code/search.go index eb20b70e71e..009d659d761 100644 --- a/modules/indexer/code/search.go +++ b/modules/indexer/code/search.go @@ -130,7 +130,7 @@ func searchResult(result *internal.SearchResult, startIndex, endIndex int) (*Res } // PerformSearch perform a search on a repository -func PerformSearch(ctx context.Context, opts *SearchOptions) (int, []*Result, []*SearchResultLanguages, error) { +func PerformSearch(ctx context.Context, opts *SearchOptions) (int64, []*Result, []*SearchResultLanguages, error) { if opts == nil || len(opts.Keyword) == 0 { return 0, nil, nil, nil } @@ -149,5 +149,5 @@ func PerformSearch(ctx context.Context, opts *SearchOptions) (int, []*Result, [] return 0, nil, nil, err } } - return int(total), displayResults, resultLanguages, nil + return total, displayResults, resultLanguages, nil } diff --git a/modules/markup/camo.go b/modules/markup/camo.go index 7e2583469d3..f07d62d4f91 100644 --- a/modules/markup/camo.go +++ b/modules/markup/camo.go @@ -11,7 +11,6 @@ import ( "strings" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/util" ) // CamoEncode encodes a lnk to fit with the go-camo and camo proxy links. The purposes of camo-proxy are: @@ -27,7 +26,7 @@ func CamoEncode(link string) string { macSum := b64encode(mac.Sum(nil)) encodedURL := b64encode([]byte(link)) - return util.URLJoin(setting.Camo.ServerURL, macSum, encodedURL) + return strings.TrimSuffix(setting.Camo.ServerURL, "/") + "/" + macSum + "/" + encodedURL } func b64encode(data []byte) string { diff --git a/modules/markup/html_commit.go b/modules/markup/html_commit.go index c319374a38d..a2c5160674f 100644 --- a/modules/markup/html_commit.go +++ b/modules/markup/html_commit.go @@ -4,13 +4,13 @@ package markup import ( + "fmt" "slices" "strings" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/httplib" "code.gitea.io/gitea/modules/references" - "code.gitea.io/gitea/modules/util" "golang.org/x/net/html" "golang.org/x/net/html/atom" @@ -219,7 +219,7 @@ func hashCurrentPatternProcessor(ctx *RenderContext, node *html.Node) { continue } - link := "/:root/" + util.URLJoin(ctx.RenderOptions.Metas["user"], ctx.RenderOptions.Metas["repo"], "commit", hash) + link := fmt.Sprintf("/:root/%s/%s/commit/%s", ctx.RenderOptions.Metas["user"], ctx.RenderOptions.Metas["repo"], hash) replaceContent(node, m[2], m[3], createCodeLink(link, base.ShortSha(hash), "commit")) start = 0 node = node.NextSibling.NextSibling @@ -236,7 +236,7 @@ func commitCrossReferencePatternProcessor(ctx *RenderContext, node *html.Node) { } refText := ref.Owner + "/" + ref.Name + "@" + base.ShortSha(ref.CommitSha) - linkHref := "/:root/" + util.URLJoin(ref.Owner, ref.Name, "commit", ref.CommitSha) + linkHref := fmt.Sprintf("/:root/%s/%s/commit/%s", ref.Owner, ref.Name, ref.CommitSha) link := createLink(ctx, linkHref, refText, "commit") replaceContent(node, ref.RefLocation.Start, ref.RefLocation.End, link) diff --git a/modules/markup/html_issue.go b/modules/markup/html_issue.go index 85bec5db20c..a94abb38831 100644 --- a/modules/markup/html_issue.go +++ b/modules/markup/html_issue.go @@ -4,6 +4,7 @@ package markup import ( + "fmt" "strconv" "strings" @@ -162,7 +163,7 @@ func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) { issueOwner := util.Iif(ref.Owner == "", ctx.RenderOptions.Metas["user"], ref.Owner) issueRepo := util.Iif(ref.Owner == "", ctx.RenderOptions.Metas["repo"], ref.Name) issuePath := util.Iif(ref.IsPull, "pulls", "issues") - linkHref := "/:root/" + util.URLJoin(issueOwner, issueRepo, issuePath, ref.Issue) + linkHref := fmt.Sprintf("/:root/%s/%s/%s/%s", issueOwner, issueRepo, issuePath, ref.Issue) // at the moment, only render the issue index in a full line (or simple line) as icon+title // otherwise it would be too noisy for "take #1 as an example" in a sentence diff --git a/modules/markup/html_link.go b/modules/markup/html_link.go index 1702950da8c..c84e0b90d81 100644 --- a/modules/markup/html_link.go +++ b/modules/markup/html_link.go @@ -113,16 +113,17 @@ func shortLinkProcessor(ctx *RenderContext, node *html.Node) { } childNode.Parent = linkNode absoluteLink := IsFullURLString(link) - if !absoluteLink { + // FIXME: it should be fully refactored in the future, it uses various hacky approaches to guess how to encode a path for wiki + // When a link contains "/", then we assume that the user has provided a well-encoded link. + if !absoluteLink && !strings.Contains(link, "/") { + // So only guess for links without "/". if image { link = strings.ReplaceAll(link, " ", "+") } else { // the hacky wiki name encoding: space to "-" link = strings.ReplaceAll(link, " ", "-") // FIXME: it should support dashes in the link, eg: "the-dash-support.-" } - if !strings.Contains(link, "/") { - link = url.PathEscape(link) // FIXME: it doesn't seem right and it might cause double-escaping - } + link = url.PathEscape(link) } if image { title := props["title"] diff --git a/modules/markup/html_mention.go b/modules/markup/html_mention.go index f97c034cf3b..00cd51ca949 100644 --- a/modules/markup/html_mention.go +++ b/modules/markup/html_mention.go @@ -4,6 +4,7 @@ package markup import ( + "fmt" "strings" "code.gitea.io/gitea/modules/references" @@ -26,14 +27,11 @@ func mentionProcessor(ctx *RenderContext, node *html.Node) { loc.End += start mention := node.Data[loc.Start:loc.End] teams, ok := ctx.RenderOptions.Metas["teams"] - // FIXME: util.URLJoin may not be necessary here: - // - setting.AppURL is defined to have a terminal '/' so unless mention[1:] - // is an AppSubURL link we can probably fallback to concatenation. - // team mention should follow @orgName/teamName style + if ok && strings.Contains(mention, "/") { mentionOrgAndTeam := strings.Split(mention, "/") if mentionOrgAndTeam[0][1:] == ctx.RenderOptions.Metas["org"] && strings.Contains(teams, ","+strings.ToLower(mentionOrgAndTeam[1])+",") { - link := "/:root/" + util.URLJoin("org", ctx.RenderOptions.Metas["org"], "teams", mentionOrgAndTeam[1]) + link := fmt.Sprintf("/:root/org/%s/teams/%s", ctx.RenderOptions.Metas["org"], mentionOrgAndTeam[1]) replaceContent(node, loc.Start, loc.End, createLink(ctx, link, mention, "" /*mention*/)) node = node.NextSibling.NextSibling start = 0 diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go index 5f873d29852..62c4ae3e0a2 100644 --- a/modules/markup/html_test.go +++ b/modules/markup/html_test.go @@ -389,7 +389,7 @@ func TestRender_ShortLinks(t *testing.T) { imgurl := util.URLJoin(tree, "Link.jpg") otherImgurl := util.URLJoin(tree, "Link+Other.jpg") encodedImgurl := util.URLJoin(tree, "Link+%23.jpg") - notencodedImgurl := util.URLJoin(tree, "some", "path", "Link+#.jpg") + notencodedImgurl := util.URLJoin(tree, "some", "path", "Link%20#.jpg") renderableFileURL := util.URLJoin(tree, "markdown_file.md") unrenderableFileURL := util.URLJoin(tree, "file.zip") favicon := "http://google.com/favicon.ico" @@ -466,6 +466,8 @@ func TestRender_ShortLinks(t *testing.T) { "[[Name|Link #.jpg|alt=\"AltName\"|title='Title']]", `
`, ) + // FIXME: it's unable to resolve: [[link?k=v]] + // FIXME: it is a wrong test case, it is not an image, but a link with anchor "#.jpg" test( "[[some/path/Link #.jpg]]", ``, diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go index 8d6b3b3c80e..261c4e780c9 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/modules/markup/markdown/markdown_test.go @@ -14,7 +14,6 @@ import ( "code.gitea.io/gitea/modules/markup/markdown" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/test" - "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" ) @@ -23,7 +22,6 @@ const ( AppURL = "http://localhost:3000/" testRepoOwnerName = "user13" testRepoName = "repo11" - FullURL = AppURL + testRepoOwnerName + "/" + testRepoName + "/" ) // these values should match the const above @@ -47,8 +45,9 @@ func TestRender_StandardLinks(t *testing.T) { func TestRender_Images(t *testing.T) { setting.AppURL = AppURL + const baseLink = "http://localhost:3000/user13/repo11" render := func(input, expected string) { - buffer, err := markdown.RenderString(markup.NewTestRenderContext(FullURL), input) + buffer, err := markdown.RenderString(markup.NewTestRenderContext(baseLink), input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(buffer))) } @@ -56,7 +55,7 @@ func TestRender_Images(t *testing.T) { url := "../../.images/src/02/train.jpg" title := "Train" href := "https://gitea.io" - result := util.URLJoin(FullURL, url) + result := baseLink + "/.images/src/02/train.jpg" // resolved link should not go out of the base link // hint: With Markdown v2.5.2, there is a new syntax: [link](URL){:target="_blank"} , but we do not support it now render( @@ -88,6 +87,7 @@ func TestRender_Images(t *testing.T) { } func TestTotal_RenderString(t *testing.T) { + const FullURL = AppURL + testRepoOwnerName + "/" + testRepoName + "/" setting.AppURL = AppURL defer test.MockVariableValue(&markup.RenderBehaviorForTesting.DisableAdditionalAttributes, true)() diff --git a/modules/markup/render_link.go b/modules/markup/render_link.go index 9cc83095ffa..c3a95622ace 100644 --- a/modules/markup/render_link.go +++ b/modules/markup/render_link.go @@ -5,28 +5,47 @@ package markup import ( "context" + "net/url" + "path" "strings" "code.gitea.io/gitea/modules/httplib" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/util" ) +// resolveLinkRelative tries to resolve the link relative to the "{base}/{cur}", and returns the final link. +// It only resolves the link, doesn't do any sanitization or validation, invalid links will be returned as is. func resolveLinkRelative(ctx context.Context, base, cur, link string, absolute bool) (finalLink string) { - if IsFullURLString(link) { - return link + linkURL, err := url.Parse(link) + if err != nil { + return link // invalid URL, return as is } + if linkURL.Scheme != "" || linkURL.Host != "" { + return link // absolute URL, return as is + } + if strings.HasPrefix(link, "/") { if strings.HasPrefix(link, base) && strings.Count(base, "/") >= 4 { - // a trick to tolerate that some users were using absolute paths (the old gitea's behavior) + // a trick to tolerate that some users were using absolute paths (the old Gitea's behavior) + // if the link is likely "{base}/src/main" while "{base}" is something like "/owner/repo" finalLink = link } else { - finalLink = util.URLJoin(base, "./", link) + // need to resolve the link relative to "{base}" + cur = "" + } + } // else: link is relative to "{base}/{cur}" + + if finalLink == "" { + finalLink = strings.TrimSuffix(base, "/") + path.Join("/"+cur, "/"+linkURL.EscapedPath()) + finalLink = strings.TrimSuffix(finalLink, "/") + if linkURL.RawQuery != "" { + finalLink += "?" + linkURL.RawQuery + } + if linkURL.Fragment != "" { + finalLink += "#" + linkURL.Fragment } - } else { - finalLink = util.URLJoin(base, "./", cur, link) } - finalLink = strings.TrimSuffix(finalLink, "/") + if absolute { finalLink = httplib.MakeAbsoluteURL(ctx, finalLink) } diff --git a/modules/markup/render_link_test.go b/modules/markup/render_link_test.go index 972e15308cf..045b728c8d1 100644 --- a/modules/markup/render_link_test.go +++ b/modules/markup/render_link_test.go @@ -18,8 +18,16 @@ func TestResolveLinkRelative(t *testing.T) { assert.Equal(t, "/a/b", resolveLinkRelative(ctx, "/a", "b", "", false)) assert.Equal(t, "/a/b/c", resolveLinkRelative(ctx, "/a", "b", "c", false)) assert.Equal(t, "/a/c", resolveLinkRelative(ctx, "/a", "b", "/c", false)) + assert.Equal(t, "/a/c#id", resolveLinkRelative(ctx, "/a", "b", "/c#id", false)) + assert.Equal(t, "/a/%2f?k=/", resolveLinkRelative(ctx, "/a", "b", "/%2f/?k=/", false)) + assert.Equal(t, "/a/b/c?k=v#id", resolveLinkRelative(ctx, "/a", "b", "c/?k=v#id", false)) + assert.Equal(t, "%invalid", resolveLinkRelative(ctx, "/a", "b", "%invalid", false)) assert.Equal(t, "http://localhost:3000/a", resolveLinkRelative(ctx, "/a", "", "", true)) + // absolute link is returned as is + assert.Equal(t, "mailto:user@domain.com", resolveLinkRelative(ctx, "/a", "", "mailto:user@domain.com", false)) + assert.Equal(t, "http://other/path/", resolveLinkRelative(ctx, "/a", "", "http://other/path/", false)) + // some users might have used absolute paths a lot, so if the prefix overlaps and has enough slashes, we should tolerate it assert.Equal(t, "/owner/repo/foo/owner/repo/foo/bar/xxx", resolveLinkRelative(ctx, "/owner/repo/foo", "", "/owner/repo/foo/bar/xxx", false)) assert.Equal(t, "/owner/repo/foo/bar/xxx", resolveLinkRelative(ctx, "/owner/repo/foo/bar", "", "/owner/repo/foo/bar/xxx", false)) diff --git a/modules/recaptcha/recaptcha.go b/modules/recaptcha/recaptcha.go index 1777d169c10..224fa38eeeb 100644 --- a/modules/recaptcha/recaptcha.go +++ b/modules/recaptcha/recaptcha.go @@ -13,7 +13,6 @@ import ( "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/util" ) // Response is the structure of JSON returned from API @@ -24,17 +23,16 @@ type Response struct { ErrorCodes []ErrorCode `json:"error-codes"` } -const apiURL = "api/siteverify" - // Verify calls Google Recaptcha API to verify token func Verify(ctx context.Context, response string) (bool, error) { post := url.Values{ "secret": {setting.Service.RecaptchaSecret}, "response": {response}, } + + reqURL := strings.TrimSuffix(setting.Service.RecaptchaURL, "/") + "/api/siteverify" // Basically a copy of http.PostForm, but with a context - req, err := http.NewRequestWithContext(ctx, http.MethodPost, - util.URLJoin(setting.Service.RecaptchaURL, apiURL), strings.NewReader(post.Encode())) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, strings.NewReader(post.Encode())) if err != nil { return false, fmt.Errorf("Failed to create CAPTCHA request: %w", err) } diff --git a/modules/setting/server.go b/modules/setting/server.go index 50a38f544ba..7e7611b802d 100644 --- a/modules/setting/server.go +++ b/modules/setting/server.go @@ -15,7 +15,6 @@ import ( "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/util" ) // Scheme describes protocol types @@ -163,7 +162,7 @@ func MakeManifestData(appName, appURL, absoluteAssetURL string) []byte { } // MakeAbsoluteAssetURL returns the absolute asset url prefix without a trailing slash -func MakeAbsoluteAssetURL(appURL, staticURLPrefix string) string { +func MakeAbsoluteAssetURL(appURL *url.URL, staticURLPrefix string) string { parsedPrefix, err := url.Parse(strings.TrimSuffix(staticURLPrefix, "/")) if err != nil { log.Fatal("Unable to parse STATIC_URL_PREFIX: %v", err) @@ -171,11 +170,12 @@ func MakeAbsoluteAssetURL(appURL, staticURLPrefix string) string { if err == nil && parsedPrefix.Hostname() == "" { if staticURLPrefix == "" { - return strings.TrimSuffix(appURL, "/") + return strings.TrimSuffix(appURL.String(), "/") } // StaticURLPrefix is just a path - return util.URLJoin(appURL, strings.TrimSuffix(staticURLPrefix, "/")) + appHostURL := &url.URL{Scheme: appURL.Scheme, Host: appURL.Host} + return appHostURL.String() + "/" + strings.Trim(staticURLPrefix, "/") } return strings.TrimSuffix(staticURLPrefix, "/") @@ -316,7 +316,7 @@ func loadServerFrom(rootCfg ConfigProvider) { Domain = urlHostname } - AbsoluteAssetURL = MakeAbsoluteAssetURL(AppURL, StaticURLPrefix) + AbsoluteAssetURL = MakeAbsoluteAssetURL(appURL, StaticURLPrefix) AssetVersion = strings.ReplaceAll(AppVer, "+", "~") // make sure the version string is clear (no real escaping is needed) manifestBytes := MakeManifestData(AppName, AppURL, AbsoluteAssetURL) diff --git a/modules/setting/setting_test.go b/modules/setting/setting_test.go index f77ee659748..13575f52a6e 100644 --- a/modules/setting/setting_test.go +++ b/modules/setting/setting_test.go @@ -4,6 +4,7 @@ package setting import ( + "net/url" "testing" "code.gitea.io/gitea/modules/json" @@ -12,18 +13,26 @@ import ( ) func TestMakeAbsoluteAssetURL(t *testing.T) { - assert.Equal(t, "https://localhost:2345", MakeAbsoluteAssetURL("https://localhost:1234", "https://localhost:2345")) - assert.Equal(t, "https://localhost:2345", MakeAbsoluteAssetURL("https://localhost:1234/", "https://localhost:2345")) - assert.Equal(t, "https://localhost:2345", MakeAbsoluteAssetURL("https://localhost:1234/", "https://localhost:2345/")) - assert.Equal(t, "https://localhost:1234/foo", MakeAbsoluteAssetURL("https://localhost:1234", "/foo")) - assert.Equal(t, "https://localhost:1234/foo", MakeAbsoluteAssetURL("https://localhost:1234/", "/foo")) - assert.Equal(t, "https://localhost:1234/foo", MakeAbsoluteAssetURL("https://localhost:1234/", "/foo/")) - assert.Equal(t, "https://localhost:1234/foo", MakeAbsoluteAssetURL("https://localhost:1234/foo", "/foo")) - assert.Equal(t, "https://localhost:1234/foo", MakeAbsoluteAssetURL("https://localhost:1234/foo/", "/foo")) - assert.Equal(t, "https://localhost:1234/foo", MakeAbsoluteAssetURL("https://localhost:1234/foo/", "/foo/")) - assert.Equal(t, "https://localhost:1234/bar", MakeAbsoluteAssetURL("https://localhost:1234/foo", "/bar")) - assert.Equal(t, "https://localhost:1234/bar", MakeAbsoluteAssetURL("https://localhost:1234/foo/", "/bar")) - assert.Equal(t, "https://localhost:1234/bar", MakeAbsoluteAssetURL("https://localhost:1234/foo/", "/bar/")) + appURL1, _ := url.Parse("https://localhost:1234") + appURL2, _ := url.Parse("https://localhost:1234/") + appURLSub1, _ := url.Parse("https://localhost:1234/foo") + appURLSub2, _ := url.Parse("https://localhost:1234/foo/") + + // static URL is an absolute URL, so should be used + assert.Equal(t, "https://localhost:2345", MakeAbsoluteAssetURL(appURL1, "https://localhost:2345")) + assert.Equal(t, "https://localhost:2345", MakeAbsoluteAssetURL(appURL1, "https://localhost:2345/")) + + assert.Equal(t, "https://localhost:1234/foo", MakeAbsoluteAssetURL(appURL1, "/foo")) + assert.Equal(t, "https://localhost:1234/foo", MakeAbsoluteAssetURL(appURL2, "/foo")) + assert.Equal(t, "https://localhost:1234/foo", MakeAbsoluteAssetURL(appURL1, "/foo/")) + + assert.Equal(t, "https://localhost:1234/foo", MakeAbsoluteAssetURL(appURLSub1, "/foo")) + assert.Equal(t, "https://localhost:1234/foo", MakeAbsoluteAssetURL(appURLSub2, "/foo")) + assert.Equal(t, "https://localhost:1234/foo", MakeAbsoluteAssetURL(appURLSub1, "/foo/")) + + assert.Equal(t, "https://localhost:1234/bar", MakeAbsoluteAssetURL(appURLSub1, "/bar")) + assert.Equal(t, "https://localhost:1234/bar", MakeAbsoluteAssetURL(appURLSub2, "/bar")) + assert.Equal(t, "https://localhost:1234/bar", MakeAbsoluteAssetURL(appURLSub1, "/bar/")) } func TestMakeManifestData(t *testing.T) { diff --git a/modules/templates/helper.go b/modules/templates/helper.go index c1ee88fc84d..d2d4d364df0 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -37,7 +37,6 @@ func NewFuncMap() template.FuncMap { "QueryEscape": queryEscape, "QueryBuild": QueryBuild, "SanitizeHTML": SanitizeHTML, - "URLJoin": util.URLJoin, "DotEscape": dotEscape, "PathEscape": url.PathEscape, diff --git a/modules/templates/htmlrenderer.go b/modules/templates/htmlrenderer.go index 59b95cdd807..ca85a2ddeea 100644 --- a/modules/templates/htmlrenderer.go +++ b/modules/templates/htmlrenderer.go @@ -89,7 +89,7 @@ func (p *templateErrorPrettier) handleGenericTemplateError(err error) string { return "" } tmplName, lineStr, message := groups[1], groups[2], groups[3] - return p.makeDetailedError(message, tmplName, lineStr, -1, "") + return p.makeDetailedError(message, tmplName, lineStr, "", "") } var reFuncNotDefinedError = regexp.MustCompile(`^template: (.*):([0-9]+): (function "(.*)" not defined)`) @@ -101,7 +101,7 @@ func (p *templateErrorPrettier) handleFuncNotDefinedError(err error) string { } tmplName, lineStr, message, funcName := groups[1], groups[2], groups[3], groups[4] funcName, _ = strconv.Unquote(`"` + funcName + `"`) - return p.makeDetailedError(message, tmplName, lineStr, -1, funcName) + return p.makeDetailedError(message, tmplName, lineStr, "", funcName) } var reUnexpectedOperandError = regexp.MustCompile(`^template: (.*):([0-9]+): (unexpected "(.*)" in operand)`) @@ -113,7 +113,7 @@ func (p *templateErrorPrettier) handleUnexpectedOperandError(err error) string { } tmplName, lineStr, message, unexpected := groups[1], groups[2], groups[3], groups[4] unexpected, _ = strconv.Unquote(`"` + unexpected + `"`) - return p.makeDetailedError(message, tmplName, lineStr, -1, unexpected) + return p.makeDetailedError(message, tmplName, lineStr, "", unexpected) } var reExpectedEndError = regexp.MustCompile(`^template: (.*):([0-9]+): (expected end; found (.*))`) @@ -124,7 +124,7 @@ func (p *templateErrorPrettier) handleExpectedEndError(err error) string { return "" } tmplName, lineStr, message, unexpected := groups[1], groups[2], groups[3], groups[4] - return p.makeDetailedError(message, tmplName, lineStr, -1, unexpected) + return p.makeDetailedError(message, tmplName, lineStr, "", unexpected) } var ( @@ -154,20 +154,20 @@ func HandleTemplateRenderingError(err error) string { const dashSeparator = "----------------------------------------------------------------------" -func (p *templateErrorPrettier) makeDetailedError(errMsg, tmplName string, lineNum, posNum any, target string) string { +func (p *templateErrorPrettier) makeDetailedError(errMsg, tmplName, lineNumStr, posNumStr, target string) string { code, layer, err := p.assets.ReadLayeredFile(tmplName + ".tmpl") if err != nil { return fmt.Sprintf("template error: %s, and unable to find template file %q", errMsg, tmplName) } - line, err := util.ToInt64(lineNum) + line, err := strconv.Atoi(lineNumStr) if err != nil { - return fmt.Sprintf("template error: %s, unable to parse template %q line number %q", errMsg, tmplName, lineNum) + return fmt.Sprintf("template error: %s, unable to parse template %q line number %s", errMsg, tmplName, lineNumStr) } - pos, err := util.ToInt64(posNum) + pos, err := strconv.Atoi(util.IfZero(posNumStr, "-1")) if err != nil { - return fmt.Sprintf("template error: %s, unable to parse template %q pos number %q", errMsg, tmplName, posNum) + return fmt.Sprintf("template error: %s, unable to parse template %q pos number %s", errMsg, tmplName, posNumStr) } - detail := extractErrorLine(code, int(line), int(pos), target) + detail := extractErrorLine(code, line, pos, target) var msg string if pos >= 0 { diff --git a/modules/util/url.go b/modules/util/url.go index 62370339c8d..6455b0b75c1 100644 --- a/modules/util/url.go +++ b/modules/util/url.go @@ -20,6 +20,8 @@ func PathEscapeSegments(path string) string { } // URLJoin joins url components, like path.Join, but preserving contents +// Deprecated: it has unclear behaviors, should not be used anymore. It is only used in some tests. +// Need to be removed in the future. func URLJoin(base string, elems ...string) string { if !strings.HasSuffix(base, "/") { base += "/" diff --git a/modules/web/handler.go b/modules/web/handler.go index 843b17e8d1b..d113bbba451 100644 --- a/modules/web/handler.go +++ b/modules/web/handler.go @@ -70,7 +70,8 @@ func preCheckHandler(fn reflect.Value, argsIn []reflect.Value) { func prepareHandleArgsIn(resp http.ResponseWriter, req *http.Request, fn reflect.Value, fnInfo *routing.FuncInfo) []reflect.Value { defer func() { - if err := recover(); err != nil { + if recovered := recover(); recovered != nil { + err := fmt.Errorf("%v\n%s", recovered, log.Stack(2)) log.Error("unable to prepare handler arguments for %s: %v", fnInfo.String(), err) panic(err) } @@ -117,7 +118,17 @@ func hasResponseBeenWritten(argsIn []reflect.Value) bool { return false } -func wrapHandlerProvider[T http.Handler](hp func(next http.Handler) T, funcInfo *routing.FuncInfo) func(next http.Handler) http.Handler { +type middlewareProvider = func(next http.Handler) http.Handler + +func executeMiddlewaresHandler(w http.ResponseWriter, r *http.Request, middlewares []middlewareProvider, endpoint http.HandlerFunc) { + handler := endpoint + for i := len(middlewares) - 1; i >= 0; i-- { + handler = middlewares[i](handler).ServeHTTP + } + handler(w, r) +} + +func wrapHandlerProvider[T http.Handler](hp func(next http.Handler) T, funcInfo *routing.FuncInfo) middlewareProvider { return func(next http.Handler) http.Handler { h := hp(next) // this handle could be dynamically generated, so we can't use it for debug info return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { @@ -129,14 +140,14 @@ func wrapHandlerProvider[T http.Handler](hp func(next http.Handler) T, funcInfo // toHandlerProvider converts a handler to a handler provider // A handler provider is a function that takes a "next" http.Handler, it can be used as a middleware -func toHandlerProvider(handler any) func(next http.Handler) http.Handler { +func toHandlerProvider(handler any) middlewareProvider { funcInfo := routing.GetFuncInfo(handler) fn := reflect.ValueOf(handler) if fn.Type().Kind() != reflect.Func { panic(fmt.Sprintf("handler must be a function, but got %s", fn.Type())) } - if hp, ok := handler.(func(next http.Handler) http.Handler); ok { + if hp, ok := handler.(middlewareProvider); ok { return wrapHandlerProvider(hp, funcInfo) } else if hp, ok := handler.(func(http.Handler) http.HandlerFunc); ok { return wrapHandlerProvider(hp, funcInfo) diff --git a/modules/web/router.go b/modules/web/router.go index 5374f82a23c..5ef18e96795 100644 --- a/modules/web/router.go +++ b/modules/web/router.go @@ -18,6 +18,13 @@ import ( "github.com/go-chi/chi/v5" ) +// PreMiddlewareProvider is a special middleware provider which will be executed +// before other middlewares on the same "routing" level (AfterRouting/Group/Methods/Any, but not BeforeRouting). +// A route can do something (e.g.: set middleware options) at the place where it is declared, +// and the code will be executed before other middlewares which are added before the declaration. +// Use cases: mark a route with some meta info, set some options for middlewares, etc. +type PreMiddlewareProvider func(next http.Handler) http.Handler + // Bind binding an obj to a handler's context data func Bind[T any](_ T) http.HandlerFunc { return func(resp http.ResponseWriter, req *http.Request) { @@ -41,7 +48,10 @@ func GetForm(dataStore reqctx.RequestDataStore) any { // Router defines a route based on chi's router type Router struct { - chiRouter *chi.Mux + chiRouter *chi.Mux + + afterRouting []any + curGroupPrefix string curMiddlewares []any } @@ -52,8 +62,9 @@ func NewRouter() *Router { return &Router{chiRouter: r} } -// Use supports two middlewares -func (r *Router) Use(middlewares ...any) { +// BeforeRouting adds middlewares which will be executed before the request path gets routed +// It should only be used for framework-level global middlewares when it needs to change request method & path. +func (r *Router) BeforeRouting(middlewares ...any) { for _, m := range middlewares { if !isNilOrFuncNil(m) { r.chiRouter.Use(toHandlerProvider(m)) @@ -61,7 +72,13 @@ func (r *Router) Use(middlewares ...any) { } } -// Group mounts a sub-Router along a `pattern` string. +// AfterRouting adds middlewares which will be executed after the request path gets routed +// It can see the routed path and resolved path parameters +func (r *Router) AfterRouting(middlewares ...any) { + r.afterRouting = append(r.afterRouting, middlewares...) +} + +// Group mounts a sub-router along a "pattern" string. func (r *Router) Group(pattern string, fn func(), middlewares ...any) { previousGroupPrefix := r.curGroupPrefix previousMiddlewares := r.curMiddlewares @@ -93,36 +110,54 @@ func isNilOrFuncNil(v any) bool { return r.Kind() == reflect.Func && r.IsNil() } -func wrapMiddlewareAndHandler(curMiddlewares, h []any) ([]func(http.Handler) http.Handler, http.HandlerFunc) { - handlerProviders := make([]func(http.Handler) http.Handler, 0, len(curMiddlewares)+len(h)+1) - for _, m := range curMiddlewares { - if !isNilOrFuncNil(m) { - handlerProviders = append(handlerProviders, toHandlerProvider(m)) +func wrapMiddlewareAppendPre(all []middlewareProvider, middlewares []any) []middlewareProvider { + for _, m := range middlewares { + if h, ok := m.(PreMiddlewareProvider); ok && h != nil { + all = append(all, toHandlerProvider(middlewareProvider(h))) } } + return all +} + +func wrapMiddlewareAppendNormal(all []middlewareProvider, middlewares []any) []middlewareProvider { + for _, m := range middlewares { + if _, ok := m.(PreMiddlewareProvider); !ok && !isNilOrFuncNil(m) { + all = append(all, toHandlerProvider(m)) + } + } + return all +} + +func wrapMiddlewareAndHandler(useMiddlewares, curMiddlewares, h []any) (_ []middlewareProvider, _ http.HandlerFunc, hasPreMiddlewares bool) { if len(h) == 0 { panic("no endpoint handler provided") } - for i, m := range h { - if !isNilOrFuncNil(m) { - handlerProviders = append(handlerProviders, toHandlerProvider(m)) - } else if i == len(h)-1 { - panic("endpoint handler can't be nil") - } + if isNilOrFuncNil(h[len(h)-1]) { + panic("endpoint handler can't be nil") } + + handlerProviders := make([]middlewareProvider, 0, len(useMiddlewares)+len(curMiddlewares)+len(h)+1) + handlerProviders = wrapMiddlewareAppendPre(handlerProviders, useMiddlewares) + handlerProviders = wrapMiddlewareAppendPre(handlerProviders, curMiddlewares) + handlerProviders = wrapMiddlewareAppendPre(handlerProviders, h) + hasPreMiddlewares = len(handlerProviders) > 0 + handlerProviders = wrapMiddlewareAppendNormal(handlerProviders, useMiddlewares) + handlerProviders = wrapMiddlewareAppendNormal(handlerProviders, curMiddlewares) + handlerProviders = wrapMiddlewareAppendNormal(handlerProviders, h) + middlewares := handlerProviders[:len(handlerProviders)-1] handlerFunc := handlerProviders[len(handlerProviders)-1](nil).ServeHTTP mockPoint := RouterMockPoint(MockAfterMiddlewares) if mockPoint != nil { middlewares = append(middlewares, mockPoint) } - return middlewares, handlerFunc + return middlewares, handlerFunc, hasPreMiddlewares } // Methods adds the same handlers for multiple http "methods" (separated by ","). // If any method is invalid, the lower level router will panic. func (r *Router) Methods(methods, pattern string, h ...any) { - middlewares, handlerFunc := wrapMiddlewareAndHandler(r.curMiddlewares, h) + middlewares, handlerFunc, _ := wrapMiddlewareAndHandler(r.afterRouting, r.curMiddlewares, h) fullPattern := r.getPattern(pattern) if strings.Contains(methods, ",") { methods := strings.SplitSeq(methods, ",") @@ -134,15 +169,19 @@ func (r *Router) Methods(methods, pattern string, h ...any) { } } -// Mount attaches another Router along ./pattern/* +// Mount attaches another Router along "/pattern/*" func (r *Router) Mount(pattern string, subRouter *Router) { - subRouter.Use(r.curMiddlewares...) - r.chiRouter.Mount(r.getPattern(pattern), subRouter.chiRouter) + handlerProviders := make([]middlewareProvider, 0, len(r.afterRouting)+len(r.curMiddlewares)) + handlerProviders = wrapMiddlewareAppendPre(handlerProviders, r.afterRouting) + handlerProviders = wrapMiddlewareAppendPre(handlerProviders, r.curMiddlewares) + handlerProviders = wrapMiddlewareAppendNormal(handlerProviders, r.afterRouting) + handlerProviders = wrapMiddlewareAppendNormal(handlerProviders, r.curMiddlewares) + r.chiRouter.With(handlerProviders...).Mount(r.getPattern(pattern), subRouter.chiRouter) } // Any delegate requests for all methods func (r *Router) Any(pattern string, h ...any) { - middlewares, handlerFunc := wrapMiddlewareAndHandler(r.curMiddlewares, h) + middlewares, handlerFunc, _ := wrapMiddlewareAndHandler(r.afterRouting, r.curMiddlewares, h) r.chiRouter.With(middlewares...).HandleFunc(r.getPattern(pattern), handlerFunc) } @@ -178,12 +217,16 @@ func (r *Router) Patch(pattern string, h ...any) { // ServeHTTP implements http.Handler func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) { + // TODO: need to move it to the top-level common middleware, otherwise each "Mount" will cause it to be executed multiple times, which is inefficient. r.normalizeRequestPath(w, req, r.chiRouter) } // NotFound defines a handler to respond whenever a route could not be found. func (r *Router) NotFound(h http.HandlerFunc) { - r.chiRouter.NotFound(h) + middlewares, handlerFunc, _ := wrapMiddlewareAndHandler(r.afterRouting, r.curMiddlewares, []any{h}) + r.chiRouter.NotFound(func(w http.ResponseWriter, r *http.Request) { + executeMiddlewaresHandler(w, r, middlewares, handlerFunc) + }) } func (r *Router) normalizeRequestPath(resp http.ResponseWriter, req *http.Request, next http.Handler) { diff --git a/modules/web/router_path.go b/modules/web/router_path.go index 64154c34a50..9e531346d13 100644 --- a/modules/web/router_path.go +++ b/modules/web/router_path.go @@ -27,11 +27,7 @@ func (g *RouterPathGroup) ServeHTTP(resp http.ResponseWriter, req *http.Request) for _, m := range g.matchers { if m.matchPath(chiCtx, path) { chiCtx.RoutePatterns = append(chiCtx.RoutePatterns, m.pattern) - handler := m.handlerFunc - for i := len(m.middlewares) - 1; i >= 0; i-- { - handler = m.middlewares[i](handler).ServeHTTP - } - handler(resp, req) + executeMiddlewaresHandler(resp, req, m.middlewares, m.handlerFunc) return } } @@ -67,7 +63,7 @@ type routerPathMatcher struct { pattern string re *regexp.Regexp params []routerPathParam - middlewares []func(http.Handler) http.Handler + middlewares []middlewareProvider handlerFunc http.HandlerFunc } @@ -111,7 +107,10 @@ func isValidMethod(name string) bool { } func newRouterPathMatcher(methods string, patternRegexp *RouterPathGroupPattern, h ...any) *routerPathMatcher { - middlewares, handlerFunc := wrapMiddlewareAndHandler(patternRegexp.middlewares, h) + middlewares, handlerFunc, hasPreMiddlewares := wrapMiddlewareAndHandler(nil, patternRegexp.middlewares, h) + if hasPreMiddlewares { + panic("pre-middlewares are not supported in router path matcher") + } p := &routerPathMatcher{methods: make(container.Set[string]), middlewares: middlewares, handlerFunc: handlerFunc} for method := range strings.SplitSeq(methods, ",") { method = strings.TrimSpace(method) diff --git a/modules/web/router_test.go b/modules/web/router_test.go index ab5fbb502c7..645e70b869d 100644 --- a/modules/web/router_test.go +++ b/modules/web/router_test.go @@ -30,6 +30,71 @@ func chiURLParamsToMap(chiCtx *chi.Context) map[string]string { return util.Iif(len(m) == 0, nil, m) } +type testResult struct { + method string + pathParams map[string]string + handlerMarks []string + chiRoutePattern *string +} + +type testRecorder struct { + res testResult +} + +func (r *testRecorder) reset() { + r.res = testResult{} +} + +func (r *testRecorder) handle(optMark ...string) func(resp http.ResponseWriter, req *http.Request) { + mark := util.OptionalArg(optMark, "") + return func(resp http.ResponseWriter, req *http.Request) { + chiCtx := chi.RouteContext(req.Context()) + r.res.method = req.Method + r.res.pathParams = chiURLParamsToMap(chiCtx) + r.res.chiRoutePattern = new(chiCtx.RoutePattern()) + if mark != "" { + r.res.handlerMarks = append(r.res.handlerMarks, mark) + } + } +} + +func (r *testRecorder) provider(optMark ...string) func(next http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { + r.handle(optMark...)(resp, req) + next.ServeHTTP(resp, req) + }) + } +} + +func (r *testRecorder) stop(optMark ...string) func(resp http.ResponseWriter, req *http.Request) { + mark := util.OptionalArg(optMark, "") + return func(resp http.ResponseWriter, req *http.Request) { + if stop := req.FormValue("stop"); stop != "" && (mark == "" || mark == stop) { + r.handle(stop)(resp, req) + resp.WriteHeader(http.StatusOK) + } else if mark != "" { + r.res.handlerMarks = append(r.res.handlerMarks, mark) + } + } +} + +func (r *testRecorder) test(t *testing.T, rt *Router, methodPath string, expected testResult) { + r.reset() + methodPathFields := strings.Fields(methodPath) + req, err := http.NewRequest(methodPathFields[0], methodPathFields[1], nil) + assert.NoError(t, err) + + buff := &bytes.Buffer{} + httpRecorder := httptest.NewRecorder() + httpRecorder.Body = buff + rt.ServeHTTP(httpRecorder, req) + if expected.chiRoutePattern == nil { + r.res.chiRoutePattern = nil + } + assert.Equal(t, expected, r.res) +} + func TestPathProcessor(t *testing.T) { testProcess := func(pattern, uri string, expectedPathParams map[string]string) { chiCtx := chi.NewRouteContext() @@ -51,42 +116,10 @@ func TestPathProcessor(t *testing.T) { } func TestRouter(t *testing.T) { - buff := &bytes.Buffer{} - recorder := httptest.NewRecorder() - recorder.Body = buff - - type resultStruct struct { - method string - pathParams map[string]string - handlerMarks []string - chiRoutePattern *string - } - - 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) { - chiCtx := chi.RouteContext(req.Context()) - res.method = req.Method - res.pathParams = chiURLParamsToMap(chiCtx) - res.chiRoutePattern = new(chiCtx.RoutePattern()) - if mark != "" { - res.handlerMarks = append(res.handlerMarks, mark) - } - } - } - - stopMark := func(optMark ...string) func(resp http.ResponseWriter, req *http.Request) { - mark := util.OptionalArg(optMark, "") - return func(resp http.ResponseWriter, req *http.Request) { - 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) - } - } - } + type resultStruct = testResult + resRecorder := &testRecorder{} + h := resRecorder.handle + stopMark := resRecorder.stop r := NewRouter() r.NotFound(h("not-found:/")) @@ -123,15 +156,7 @@ func TestRouter(t *testing.T) { testRoute := func(t *testing.T, methodPath string, expected resultStruct) { t.Run(methodPath, func(t *testing.T) { - res = resultStruct{} - methodPathFields := strings.Fields(methodPath) - req, err := http.NewRequest(methodPathFields[0], methodPathFields[1], nil) - assert.NoError(t, err) - r.ServeHTTP(recorder, req) - if expected.chiRoutePattern == nil { - res.chiRoutePattern = nil - } - assert.Equal(t, expected, res) + resRecorder.test(t, r, methodPath, expected) }) } @@ -273,3 +298,39 @@ func TestRouteNormalizePath(t *testing.T) { testPath("/v2/", paths{EscapedPath: "/v2", RawPath: "/v2", Path: "/v2"}) testPath("/v2/%2f", paths{EscapedPath: "/v2/%2f", RawPath: "/v2/%2f", Path: "/v2//"}) } + +func TestPreMiddlewareProvider(t *testing.T) { + resRecorder := &testRecorder{} + h := resRecorder.handle + p := resRecorder.provider + + root := NewRouter() + root.BeforeRouting(h("before-root")) + root.AfterRouting(h("root")) + root.Get("/a/1", h("mid"), PreMiddlewareProvider(p("pre-root")), h("end1")) + + sub := NewRouter() + sub.BeforeRouting(h("before-sub")) + sub.AfterRouting(h("sub")) + sub.Get("/2", h("mid"), PreMiddlewareProvider(p("pre-sub")), h("end2")) + sub.NotFound(h("not-found")) + + root.Mount("/a", sub) + + resRecorder.test(t, root, "GET /a/1", testResult{ + method: "GET", + handlerMarks: []string{"before-root", "pre-root", "root", "mid", "end1"}, + }) + resRecorder.test(t, root, "GET /a/2", testResult{ + method: "GET", + handlerMarks: []string{"before-root", "root", "before-sub", "pre-sub", "sub", "mid", "end2"}, + }) + resRecorder.test(t, root, "GET /no-such", testResult{ + method: "GET", + handlerMarks: []string{"before-root"}, + }) + resRecorder.test(t, root, "GET /a/no-such", testResult{ + method: "GET", + handlerMarks: []string{"before-root", "root", "before-sub", "sub", "not-found"}, + }) +} diff --git a/modules/web/routing/context.go b/modules/web/routing/context.go index d3eb98f83db..838abea1587 100644 --- a/modules/web/routing/context.go +++ b/modules/web/routing/context.go @@ -44,7 +44,7 @@ func MarkLongPolling(resp http.ResponseWriter, req *http.Request) { } // UpdatePanicError updates a context's error info, a panic may be recovered by other middlewares, but we still need to know that. -func UpdatePanicError(ctx context.Context, err any) { +func UpdatePanicError(ctx context.Context, err error) { record, ok := ctx.Value(contextKey).(*requestRecord) if !ok { return diff --git a/modules/web/routing/logger_manager.go b/modules/web/routing/logger_manager.go index aa25ec3a271..2f767c3d669 100644 --- a/modules/web/routing/logger_manager.go +++ b/modules/web/routing/logger_manager.go @@ -5,11 +5,13 @@ package routing import ( "context" + "fmt" "net/http" "sync" "time" "code.gitea.io/gitea/modules/graceful" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/process" ) @@ -99,7 +101,7 @@ func (manager *requestRecordsManager) handler(next http.Handler) http.Handler { localPanicErr := recover() if localPanicErr != nil { record.lock.Lock() - record.panicError = localPanicErr + record.panicError = fmt.Errorf("%v\n%s", localPanicErr, log.Stack(2)) record.lock.Unlock() } diff --git a/modules/web/routing/requestrecord.go b/modules/web/routing/requestrecord.go index cc61fc4d348..888c3e5c2f4 100644 --- a/modules/web/routing/requestrecord.go +++ b/modules/web/routing/requestrecord.go @@ -24,5 +24,5 @@ type requestRecord struct { // mutable fields isLongPolling bool funcInfo *FuncInfo - panicError any + panicError error } diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index 6bef6c03c06..d7e7203a857 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -103,7 +103,7 @@ func init() { func ArtifactsRoutes(prefix string) *web.Router { m := web.NewRouter() - m.Use(ArtifactContexter()) + m.AfterRouting(ArtifactContexter()) r := artifactRoutes{ prefix: prefix, diff --git a/routers/api/packages/api.go b/routers/api/packages/api.go index 71fee23c920..7f81242db4a 100644 --- a/routers/api/packages/api.go +++ b/routers/api/packages/api.go @@ -94,7 +94,7 @@ func verifyAuth(r *web.Router, authMethods []auth.Method) { } authGroup := auth.NewGroup(authMethods...) - r.Use(func(ctx *context.Context) { + r.AfterRouting(func(ctx *context.Context) { var err error ctx.Doer, err = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session) if err != nil { @@ -111,7 +111,7 @@ func verifyAuth(r *web.Router, authMethods []auth.Method) { func CommonRoutes() *web.Router { r := web.NewRouter() - r.Use(context.PackageContexter()) + r.AfterRouting(context.PackageContexter()) verifyAuth(r, []auth.Method{ &auth.OAuth2{}, @@ -533,7 +533,7 @@ func CommonRoutes() *web.Router { func ContainerRoutes() *web.Router { r := web.NewRouter() - r.Use(context.PackageContexter()) + r.AfterRouting(context.PackageContexter()) verifyAuth(r, []auth.Method{ &auth.Basic{}, diff --git a/routers/api/v1/admin/adopt.go b/routers/api/v1/admin/adopt.go index 92711409f00..9f1175a1ffe 100644 --- a/routers/api/v1/admin/adopt.go +++ b/routers/api/v1/admin/adopt.go @@ -50,7 +50,7 @@ func ListUnadoptedRepositories(ctx *context.APIContext) { return } - ctx.SetTotalCountHeader(int64(count)) + ctx.SetTotalCountHeader(count) ctx.JSON(http.StatusOK, repoNames) } diff --git a/routers/api/v1/admin/email.go b/routers/api/v1/admin/email.go index ad078347a40..1212babac45 100644 --- a/routers/api/v1/admin/email.go +++ b/routers/api/v1/admin/email.go @@ -51,7 +51,7 @@ func GetAllEmails(ctx *context.APIContext) { results[i] = convert.ToEmailSearch(emails[i]) } - ctx.SetLinkHeader(int(maxResults), listOptions.PageSize) + ctx.SetLinkHeader(maxResults, listOptions.PageSize) ctx.SetTotalCountHeader(maxResults) ctx.JSON(http.StatusOK, &results) } diff --git a/routers/api/v1/admin/hooks.go b/routers/api/v1/admin/hooks.go index 6170e7343af..80e9e964c25 100644 --- a/routers/api/v1/admin/hooks.go +++ b/routers/api/v1/admin/hooks.go @@ -77,7 +77,7 @@ func ListHooks(ctx *context.APIContext) { } hooks[i] = h } - ctx.SetLinkHeader(int(total), listOptions.PageSize) + ctx.SetLinkHeader(total, listOptions.PageSize) ctx.SetTotalCountHeader(total) ctx.JSON(http.StatusOK, hooks) } diff --git a/routers/api/v1/admin/org.go b/routers/api/v1/admin/org.go index 62afcb00d9a..6390bb7e829 100644 --- a/routers/api/v1/admin/org.go +++ b/routers/api/v1/admin/org.go @@ -117,7 +117,7 @@ func GetAllOrgs(ctx *context.APIContext) { orgs[i] = convert.ToOrganization(ctx, organization.OrgFromUser(users[i])) } - ctx.SetLinkHeader(int(maxResults), listOptions.PageSize) + ctx.SetLinkHeader(maxResults, listOptions.PageSize) ctx.SetTotalCountHeader(maxResults) ctx.JSON(http.StatusOK, &orgs) } diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index 6bed4106427..b9dd12f8ff6 100644 --- a/routers/api/v1/admin/user.go +++ b/routers/api/v1/admin/user.go @@ -534,7 +534,7 @@ func SearchUsers(ctx *context.APIContext) { results[i] = convert.ToUser(ctx, users[i], ctx.Doer) } - ctx.SetLinkHeader(int(maxResults), listOptions.PageSize) + ctx.SetLinkHeader(maxResults, listOptions.PageSize) ctx.SetTotalCountHeader(maxResults) ctx.JSON(http.StatusOK, &results) } diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 5a6da089dee..907f1dba273 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -77,7 +77,6 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" @@ -756,13 +755,9 @@ func buildAuthGroup() *auth.Group { &auth.Basic{}, // FIXME: this should be removed once we don't allow basic auth in API ) if setting.Service.EnableReverseProxyAuthAPI { - group.Add(&auth.ReverseProxy{}) + group.Add(&auth.ReverseProxy{}) // TODO: does it still make sense to support reverse proxy auth in API? } - - if setting.IsWindows && auth_model.IsSSPIEnabled(graceful.GetManager().ShutdownContext()) { - group.Add(&auth.SSPI{}) // it MUST be the last, see the comment of SSPI - } - + // others: API doesn't support SSPI auth because the caller should use token return group } @@ -872,9 +867,9 @@ func checkDeprecatedAuthMethods(ctx *context.APIContext) { func Routes() *web.Router { m := web.NewRouter() - m.Use(securityHeaders()) + m.BeforeRouting(securityHeaders()) if setting.CORSConfig.Enabled { - m.Use(cors.Handler(cors.Options{ + m.BeforeRouting(cors.Handler(cors.Options{ AllowedOrigins: setting.CORSConfig.AllowDomain, AllowedMethods: setting.CORSConfig.Methods, AllowCredentials: setting.CORSConfig.AllowCredentials, @@ -882,14 +877,14 @@ func Routes() *web.Router { MaxAge: int(setting.CORSConfig.MaxAge.Seconds()), })) } - m.Use(context.APIContexter()) - m.Use(checkDeprecatedAuthMethods) + m.AfterRouting(context.APIContexter()) + m.AfterRouting(checkDeprecatedAuthMethods) // Get user from session if logged in. - m.Use(apiAuth(buildAuthGroup())) + m.AfterRouting(apiAuth(buildAuthGroup())) - m.Use(verifyAuthWithOptions(&common.VerifyOptions{ + m.AfterRouting(verifyAuthWithOptions(&common.VerifyOptions{ SignInRequired: setting.Service.RequireSignInViewStrict, })) diff --git a/routers/api/v1/notify/repo.go b/routers/api/v1/notify/repo.go index 51695a52c8d..5e23e2285da 100644 --- a/routers/api/v1/notify/repo.go +++ b/routers/api/v1/notify/repo.go @@ -125,7 +125,7 @@ func ListRepoNotifications(ctx *context.APIContext) { return } - ctx.SetLinkHeader(int(totalCount), opts.PageSize) + ctx.SetLinkHeader(totalCount, opts.PageSize) ctx.SetTotalCountHeader(totalCount) ctx.JSON(http.StatusOK, convert.ToNotifications(ctx, nl)) } diff --git a/routers/api/v1/notify/user.go b/routers/api/v1/notify/user.go index 82cedd418b8..629a5ec2288 100644 --- a/routers/api/v1/notify/user.go +++ b/routers/api/v1/notify/user.go @@ -86,7 +86,7 @@ func ListNotifications(ctx *context.APIContext) { return } - ctx.SetLinkHeader(int(totalCount), opts.PageSize) + ctx.SetLinkHeader(totalCount, opts.PageSize) ctx.SetTotalCountHeader(totalCount) ctx.JSON(http.StatusOK, convert.ToNotifications(ctx, nl)) } diff --git a/routers/api/v1/org/action.go b/routers/api/v1/org/action.go index d058964f52d..687e9fcbfb6 100644 --- a/routers/api/v1/org/action.go +++ b/routers/api/v1/org/action.go @@ -67,7 +67,7 @@ func (Action) ListActionsSecrets(ctx *context.APIContext) { } } - ctx.SetLinkHeader(int(count), opts.PageSize) + ctx.SetLinkHeader(count, opts.PageSize) ctx.SetTotalCountHeader(count) ctx.JSON(http.StatusOK, apiSecrets) } @@ -240,7 +240,7 @@ func (Action) ListVariables(ctx *context.APIContext) { Description: v.Description, } } - ctx.SetLinkHeader(int(count), listOptions.PageSize) + ctx.SetLinkHeader(count, listOptions.PageSize) ctx.SetTotalCountHeader(count) ctx.JSON(http.StatusOK, variables) } diff --git a/routers/api/v1/org/member.go b/routers/api/v1/org/member.go index b72cafee0c7..e7311fc6fd3 100644 --- a/routers/api/v1/org/member.go +++ b/routers/api/v1/org/member.go @@ -45,7 +45,7 @@ func listMembers(ctx *context.APIContext, isMember bool) { apiMembers[i] = convert.ToUser(ctx, member, ctx.Doer) } - ctx.SetLinkHeader(int(count), listOptions.PageSize) + ctx.SetLinkHeader(count, listOptions.PageSize) ctx.SetTotalCountHeader(count) ctx.JSON(http.StatusOK, apiMembers) } diff --git a/routers/api/v1/org/org.go b/routers/api/v1/org/org.go index f229a84f66a..0f4da7966ca 100644 --- a/routers/api/v1/org/org.go +++ b/routers/api/v1/org/org.go @@ -48,7 +48,7 @@ func listUserOrgs(ctx *context.APIContext, u *user_model.User) { apiOrgs[i] = convert.ToOrganization(ctx, orgs[i]) } - ctx.SetLinkHeader(int(maxResults), listOptions.PageSize) + ctx.SetLinkHeader(maxResults, listOptions.PageSize) ctx.SetTotalCountHeader(maxResults) ctx.JSON(http.StatusOK, &apiOrgs) } @@ -221,7 +221,7 @@ func GetAll(ctx *context.APIContext) { orgs[i] = convert.ToOrganization(ctx, organization.OrgFromUser(publicOrgs[i])) } - ctx.SetLinkHeader(int(maxResults), listOptions.PageSize) + ctx.SetLinkHeader(maxResults, listOptions.PageSize) ctx.SetTotalCountHeader(maxResults) ctx.JSON(http.StatusOK, &orgs) } diff --git a/routers/api/v1/org/team.go b/routers/api/v1/org/team.go index 211b7a15b25..3b5711eea32 100644 --- a/routers/api/v1/org/team.go +++ b/routers/api/v1/org/team.go @@ -70,7 +70,7 @@ func ListTeams(ctx *context.APIContext) { return } - ctx.SetLinkHeader(int(count), listOptions.PageSize) + ctx.SetLinkHeader(count, listOptions.PageSize) ctx.SetTotalCountHeader(count) ctx.JSON(http.StatusOK, apiTeams) } @@ -111,7 +111,7 @@ func ListUserTeams(ctx *context.APIContext) { return } - ctx.SetLinkHeader(int(count), listOptions.PageSize) + ctx.SetLinkHeader(count, listOptions.PageSize) ctx.SetTotalCountHeader(count) ctx.JSON(http.StatusOK, apiTeams) } @@ -411,7 +411,7 @@ func GetTeamMembers(ctx *context.APIContext) { members[i] = convert.ToUser(ctx, member, ctx.Doer) } - ctx.SetLinkHeader(ctx.Org.Team.NumMembers, listOptions.PageSize) + ctx.SetLinkHeader(int64(ctx.Org.Team.NumMembers), listOptions.PageSize) ctx.SetTotalCountHeader(int64(ctx.Org.Team.NumMembers)) ctx.JSON(http.StatusOK, members) } @@ -583,7 +583,7 @@ func GetTeamRepos(ctx *context.APIContext) { } repos[i] = convert.ToRepo(ctx, repo, permission) } - ctx.SetLinkHeader(team.NumRepos, listOptions.PageSize) + ctx.SetLinkHeader(int64(team.NumRepos), listOptions.PageSize) ctx.SetTotalCountHeader(int64(team.NumRepos)) ctx.JSON(http.StatusOK, repos) } @@ -827,7 +827,7 @@ func SearchTeam(ctx *context.APIContext) { return } - ctx.SetLinkHeader(int(maxResults), listOptions.PageSize) + ctx.SetLinkHeader(maxResults, listOptions.PageSize) ctx.SetTotalCountHeader(maxResults) ctx.JSON(http.StatusOK, map[string]any{ "ok": true, @@ -882,7 +882,7 @@ func ListTeamActivityFeeds(ctx *context.APIContext) { ctx.APIErrorInternal(err) return } - ctx.SetLinkHeader(int(count), listOptions.PageSize) + ctx.SetLinkHeader(count, listOptions.PageSize) ctx.SetTotalCountHeader(count) ctx.JSON(http.StatusOK, convert.ToActivities(ctx, feeds, ctx.Doer)) } diff --git a/routers/api/v1/packages/package.go b/routers/api/v1/packages/package.go index 41b7f2a43f6..cee0daccaec 100644 --- a/routers/api/v1/packages/package.go +++ b/routers/api/v1/packages/package.go @@ -68,7 +68,7 @@ func ListPackages(ctx *context.APIContext) { return } - ctx.SetLinkHeader(int(count), listOptions.PageSize) + ctx.SetLinkHeader(count, listOptions.PageSize) ctx.SetTotalCountHeader(count) ctx.JSON(http.StatusOK, apiPackages) } @@ -249,7 +249,7 @@ func ListPackageVersions(ctx *context.APIContext) { return } - ctx.SetLinkHeader(int(count), listOptions.PageSize) + ctx.SetLinkHeader(count, listOptions.PageSize) ctx.SetTotalCountHeader(count) ctx.JSON(http.StatusOK, apiPackages) } diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index 13da5aa8151..6f4d5d35727 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -91,7 +91,7 @@ func (Action) ListActionsSecrets(ctx *context.APIContext) { Created: v.CreatedUnix.AsTime(), } } - ctx.SetLinkHeader(int(count), listOptions.PageSize) + ctx.SetLinkHeader(count, listOptions.PageSize) ctx.SetTotalCountHeader(count) ctx.JSON(http.StatusOK, apiSecrets) } @@ -506,7 +506,7 @@ func (Action) ListVariables(ctx *context.APIContext) { } } - ctx.SetLinkHeader(int(count), listOptions.PageSize) + ctx.SetLinkHeader(count, listOptions.PageSize) ctx.SetTotalCountHeader(count) ctx.JSON(http.StatusOK, variables) } @@ -811,7 +811,7 @@ func ListActionTasks(ctx *context.APIContext) { res.Entries[i] = convertedTask } - ctx.SetLinkHeader(int(total), listOptions.PageSize) + ctx.SetLinkHeader(total, listOptions.PageSize) ctx.SetTotalCountHeader(total) // Duplicates api response field but it's better to set it for consistency ctx.JSON(http.StatusOK, &res) } diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index eaa8afb1e13..c3b3fc10855 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -375,7 +375,7 @@ func ListBranches(ctx *context.APIContext) { } } - ctx.SetLinkHeader(int(totalNumOfBranches), listOptions.PageSize) + ctx.SetLinkHeader(totalNumOfBranches, listOptions.PageSize) ctx.SetTotalCountHeader(totalNumOfBranches) ctx.JSON(http.StatusOK, apiBranches) } diff --git a/routers/api/v1/repo/commits.go b/routers/api/v1/repo/commits.go index 2a7efa0ea6f..008e5dc56da 100644 --- a/routers/api/v1/repo/commits.go +++ b/routers/api/v1/repo/commits.go @@ -290,7 +290,7 @@ func GetAllCommits(ctx *context.APIContext) { } } - ctx.SetLinkHeader(int(commitsCountTotal), listOptions.PageSize) + ctx.SetLinkHeader(commitsCountTotal, listOptions.PageSize) ctx.SetTotalCountHeader(commitsCountTotal) // kept for backwards compatibility diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 22324e19233..db205380e45 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -299,7 +299,7 @@ func SearchIssues(ctx *context.APIContext) { return } - ctx.SetLinkHeader(int(total), limit) + ctx.SetLinkHeader(total, limit) ctx.SetTotalCountHeader(total) ctx.JSON(http.StatusOK, convert.ToAPIIssueList(ctx, ctx.Doer, issues)) } @@ -527,7 +527,7 @@ func ListIssues(ctx *context.APIContext) { return } - ctx.SetLinkHeader(int(total), listOptions.PageSize) + ctx.SetLinkHeader(total, listOptions.PageSize) ctx.SetTotalCountHeader(total) ctx.JSON(http.StatusOK, convert.ToAPIIssueList(ctx, ctx.Doer, issues)) } diff --git a/routers/api/v1/repo/issue_dependency.go b/routers/api/v1/repo/issue_dependency.go index 6c66e719eb8..139779dcec9 100644 --- a/routers/api/v1/repo/issue_dependency.go +++ b/routers/api/v1/repo/issue_dependency.go @@ -81,7 +81,7 @@ func GetIssueDependencies(ctx *context.APIContext) { canWrite := ctx.Repo.Permission.CanWriteIssuesOrPulls(issue.IsPull) - blockerIssues := make([]*issues_model.Issue, 0, listOptions.PageSize) + blockerIssues := make([]*issues_model.Issue, 0, min(listOptions.PageSize, setting.API.MaxResponseItems)) // 2. Get the issues this issue depends on, i.e. the `<#b>`: `