From ca31d478ee84df98d64521ba42e47ca2d61c5f77 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 25 Dec 2024 12:03:14 +0800 Subject: [PATCH] Refactor arch route handlers (#32972) --- modules/web/route.go | 92 ++++++++++++++ modules/web/route_test.go | 245 +++++++++++++++++------------------- routers/api/packages/api.go | 28 ++--- 3 files changed, 218 insertions(+), 147 deletions(-) diff --git a/modules/web/route.go b/modules/web/route.go index 533ac3eaf1..729ac46cdc 100644 --- a/modules/web/route.go +++ b/modules/web/route.go @@ -4,14 +4,18 @@ package web import ( + "fmt" "net/http" "net/url" "reflect" + "regexp" "strings" + "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/htmlutil" "code.gitea.io/gitea/modules/reqctx" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web/middleware" "gitea.com/go-chi/binding" @@ -181,6 +185,17 @@ func (r *Router) NotFound(h http.HandlerFunc) { r.chiRouter.NotFound(h) } +type pathProcessorParam struct { + name string + captureGroup int +} + +type PathProcessor struct { + methods container.Set[string] + re *regexp.Regexp + params []pathProcessorParam +} + func (r *Router) normalizeRequestPath(resp http.ResponseWriter, req *http.Request, next http.Handler) { normalized := false normalizedPath := req.URL.EscapedPath() @@ -238,6 +253,83 @@ func (r *Router) normalizeRequestPath(resp http.ResponseWriter, req *http.Reques next.ServeHTTP(resp, req) } +func (p *PathProcessor) ProcessRequestPath(chiCtx *chi.Context, path string) bool { + if !p.methods.Contains(chiCtx.RouteMethod) { + return false + } + if !strings.HasPrefix(path, "/") { + path = "/" + path + } + pathMatches := p.re.FindStringSubmatchIndex(path) // Golang regexp match pairs [start, end, start, end, ...] + if pathMatches == nil { + return false + } + var paramMatches [][]int + for i := 2; i < len(pathMatches); { + paramMatches = append(paramMatches, []int{pathMatches[i], pathMatches[i+1]}) + pmIdx := len(paramMatches) - 1 + end := pathMatches[i+1] + i += 2 + for ; i < len(pathMatches); i += 2 { + if pathMatches[i] >= end { + break + } + paramMatches[pmIdx] = append(paramMatches[pmIdx], pathMatches[i], pathMatches[i+1]) + } + } + for i, pm := range paramMatches { + groupIdx := p.params[i].captureGroup * 2 + chiCtx.URLParams.Add(p.params[i].name, path[pm[groupIdx]:pm[groupIdx+1]]) + } + return true +} + +func NewPathProcessor(methods, pattern string) *PathProcessor { + p := &PathProcessor{methods: make(container.Set[string])} + for _, method := range strings.Split(methods, ",") { + p.methods.Add(strings.TrimSpace(method)) + } + re := []byte{'^'} + lastEnd := 0 + for lastEnd < len(pattern) { + start := strings.IndexByte(pattern[lastEnd:], '<') + if start == -1 { + re = append(re, pattern[lastEnd:]...) + break + } + end := strings.IndexByte(pattern[lastEnd+start:], '>') + if end == -1 { + panic(fmt.Sprintf("invalid pattern: %s", pattern)) + } + re = append(re, pattern[lastEnd:lastEnd+start]...) + partName, partExp, _ := strings.Cut(pattern[lastEnd+start+1:lastEnd+start+end], ":") + lastEnd += start + end + 1 + + // TODO: it could support to specify a "capture group" for the name, for example: "/" + // it is not used so no need to implement it now + param := pathProcessorParam{} + if partExp == "*" { + re = append(re, "(.*?)/?"...) + if lastEnd < len(pattern) { + if pattern[lastEnd] == '/' { + lastEnd++ + } + } + } else { + partExp = util.IfZero(partExp, "[^/]+") + re = append(re, '(') + re = append(re, partExp...) + re = append(re, ')') + } + param.name = partName + p.params = append(p.params, param) + } + re = append(re, '$') + reStr := string(re) + p.re = regexp.MustCompile(reStr) + return p +} + // Combo delegates requests to Combo func (r *Router) Combo(pattern string, h ...any) *Combo { return &Combo{r, pattern, h} diff --git a/modules/web/route_test.go b/modules/web/route_test.go index 6e4c309293..ca3134b546 100644 --- a/modules/web/route_test.go +++ b/modules/web/route_test.go @@ -7,177 +7,160 @@ import ( "bytes" "net/http" "net/http/httptest" - "strconv" + "strings" "testing" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/test" + "code.gitea.io/gitea/modules/util" - chi "github.com/go-chi/chi/v5" + "github.com/go-chi/chi/v5" "github.com/stretchr/testify/assert" ) -func TestRoute1(t *testing.T) { - buff := bytes.NewBufferString("") - recorder := httptest.NewRecorder() - recorder.Body = buff - - r := NewRouter() - r.Get("/{username}/{reponame}/{type:issues|pulls}", func(resp http.ResponseWriter, req *http.Request) { - username := chi.URLParam(req, "username") - assert.EqualValues(t, "gitea", username) - reponame := chi.URLParam(req, "reponame") - assert.EqualValues(t, "gitea", reponame) - tp := chi.URLParam(req, "type") - assert.EqualValues(t, "issues", tp) - }) - - req, err := http.NewRequest("GET", "http://localhost:8000/gitea/gitea/issues", nil) - assert.NoError(t, err) - r.ServeHTTP(recorder, req) - assert.EqualValues(t, http.StatusOK, recorder.Code) +func chiURLParamsToMap(chiCtx *chi.Context) map[string]string { + pathParams := chiCtx.URLParams + m := make(map[string]string, len(pathParams.Keys)) + for i, key := range pathParams.Keys { + if key == "*" && pathParams.Values[i] == "" { + continue // chi router will add an empty "*" key if there is a "Mount" + } + m[key] = pathParams.Values[i] + } + return m } -func TestRoute2(t *testing.T) { +func TestPathProcessor(t *testing.T) { + testProcess := func(pattern, uri string, expectedPathParams map[string]string) { + chiCtx := chi.NewRouteContext() + chiCtx.RouteMethod = "GET" + p := NewPathProcessor("GET", pattern) + assert.True(t, p.ProcessRequestPath(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) + } + testProcess("//", "/a/b", map[string]string{"p1": "a", "p2": "b"}) + testProcess("/", "", map[string]string{"p1": ""}) // this is a special case, because chi router could use empty path + testProcess("/", "/", map[string]string{"p1": ""}) + testProcess("//", "/a", map[string]string{"p1": "", "p2": "a"}) + testProcess("//", "/a/b", map[string]string{"p1": "a", "p2": "b"}) + testProcess("//", "/a/b/c", map[string]string{"p1": "a/b", "p2": "c"}) +} + +func TestRouter(t *testing.T) { buff := bytes.NewBufferString("") recorder := httptest.NewRecorder() recorder.Body = buff - hit := -1 + type resultStruct struct { + method string + pathParams map[string]string + handlerMark 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) { + res.method = req.Method + res.pathParams = chiURLParamsToMap(chi.RouteContext(req.Context())) + res.handlerMark = mark + } + } r := NewRouter() + r.Get("/{username}/{reponame}/{type:issues|pulls}", h("list-issues-a")) // this one will never be called r.Group("/{username}/{reponame}", func() { + r.Get("/{type:issues|pulls}", h("list-issues-b")) r.Group("", func() { - r.Get("/{type:issues|pulls}", func(resp http.ResponseWriter, req *http.Request) { - username := chi.URLParam(req, "username") - assert.EqualValues(t, "gitea", username) - reponame := chi.URLParam(req, "reponame") - assert.EqualValues(t, "gitea", reponame) - tp := chi.URLParam(req, "type") - assert.EqualValues(t, "issues", tp) - hit = 0 - }) - - r.Get("/{type:issues|pulls}/{index}", func(resp http.ResponseWriter, req *http.Request) { - username := chi.URLParam(req, "username") - assert.EqualValues(t, "gitea", username) - reponame := chi.URLParam(req, "reponame") - assert.EqualValues(t, "gitea", reponame) - tp := chi.URLParam(req, "type") - assert.EqualValues(t, "issues", tp) - index := chi.URLParam(req, "index") - assert.EqualValues(t, "1", index) - hit = 1 - }) + r.Get("/{type:issues|pulls}/{index}", h("view-issue")) }, func(resp http.ResponseWriter, req *http.Request) { - if stop, err := strconv.Atoi(req.FormValue("stop")); err == nil { - hit = stop + if stop := req.FormValue("stop"); stop != "" { + h(stop)(resp, req) resp.WriteHeader(http.StatusOK) } }) - r.Group("/issues/{index}", func() { - r.Get("/view", func(resp http.ResponseWriter, req *http.Request) { - username := chi.URLParam(req, "username") - assert.EqualValues(t, "gitea", username) - reponame := chi.URLParam(req, "reponame") - assert.EqualValues(t, "gitea", reponame) - index := chi.URLParam(req, "index") - assert.EqualValues(t, "1", index) - hit = 2 - }) + r.Post("/update", h("update-issue")) }) }) - req, err := http.NewRequest("GET", "http://localhost:8000/gitea/gitea/issues", nil) - assert.NoError(t, err) - r.ServeHTTP(recorder, req) - assert.EqualValues(t, http.StatusOK, recorder.Code) - assert.EqualValues(t, 0, hit) - - req, err = http.NewRequest("GET", "http://localhost:8000/gitea/gitea/issues/1", nil) - assert.NoError(t, err) - r.ServeHTTP(recorder, req) - assert.EqualValues(t, http.StatusOK, recorder.Code) - assert.EqualValues(t, 1, hit) - - req, err = http.NewRequest("GET", "http://localhost:8000/gitea/gitea/issues/1?stop=100", nil) - assert.NoError(t, err) - r.ServeHTTP(recorder, req) - assert.EqualValues(t, http.StatusOK, recorder.Code) - assert.EqualValues(t, 100, hit) - - req, err = http.NewRequest("GET", "http://localhost:8000/gitea/gitea/issues/1/view", nil) - assert.NoError(t, err) - r.ServeHTTP(recorder, req) - assert.EqualValues(t, http.StatusOK, recorder.Code) - assert.EqualValues(t, 2, hit) -} - -func TestRoute3(t *testing.T) { - buff := bytes.NewBufferString("") - recorder := httptest.NewRecorder() - recorder.Body = buff - - hit := -1 - m := NewRouter() - r := NewRouter() r.Mount("/api/v1", m) - m.Group("/repos", func() { m.Group("/{username}/{reponame}", func() { - m.Group("/branch_protections", func() { - m.Get("", func(resp http.ResponseWriter, req *http.Request) { - hit = 0 - }) - m.Post("", func(resp http.ResponseWriter, req *http.Request) { - hit = 1 - }) + m.Group("/branches", func() { + m.Get("", h()) + m.Post("", h()) m.Group("/{name}", func() { - m.Get("", func(resp http.ResponseWriter, req *http.Request) { - hit = 2 - }) - m.Patch("", func(resp http.ResponseWriter, req *http.Request) { - hit = 3 - }) - m.Delete("", func(resp http.ResponseWriter, req *http.Request) { - hit = 4 - }) + m.Get("", h()) + m.Patch("", h()) + m.Delete("", h()) }) }) }) }) - req, err := http.NewRequest("GET", "http://localhost:8000/api/v1/repos/gitea/gitea/branch_protections", nil) - assert.NoError(t, err) - r.ServeHTTP(recorder, req) - assert.EqualValues(t, http.StatusOK, recorder.Code) - assert.EqualValues(t, 0, hit) + testRoute := func(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) + assert.EqualValues(t, expected, res) + }) + } - req, err = http.NewRequest("POST", "http://localhost:8000/api/v1/repos/gitea/gitea/branch_protections", nil) - assert.NoError(t, err) - r.ServeHTTP(recorder, req) - assert.EqualValues(t, http.StatusOK, recorder.Code, http.StatusOK) - assert.EqualValues(t, 1, hit) + t.Run("Root Router", func(t *testing.T) { + testRoute("GET /the-user/the-repo/other", resultStruct{}) + testRoute("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", + }) + testRoute("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", + }) + testRoute("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", + }) + testRoute("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", + }) + }) - req, err = http.NewRequest("GET", "http://localhost:8000/api/v1/repos/gitea/gitea/branch_protections/master", nil) - assert.NoError(t, err) - r.ServeHTTP(recorder, req) - assert.EqualValues(t, http.StatusOK, recorder.Code) - assert.EqualValues(t, 2, hit) + t.Run("Sub Router", func(t *testing.T) { + testRoute("GET /api/v1/repos/the-user/the-repo/branches", resultStruct{ + method: "GET", + pathParams: map[string]string{"username": "the-user", "reponame": "the-repo"}, + }) - req, err = http.NewRequest("PATCH", "http://localhost:8000/api/v1/repos/gitea/gitea/branch_protections/master", nil) - assert.NoError(t, err) - r.ServeHTTP(recorder, req) - assert.EqualValues(t, http.StatusOK, recorder.Code) - assert.EqualValues(t, 3, hit) + testRoute("POST /api/v1/repos/the-user/the-repo/branches", resultStruct{ + method: "POST", + pathParams: map[string]string{"username": "the-user", "reponame": "the-repo"}, + }) - req, err = http.NewRequest("DELETE", "http://localhost:8000/api/v1/repos/gitea/gitea/branch_protections/master", nil) - assert.NoError(t, err) - r.ServeHTTP(recorder, req) - assert.EqualValues(t, http.StatusOK, recorder.Code) - assert.EqualValues(t, 4, hit) + testRoute("GET /api/v1/repos/the-user/the-repo/branches/master", resultStruct{ + method: "GET", + pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "name": "master"}, + }) + + testRoute("PATCH /api/v1/repos/the-user/the-repo/branches/master", resultStruct{ + method: "PATCH", + pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "name": "master"}, + }) + + testRoute("DELETE /api/v1/repos/the-user/the-repo/branches/master", resultStruct{ + method: "DELETE", + pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "name": "master"}, + }) + }) } func TestRouteNormalizePath(t *testing.T) { diff --git a/routers/api/packages/api.go b/routers/api/packages/api.go index 47ea7137b8..92411ecb4f 100644 --- a/routers/api/packages/api.go +++ b/routers/api/packages/api.go @@ -37,6 +37,8 @@ import ( "code.gitea.io/gitea/routers/api/packages/vagrant" "code.gitea.io/gitea/services/auth" "code.gitea.io/gitea/services/context" + + "github.com/go-chi/chi/v5" ) func reqPackageAccess(accessMode perm.AccessMode) func(ctx *context.Context) { @@ -139,39 +141,33 @@ func CommonRoutes() *web.Router { r.Group("/arch", func() { r.Methods("HEAD,GET", "/repository.key", arch.GetRepositoryKey) - r.Methods("HEAD,GET,PUT,DELETE", "*", func(ctx *context.Context) { - path := strings.Trim(ctx.PathParam("*"), "/") + reqPutRepository := web.NewPathProcessor("PUT", "/") + reqGetRepoArchFile := web.NewPathProcessor("HEAD,GET", "///") + reqDeleteRepoNameVerArch := web.NewPathProcessor("DELETE", "////") - if ctx.Req.Method == "PUT" { + r.Any("*", func(ctx *context.Context) { + chiCtx := chi.RouteContext(ctx.Req.Context()) + path := ctx.PathParam("*") + + if reqPutRepository.ProcessRequestPath(chiCtx, path) { reqPackageAccess(perm.AccessModeWrite)(ctx) if ctx.Written() { return } - ctx.SetPathParam("repository", path) arch.UploadPackageFile(ctx) return } - pathFields := strings.Split(path, "/") - pathFieldsLen := len(pathFields) - - if (ctx.Req.Method == "HEAD" || ctx.Req.Method == "GET") && pathFieldsLen >= 2 { - ctx.SetPathParam("repository", strings.Join(pathFields[:pathFieldsLen-2], "/")) - ctx.SetPathParam("architecture", pathFields[pathFieldsLen-2]) - ctx.SetPathParam("filename", pathFields[pathFieldsLen-1]) + if reqGetRepoArchFile.ProcessRequestPath(chiCtx, path) { arch.GetPackageOrRepositoryFile(ctx) return } - if ctx.Req.Method == "DELETE" && pathFieldsLen >= 3 { + if reqDeleteRepoNameVerArch.ProcessRequestPath(chiCtx, path) { reqPackageAccess(perm.AccessModeWrite)(ctx) if ctx.Written() { return } - ctx.SetPathParam("repository", strings.Join(pathFields[:pathFieldsLen-3], "/")) - ctx.SetPathParam("name", pathFields[pathFieldsLen-3]) - ctx.SetPathParam("version", pathFields[pathFieldsLen-2]) - ctx.SetPathParam("architecture", pathFields[pathFieldsLen-1]) arch.DeletePackageVersion(ctx) return }