diff options
author | wxiaoguang <wxiaoguang@gmail.com> | 2024-12-24 11:43:57 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-12-24 11:43:57 +0800 |
commit | 6d5aa9218e78ac500b21fb3f36674284118a7c78 (patch) | |
tree | b715679f96f126958457b6e6c42d0d26aebcb58a | |
parent | 781c6df40fcd8c8a112d048b4beb079d0b9a7f2b (diff) | |
download | gitea-6d5aa9218e78ac500b21fb3f36674284118a7c78.tar.gz gitea-6d5aa9218e78ac500b21fb3f36674284118a7c78.zip |
Refactor request context (#32956)
Introduce RequestContext: is a short-lived context that is used to store
request-specific data.
RequestContext could be used to clean form tmp files, close context git
repo, and do some tracing in the future.
Then a lot of legacy code could be removed or improved. For example:
most `ctx.Repo.GitRepo.Close()` could be removed because the git repo
could be closed when the request is done.
34 files changed, 376 insertions, 419 deletions
diff --git a/modules/gitrepo/gitrepo.go b/modules/gitrepo/gitrepo.go index 14d809aedb..831b9d7bb7 100644 --- a/modules/gitrepo/gitrepo.go +++ b/modules/gitrepo/gitrepo.go @@ -10,6 +10,7 @@ import ( "strings" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/reqctx" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" ) @@ -38,63 +39,32 @@ func OpenWikiRepository(ctx context.Context, repo Repository) (*git.Repository, // contextKey is a value for use with context.WithValue. type contextKey struct { - name string -} - -// RepositoryContextKey is a context key. It is used with context.Value() to get the current Repository for the context -var RepositoryContextKey = &contextKey{"repository"} - -// RepositoryFromContext attempts to get the repository from the context -func repositoryFromContext(ctx context.Context, repo Repository) *git.Repository { - value := ctx.Value(RepositoryContextKey) - if value == nil { - return nil - } - - if gitRepo, ok := value.(*git.Repository); ok && gitRepo != nil { - if gitRepo.Path == repoPath(repo) { - return gitRepo - } - } - - return nil + repoPath string } // RepositoryFromContextOrOpen attempts to get the repository from the context or just opens it func RepositoryFromContextOrOpen(ctx context.Context, repo Repository) (*git.Repository, io.Closer, error) { - gitRepo := repositoryFromContext(ctx, repo) - if gitRepo != nil { - return gitRepo, util.NopCloser{}, nil + ds := reqctx.GetRequestDataStore(ctx) + if ds != nil { + gitRepo, err := RepositoryFromRequestContextOrOpen(ctx, ds, repo) + return gitRepo, util.NopCloser{}, err } - gitRepo, err := OpenRepository(ctx, repo) return gitRepo, gitRepo, err } -// repositoryFromContextPath attempts to get the repository from the context -func repositoryFromContextPath(ctx context.Context, path string) *git.Repository { - value := ctx.Value(RepositoryContextKey) - if value == nil { - return nil +// RepositoryFromRequestContextOrOpen opens the repository at the given relative path in the provided request context +// The repo will be automatically closed when the request context is done +func RepositoryFromRequestContextOrOpen(ctx context.Context, ds reqctx.RequestDataStore, repo Repository) (*git.Repository, error) { + ck := contextKey{repoPath: repoPath(repo)} + if gitRepo, ok := ctx.Value(ck).(*git.Repository); ok { + return gitRepo, nil } - - if repo, ok := value.(*git.Repository); ok && repo != nil { - if repo.Path == path { - return repo - } + gitRepo, err := git.OpenRepository(ctx, ck.repoPath) + if err != nil { + return nil, err } - - return nil -} - -// RepositoryFromContextOrOpenPath attempts to get the repository from the context or just opens it -// Deprecated: Use RepositoryFromContextOrOpen instead -func RepositoryFromContextOrOpenPath(ctx context.Context, path string) (*git.Repository, io.Closer, error) { - gitRepo := repositoryFromContextPath(ctx, path) - if gitRepo != nil { - return gitRepo, util.NopCloser{}, nil - } - - gitRepo, err := git.OpenRepository(ctx, path) - return gitRepo, gitRepo, err + ds.AddCloser(gitRepo) + ds.SetContextValue(ck, gitRepo) + return gitRepo, nil } diff --git a/modules/gitrepo/walk_gogit.go b/modules/gitrepo/walk_gogit.go index 6370faf08e..709897ba0c 100644 --- a/modules/gitrepo/walk_gogit.go +++ b/modules/gitrepo/walk_gogit.go @@ -14,15 +14,11 @@ import ( // WalkReferences walks all the references from the repository // refname is empty, ObjectTag or ObjectBranch. All other values should be treated as equivalent to empty. func WalkReferences(ctx context.Context, repo Repository, walkfn func(sha1, refname string) error) (int, error) { - gitRepo := repositoryFromContext(ctx, repo) - if gitRepo == nil { - var err error - gitRepo, err = OpenRepository(ctx, repo) - if err != nil { - return 0, err - } - defer gitRepo.Close() + gitRepo, closer, err := RepositoryFromContextOrOpen(ctx, repo) + if err != nil { + return 0, err } + defer closer.Close() i := 0 iter, err := gitRepo.GoGitRepo().References() diff --git a/modules/reqctx/datastore.go b/modules/reqctx/datastore.go new file mode 100644 index 0000000000..66361a4587 --- /dev/null +++ b/modules/reqctx/datastore.go @@ -0,0 +1,123 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package reqctx + +import ( + "context" + "io" + "sync" + + "code.gitea.io/gitea/modules/process" +) + +type ContextDataProvider interface { + GetData() ContextData +} + +type ContextData map[string]any + +func (ds ContextData) GetData() ContextData { + return ds +} + +func (ds ContextData) MergeFrom(other ContextData) ContextData { + for k, v := range other { + ds[k] = v + } + return ds +} + +// RequestDataStore is a short-lived context-related object that is used to store request-specific data. +type RequestDataStore interface { + GetData() ContextData + SetContextValue(k, v any) + GetContextValue(key any) any + AddCleanUp(f func()) + AddCloser(c io.Closer) +} + +type requestDataStoreKeyType struct{} + +var RequestDataStoreKey requestDataStoreKeyType + +type requestDataStore struct { + data ContextData + + mu sync.RWMutex + values map[any]any + cleanUpFuncs []func() +} + +func (r *requestDataStore) GetContextValue(key any) any { + if key == RequestDataStoreKey { + return r + } + r.mu.RLock() + defer r.mu.RUnlock() + return r.values[key] +} + +func (r *requestDataStore) SetContextValue(k, v any) { + r.mu.Lock() + r.values[k] = v + r.mu.Unlock() +} + +// GetData and the underlying ContextData are not thread-safe, callers should ensure thread-safety. +func (r *requestDataStore) GetData() ContextData { + if r.data == nil { + r.data = make(ContextData) + } + return r.data +} + +func (r *requestDataStore) AddCleanUp(f func()) { + r.mu.Lock() + r.cleanUpFuncs = append(r.cleanUpFuncs, f) + r.mu.Unlock() +} + +func (r *requestDataStore) AddCloser(c io.Closer) { + r.AddCleanUp(func() { _ = c.Close() }) +} + +func (r *requestDataStore) cleanUp() { + for _, f := range r.cleanUpFuncs { + f() + } +} + +func GetRequestDataStore(ctx context.Context) RequestDataStore { + if req, ok := ctx.Value(RequestDataStoreKey).(*requestDataStore); ok { + return req + } + return nil +} + +type requestContext struct { + context.Context + dataStore *requestDataStore +} + +func (c *requestContext) Value(key any) any { + if v := c.dataStore.GetContextValue(key); v != nil { + return v + } + return c.Context.Value(key) +} + +func NewRequestContext(parentCtx context.Context, profDesc string) (_ context.Context, finished func()) { + ctx, _, processFinished := process.GetManager().AddTypedContext(parentCtx, profDesc, process.RequestProcessType, true) + reqCtx := &requestContext{Context: ctx, dataStore: &requestDataStore{values: make(map[any]any)}} + return reqCtx, func() { + reqCtx.dataStore.cleanUp() + processFinished() + } +} + +// NewRequestContextForTest creates a new RequestContext for testing purposes +// It doesn't add the context to the process manager, nor do cleanup +func NewRequestContextForTest(parentCtx context.Context) context.Context { + return &requestContext{Context: parentCtx, dataStore: &requestDataStore{values: make(map[any]any)}} +} diff --git a/modules/web/handler.go b/modules/web/handler.go index 1812c664b3..9a3e4a7f17 100644 --- a/modules/web/handler.go +++ b/modules/web/handler.go @@ -4,7 +4,6 @@ package web import ( - goctx "context" "fmt" "net/http" "reflect" @@ -51,7 +50,6 @@ func (r *responseWriter) WriteHeader(statusCode int) { var ( httpReqType = reflect.TypeOf((*http.Request)(nil)) respWriterType = reflect.TypeOf((*http.ResponseWriter)(nil)).Elem() - cancelFuncType = reflect.TypeOf((*goctx.CancelFunc)(nil)).Elem() ) // preCheckHandler checks whether the handler is valid, developers could get first-time feedback, all mistakes could be found at startup @@ -65,11 +63,8 @@ func preCheckHandler(fn reflect.Value, argsIn []reflect.Value) { if !hasStatusProvider { panic(fmt.Sprintf("handler should have at least one ResponseStatusProvider argument, but got %s", fn.Type())) } - if fn.Type().NumOut() != 0 && fn.Type().NumIn() != 1 { - panic(fmt.Sprintf("handler should have no return value or only one argument, but got %s", fn.Type())) - } - if fn.Type().NumOut() == 1 && fn.Type().Out(0) != cancelFuncType { - panic(fmt.Sprintf("handler should return a cancel function, but got %s", fn.Type())) + if fn.Type().NumOut() != 0 { + panic(fmt.Sprintf("handler should have no return value other than registered ones, but got %s", fn.Type())) } } @@ -105,16 +100,10 @@ func prepareHandleArgsIn(resp http.ResponseWriter, req *http.Request, fn reflect return argsIn } -func handleResponse(fn reflect.Value, ret []reflect.Value) goctx.CancelFunc { - if len(ret) == 1 { - if cancelFunc, ok := ret[0].Interface().(goctx.CancelFunc); ok { - return cancelFunc - } - panic(fmt.Sprintf("unsupported return type: %s", ret[0].Type())) - } else if len(ret) > 1 { +func handleResponse(fn reflect.Value, ret []reflect.Value) { + if len(ret) != 0 { panic(fmt.Sprintf("unsupported return values: %s", fn.Type())) } - return nil } func hasResponseBeenWritten(argsIn []reflect.Value) bool { @@ -171,11 +160,8 @@ func toHandlerProvider(handler any) func(next http.Handler) http.Handler { routing.UpdateFuncInfo(req.Context(), funcInfo) ret := fn.Call(argsIn) - // handle the return value, and defer the cancel function if there is one - cancelFunc := handleResponse(fn, ret) - if cancelFunc != nil { - defer cancelFunc() - } + // handle the return value (no-op at the moment) + handleResponse(fn, ret) // if the response has not been written, call the next handler if next != nil && !hasResponseBeenWritten(argsIn) { diff --git a/modules/web/middleware/data.go b/modules/web/middleware/data.go index 08d83f94be..a47da0f836 100644 --- a/modules/web/middleware/data.go +++ b/modules/web/middleware/data.go @@ -7,46 +7,21 @@ import ( "context" "time" + "code.gitea.io/gitea/modules/reqctx" "code.gitea.io/gitea/modules/setting" ) -// ContextDataStore represents a data store -type ContextDataStore interface { - GetData() ContextData -} - -type ContextData map[string]any - -func (ds ContextData) GetData() ContextData { - return ds -} - -func (ds ContextData) MergeFrom(other ContextData) ContextData { - for k, v := range other { - ds[k] = v - } - return ds -} - const ContextDataKeySignedUser = "SignedUser" -type contextDataKeyType struct{} - -var contextDataKey contextDataKeyType - -func WithContextData(c context.Context) context.Context { - return context.WithValue(c, contextDataKey, make(ContextData, 10)) -} - -func GetContextData(c context.Context) ContextData { - if ds, ok := c.Value(contextDataKey).(ContextData); ok { - return ds +func GetContextData(c context.Context) reqctx.ContextData { + if rc := reqctx.GetRequestDataStore(c); rc != nil { + return rc.GetData() } return nil } -func CommonTemplateContextData() ContextData { - return ContextData{ +func CommonTemplateContextData() reqctx.ContextData { + return reqctx.ContextData{ "IsLandingPageOrganizations": setting.LandingPageURL == setting.LandingPageOrganizations, "ShowRegistrationButton": setting.Service.ShowRegistrationButton, diff --git a/modules/web/middleware/flash.go b/modules/web/middleware/flash.go index 88da2049a4..0caaa8c036 100644 --- a/modules/web/middleware/flash.go +++ b/modules/web/middleware/flash.go @@ -7,11 +7,13 @@ import ( "fmt" "html/template" "net/url" + + "code.gitea.io/gitea/modules/reqctx" ) // Flash represents a one time data transfer between two requests. type Flash struct { - DataStore ContextDataStore + DataStore reqctx.RequestDataStore url.Values ErrorMsg, WarningMsg, InfoMsg, SuccessMsg string } diff --git a/modules/web/route.go b/modules/web/route.go index 787521dfb0..533ac3eaf1 100644 --- a/modules/web/route.go +++ b/modules/web/route.go @@ -10,6 +10,7 @@ import ( "strings" "code.gitea.io/gitea/modules/htmlutil" + "code.gitea.io/gitea/modules/reqctx" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/web/middleware" @@ -29,12 +30,12 @@ func Bind[T any](_ T) http.HandlerFunc { } // SetForm set the form object -func SetForm(dataStore middleware.ContextDataStore, obj any) { +func SetForm(dataStore reqctx.ContextDataProvider, obj any) { dataStore.GetData()["__form"] = obj } // GetForm returns the validate form information -func GetForm(dataStore middleware.ContextDataStore) any { +func GetForm(dataStore reqctx.RequestDataStore) any { return dataStore.GetData()["__form"] } diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index 0a7f92ac40..910edd6d58 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -126,11 +126,10 @@ func ArtifactsRoutes(prefix string) *web.Router { func ArtifactContexter() func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { - base, baseCleanUp := context.NewBaseContext(resp, req) - defer baseCleanUp() + base := context.NewBaseContext(resp, req) ctx := &ArtifactContext{Base: base} - ctx.AppendContextValue(artifactContextKey, ctx) + ctx.SetContextValue(artifactContextKey, ctx) // action task call server api with Bearer ACTIONS_RUNTIME_TOKEN // we should verify the ACTIONS_RUNTIME_TOKEN diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index 6dd36888d2..8917a7a8a2 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -126,12 +126,9 @@ type artifactV4Routes struct { func ArtifactV4Contexter() func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { - base, baseCleanUp := context.NewBaseContext(resp, req) - defer baseCleanUp() - + base := context.NewBaseContext(resp, req) ctx := &ArtifactContext{Base: base} - ctx.AppendContextValue(artifactContextKey, ctx) - + ctx.SetContextValue(artifactContextKey, ctx) next.ServeHTTP(ctx.Resp, ctx.Req) }) } diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 9a31aec314..f6df866efc 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -729,15 +729,11 @@ func CreateBranchProtection(ctx *context.APIContext) { } else { if !isPlainRule { if ctx.Repo.GitRepo == nil { - ctx.Repo.GitRepo, err = gitrepo.OpenRepository(ctx, ctx.Repo.Repository) + ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, ctx.Repo.Repository) if err != nil { ctx.Error(http.StatusInternalServerError, "OpenRepository", err) return } - defer func() { - ctx.Repo.GitRepo.Close() - ctx.Repo.GitRepo = nil - }() } // FIXME: since we only need to recheck files protected rules, we could improve this matchedBranches, err := git_model.FindAllMatchedBranches(ctx, ctx.Repo.Repository.ID, ruleName) @@ -1061,15 +1057,11 @@ func EditBranchProtection(ctx *context.APIContext) { } else { if !isPlainRule { if ctx.Repo.GitRepo == nil { - ctx.Repo.GitRepo, err = gitrepo.OpenRepository(ctx, ctx.Repo.Repository) + ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, ctx.Repo.Repository) if err != nil { ctx.Error(http.StatusInternalServerError, "OpenRepository", err) return } - defer func() { - ctx.Repo.GitRepo.Close() - ctx.Repo.GitRepo = nil - }() } // FIXME: since we only need to recheck files protected rules, we could improve this diff --git a/routers/api/v1/repo/compare.go b/routers/api/v1/repo/compare.go index 1678bc033c..87b890cb62 100644 --- a/routers/api/v1/repo/compare.go +++ b/routers/api/v1/repo/compare.go @@ -44,13 +44,12 @@ func CompareDiff(ctx *context.APIContext) { // "$ref": "#/responses/notFound" if ctx.Repo.GitRepo == nil { - gitRepo, err := gitrepo.OpenRepository(ctx, ctx.Repo.Repository) + var err error + ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, ctx.Repo.Repository) if err != nil { ctx.Error(http.StatusInternalServerError, "OpenRepository", err) return } - ctx.Repo.GitRepo = gitRepo - defer gitRepo.Close() } infoPath := ctx.PathParam("*") diff --git a/routers/api/v1/repo/download.go b/routers/api/v1/repo/download.go index 3620c1465f..eb967772ed 100644 --- a/routers/api/v1/repo/download.go +++ b/routers/api/v1/repo/download.go @@ -28,13 +28,12 @@ func DownloadArchive(ctx *context.APIContext) { } if ctx.Repo.GitRepo == nil { - gitRepo, err := gitrepo.OpenRepository(ctx, ctx.Repo.Repository) + var err error + ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, ctx.Repo.Repository) if err != nil { ctx.Error(http.StatusInternalServerError, "OpenRepository", err) return } - ctx.Repo.GitRepo = gitRepo - defer gitRepo.Close() } r, err := archiver_service.NewRequest(ctx.Repo.Repository.ID, ctx.Repo.GitRepo, ctx.PathParam("*"), tp) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 83848b7add..6591b9a752 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -287,13 +287,12 @@ func GetArchive(ctx *context.APIContext) { // "$ref": "#/responses/notFound" if ctx.Repo.GitRepo == nil { - gitRepo, err := gitrepo.OpenRepository(ctx, ctx.Repo.Repository) + var err error + ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, ctx.Repo.Repository) if err != nil { ctx.Error(http.StatusInternalServerError, "OpenRepository", err) return } - ctx.Repo.GitRepo = gitRepo - defer gitRepo.Close() } archiveDownload(ctx) diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 40990a28cb..f0f5db0536 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -726,12 +726,11 @@ func updateBasicProperties(ctx *context.APIContext, opts api.EditRepoOption) err if ctx.Repo.GitRepo == nil && !repo.IsEmpty { var err error - ctx.Repo.GitRepo, err = gitrepo.OpenRepository(ctx, repo) + ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, repo) if err != nil { ctx.Error(http.StatusInternalServerError, "Unable to OpenRepository", err) return err } - defer ctx.Repo.GitRepo.Close() } // Default branch only updated if changed and exist or the repository is empty diff --git a/routers/api/v1/repo/transfer.go b/routers/api/v1/repo/transfer.go index 787ec34404..b2090cac41 100644 --- a/routers/api/v1/repo/transfer.go +++ b/routers/api/v1/repo/transfer.go @@ -100,7 +100,7 @@ func Transfer(ctx *context.APIContext) { } if ctx.Repo.GitRepo != nil { - ctx.Repo.GitRepo.Close() + _ = ctx.Repo.GitRepo.Close() ctx.Repo.GitRepo = nil } diff --git a/routers/common/errpage_test.go b/routers/common/errpage_test.go index 4fd63ba49e..dfea55f510 100644 --- a/routers/common/errpage_test.go +++ b/routers/common/errpage_test.go @@ -12,8 +12,8 @@ import ( "testing" "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/reqctx" "code.gitea.io/gitea/modules/test" - "code.gitea.io/gitea/modules/web/middleware" "github.com/stretchr/testify/assert" ) @@ -21,7 +21,7 @@ import ( func TestRenderPanicErrorPage(t *testing.T) { w := httptest.NewRecorder() req := &http.Request{URL: &url.URL{}} - req = req.WithContext(middleware.WithContextData(context.Background())) + req = req.WithContext(reqctx.NewRequestContextForTest(context.Background())) RenderPanicErrorPage(w, req, errors.New("fake panic error (for test only)")) respContent := w.Body.String() assert.Contains(t, respContent, `class="page-content status-page-500"`) diff --git a/routers/common/middleware.go b/routers/common/middleware.go index 51e42d87a0..047d327ce8 100644 --- a/routers/common/middleware.go +++ b/routers/common/middleware.go @@ -4,16 +4,14 @@ package common import ( - go_context "context" "fmt" "net/http" "strings" "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/httplib" - "code.gitea.io/gitea/modules/process" + "code.gitea.io/gitea/modules/reqctx" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/web/middleware" "code.gitea.io/gitea/modules/web/routing" "code.gitea.io/gitea/services/context" @@ -24,65 +22,76 @@ import ( // ProtocolMiddlewares returns HTTP protocol related middlewares, and it provides a global panic recovery func ProtocolMiddlewares() (handlers []any) { - // make sure chi uses EscapedPath(RawPath) as RoutePath, then "%2f" could be handled correctly - handlers = append(handlers, func(next http.Handler) http.Handler { - return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { - ctx := chi.RouteContext(req.Context()) - if req.URL.RawPath == "" { - ctx.RoutePath = req.URL.EscapedPath() - } else { - ctx.RoutePath = req.URL.RawPath - } - next.ServeHTTP(resp, req) - }) - }) + // the order is important + handlers = append(handlers, ChiRoutePathHandler()) // make sure chi has correct paths + handlers = append(handlers, RequestContextHandler()) // // prepare the context and panic recovery - // prepare the ContextData and panic recovery - handlers = append(handlers, func(next http.Handler) http.Handler { + if setting.ReverseProxyLimit > 0 && len(setting.ReverseProxyTrustedProxies) > 0 { + handlers = append(handlers, ForwardedHeadersHandler(setting.ReverseProxyLimit, setting.ReverseProxyTrustedProxies)) + } + + if setting.IsRouteLogEnabled() { + handlers = append(handlers, routing.NewLoggerHandler()) + } + + if setting.IsAccessLogEnabled() { + handlers = append(handlers, context.AccessLogger()) + } + + return handlers +} + +func RequestContextHandler() func(h http.Handler) http.Handler { + return func(next http.Handler) http.Handler { return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { + profDesc := fmt.Sprintf("%s: %s", req.Method, req.RequestURI) + ctx, finished := reqctx.NewRequestContext(req.Context(), profDesc) + defer finished() + defer func() { if err := recover(); err != nil { RenderPanicErrorPage(resp, req, err) // it should never panic } }() - req = req.WithContext(middleware.WithContextData(req.Context())) - req = req.WithContext(go_context.WithValue(req.Context(), httplib.RequestContextKey, req)) - next.ServeHTTP(resp, req) - }) - }) - // wrap the request and response, use the process context and add it to the process manager - handlers = append(handlers, func(next http.Handler) http.Handler { - return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { - ctx, _, finished := process.GetManager().AddTypedContext(req.Context(), fmt.Sprintf("%s: %s", req.Method, req.RequestURI), process.RequestProcessType, true) - defer finished() - next.ServeHTTP(context.WrapResponseWriter(resp), req.WithContext(cache.WithCacheContext(ctx))) + ds := reqctx.GetRequestDataStore(ctx) + req = req.WithContext(cache.WithCacheContext(ctx)) + ds.SetContextValue(httplib.RequestContextKey, req) + ds.AddCleanUp(func() { + if req.MultipartForm != nil { + _ = req.MultipartForm.RemoveAll() // remove the temp files buffered to tmp directory + } + }) + next.ServeHTTP(context.WrapResponseWriter(resp), req) }) - }) + } +} - if setting.ReverseProxyLimit > 0 { - opt := proxy.NewForwardedHeadersOptions(). - WithForwardLimit(setting.ReverseProxyLimit). - ClearTrustedProxies() - for _, n := range setting.ReverseProxyTrustedProxies { - if !strings.Contains(n, "/") { - opt.AddTrustedProxy(n) +func ChiRoutePathHandler() func(h http.Handler) http.Handler { + // make sure chi uses EscapedPath(RawPath) as RoutePath, then "%2f" could be handled correctly + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { + ctx := chi.RouteContext(req.Context()) + if req.URL.RawPath == "" { + ctx.RoutePath = req.URL.EscapedPath() } else { - opt.AddTrustedNetwork(n) + ctx.RoutePath = req.URL.RawPath } - } - handlers = append(handlers, proxy.ForwardedHeaders(opt)) - } - - if setting.IsRouteLogEnabled() { - handlers = append(handlers, routing.NewLoggerHandler()) + next.ServeHTTP(resp, req) + }) } +} - if setting.IsAccessLogEnabled() { - handlers = append(handlers, context.AccessLogger()) +func ForwardedHeadersHandler(limit int, trustedProxies []string) func(h http.Handler) http.Handler { + opt := proxy.NewForwardedHeadersOptions().WithForwardLimit(limit).ClearTrustedProxies() + for _, n := range trustedProxies { + if !strings.Contains(n, "/") { + opt.AddTrustedProxy(n) + } else { + opt.AddTrustedNetwork(n) + } } - - return handlers + return proxy.ForwardedHeaders(opt) } func Sessioner() func(next http.Handler) http.Handler { diff --git a/routers/install/install.go b/routers/install/install.go index 1819bafc62..8a1d57aa0b 100644 --- a/routers/install/install.go +++ b/routers/install/install.go @@ -25,6 +25,7 @@ import ( "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/optional" + "code.gitea.io/gitea/modules/reqctx" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/modules/timeutil" @@ -61,15 +62,11 @@ func Contexter() func(next http.Handler) http.Handler { envConfigKeys := setting.CollectEnvConfigKeys() return func(next http.Handler) http.Handler { return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { - base, baseCleanUp := context.NewBaseContext(resp, req) - defer baseCleanUp() - + base := context.NewBaseContext(resp, req) ctx := context.NewWebContext(base, rnd, session.GetSession(req)) - ctx.AppendContextValue(context.WebContextKey, ctx) + ctx.SetContextValue(context.WebContextKey, ctx) ctx.Data.MergeFrom(middleware.CommonTemplateContextData()) - ctx.Data.MergeFrom(middleware.ContextData{ - "Context": ctx, // TODO: use "ctx" in template and remove this - "locale": ctx.Locale, + ctx.Data.MergeFrom(reqctx.ContextData{ "Title": ctx.Locale.Tr("install.install"), "PageIsInstall": true, "DbTypeNames": dbTypeNames, diff --git a/routers/private/internal.go b/routers/private/internal.go index 1fb72f13d9..a78c76f897 100644 --- a/routers/private/internal.go +++ b/routers/private/internal.go @@ -63,8 +63,8 @@ func Routes() *web.Router { r.Post("/ssh/{id}/update/{repoid}", UpdatePublicKeyInRepo) r.Post("/ssh/log", bind(private.SSHLogOption{}), SSHLog) r.Post("/hook/pre-receive/{owner}/{repo}", RepoAssignment, bind(private.HookOptions{}), HookPreReceive) - r.Post("/hook/post-receive/{owner}/{repo}", context.OverrideContext, bind(private.HookOptions{}), HookPostReceive) - r.Post("/hook/proc-receive/{owner}/{repo}", context.OverrideContext, RepoAssignment, bind(private.HookOptions{}), HookProcReceive) + r.Post("/hook/post-receive/{owner}/{repo}", context.OverrideContext(), bind(private.HookOptions{}), HookPostReceive) + r.Post("/hook/proc-receive/{owner}/{repo}", context.OverrideContext(), RepoAssignment, bind(private.HookOptions{}), HookProcReceive) r.Post("/hook/set-default-branch/{owner}/{repo}/{branch}", RepoAssignment, SetDefaultBranch) r.Get("/serv/none/{keyid}", ServNoCommand) r.Get("/serv/command/{keyid}/{owner}/{repo}", ServCommand) @@ -88,7 +88,7 @@ func Routes() *web.Router { // Fortunately, the LFS handlers are able to handle requests without a complete web context common.AddOwnerRepoGitLFSRoutes(r, func(ctx *context.PrivateContext) { webContext := &context.Context{Base: ctx.Base} - ctx.AppendContextValue(context.WebContextKey, webContext) + ctx.SetContextValue(context.WebContextKey, webContext) }) }) diff --git a/routers/private/internal_repo.go b/routers/private/internal_repo.go index aad0a3fb1a..4255be7e52 100644 --- a/routers/private/internal_repo.go +++ b/routers/private/internal_repo.go @@ -4,7 +4,6 @@ package private import ( - "context" "fmt" "net/http" @@ -17,40 +16,29 @@ import ( // This file contains common functions relating to setting the Repository for the internal routes -// RepoAssignment assigns the repository and gitrepository to the private context -func RepoAssignment(ctx *gitea_context.PrivateContext) context.CancelFunc { +// RepoAssignment assigns the repository and git repository to the private context +func RepoAssignment(ctx *gitea_context.PrivateContext) { ownerName := ctx.PathParam(":owner") repoName := ctx.PathParam(":repo") repo := loadRepository(ctx, ownerName, repoName) if ctx.Written() { // Error handled in loadRepository - return nil + return } - gitRepo, err := gitrepo.OpenRepository(ctx, repo) + gitRepo, err := gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, repo) if err != nil { log.Error("Failed to open repository: %s/%s Error: %v", ownerName, repoName, err) ctx.JSON(http.StatusInternalServerError, private.Response{ Err: fmt.Sprintf("Failed to open repository: %s/%s Error: %v", ownerName, repoName, err), }) - return nil + return } - ctx.Repo = &gitea_context.Repository{ Repository: repo, GitRepo: gitRepo, } - - // We opened it, we should close it - cancel := func() { - // If it's been set to nil then assume someone else has closed it. - if ctx.Repo.GitRepo != nil { - ctx.Repo.GitRepo.Close() - } - } - - return cancel } func loadRepository(ctx *gitea_context.PrivateContext, ownerName, repoName string) *repo_model.Repository { diff --git a/routers/web/web.go b/routers/web/web.go index e1005aae44..4f2a8b72c0 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -4,7 +4,6 @@ package web import ( - gocontext "context" "net/http" "strings" @@ -1521,24 +1520,23 @@ func registerRoutes(m *web.Router) { m.Group("/blob_excerpt", func() { m.Get("/{sha}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.ExcerptBlob) - }, func(ctx *context.Context) gocontext.CancelFunc { + }, func(ctx *context.Context) { // FIXME: refactor this function, use separate routes for wiki/code if ctx.FormBool("wiki") { ctx.Data["PageIsWiki"] = true repo.MustEnableWiki(ctx) - return nil + return } if ctx.Written() { - return nil + return } - cancel := context.RepoRef()(ctx) + context.RepoRef()(ctx) if ctx.Written() { - return cancel + return } repo.MustBeNotEmpty(ctx) - return cancel }) m.Group("/media", func() { diff --git a/services/auth/interface.go b/services/auth/interface.go index ece28af12d..275b4dd56c 100644 --- a/services/auth/interface.go +++ b/services/auth/interface.go @@ -8,12 +8,11 @@ import ( "net/http" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/reqctx" "code.gitea.io/gitea/modules/session" - "code.gitea.io/gitea/modules/web/middleware" ) -// DataStore represents a data store -type DataStore middleware.ContextDataStore +type DataStore = reqctx.ContextDataProvider // SessionStore represents a session store type SessionStore session.Store diff --git a/services/auth/oauth2_test.go b/services/auth/oauth2_test.go index b706847e8e..0d9e793cf3 100644 --- a/services/auth/oauth2_test.go +++ b/services/auth/oauth2_test.go @@ -9,7 +9,7 @@ import ( "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/web/middleware" + "code.gitea.io/gitea/modules/reqctx" "code.gitea.io/gitea/services/actions" "github.com/stretchr/testify/assert" @@ -23,7 +23,7 @@ func TestUserIDFromToken(t *testing.T) { token, err := actions.CreateAuthorizationToken(RunningTaskID, 1, 2) assert.NoError(t, err) - ds := make(middleware.ContextData) + ds := make(reqctx.ContextData) o := OAuth2{} uid := o.userIDFromToken(context.Background(), token, ds) diff --git a/services/context/api.go b/services/context/api.go index b45e80a329..7b604c5ea1 100644 --- a/services/context/api.go +++ b/services/context/api.go @@ -5,7 +5,6 @@ package context import ( - "context" "fmt" "net/http" "net/url" @@ -212,17 +211,15 @@ func (ctx *APIContext) SetLinkHeader(total, pageSize int) { func APIContexter() func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - base, baseCleanUp := NewBaseContext(w, req) + base := NewBaseContext(w, req) ctx := &APIContext{ Base: base, Cache: cache.GetCache(), Repo: &Repository{PullRequest: &PullRequest{}}, Org: &APIOrganization{}, } - defer baseCleanUp() - ctx.Base.AppendContextValue(apiContextKey, ctx) - ctx.Base.AppendContextValueFunc(gitrepo.RepositoryContextKey, func() any { return ctx.Repo.GitRepo }) + ctx.SetContextValue(apiContextKey, ctx) // If request sends files, parse them here otherwise the Query() can't be parsed and the CsrfToken will be invalid. if ctx.Req.Method == "POST" && strings.Contains(ctx.Req.Header.Get("Content-Type"), "multipart/form-data") { @@ -267,31 +264,22 @@ func (ctx *APIContext) NotFound(objs ...any) { // ReferencesGitRepo injects the GitRepo into the Context // you can optional skip the IsEmpty check -func ReferencesGitRepo(allowEmpty ...bool) func(ctx *APIContext) (cancel context.CancelFunc) { - return func(ctx *APIContext) (cancel context.CancelFunc) { +func ReferencesGitRepo(allowEmpty ...bool) func(ctx *APIContext) { + return func(ctx *APIContext) { // Empty repository does not have reference information. if ctx.Repo.Repository.IsEmpty && !(len(allowEmpty) != 0 && allowEmpty[0]) { - return nil + return } // For API calls. if ctx.Repo.GitRepo == nil { - gitRepo, err := gitrepo.OpenRepository(ctx, ctx.Repo.Repository) + var err error + ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, ctx.Repo.Repository) if err != nil { ctx.Error(http.StatusInternalServerError, fmt.Sprintf("Open Repository %v failed", ctx.Repo.Repository.FullName()), err) - return cancel - } - ctx.Repo.GitRepo = gitRepo - // We opened it, we should close it - return func() { - // If it's been set to nil then assume someone else has closed it. - if ctx.Repo.GitRepo != nil { - _ = ctx.Repo.GitRepo.Close() - } + return } } - - return cancel } } diff --git a/services/context/base.go b/services/context/base.go index d627095584..cb89cdea02 100644 --- a/services/context/base.go +++ b/services/context/base.go @@ -12,12 +12,12 @@ import ( "net/url" "strconv" "strings" - "time" "code.gitea.io/gitea/modules/httplib" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/optional" + "code.gitea.io/gitea/modules/reqctx" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/translation" "code.gitea.io/gitea/modules/web/middleware" @@ -25,65 +25,25 @@ import ( "github.com/go-chi/chi/v5" ) -type contextValuePair struct { - key any - valueFn func() any -} - type BaseContextKeyType struct{} var BaseContextKey BaseContextKeyType type Base struct { - originCtx context.Context - contextValues []contextValuePair + context.Context + reqctx.RequestDataStore Resp ResponseWriter Req *http.Request // Data is prepared by ContextDataStore middleware, this field only refers to the pre-created/prepared ContextData. // Although it's mainly used for MVC templates, sometimes it's also used to pass data between middlewares/handler - Data middleware.ContextData + Data reqctx.ContextData // Locale is mainly for Web context, although the API context also uses it in some cases: message response, form validation Locale translation.Locale } -func (b *Base) Deadline() (deadline time.Time, ok bool) { - return b.originCtx.Deadline() -} - -func (b *Base) Done() <-chan struct{} { - return b.originCtx.Done() -} - -func (b *Base) Err() error { - return b.originCtx.Err() -} - -func (b *Base) Value(key any) any { - for _, pair := range b.contextValues { - if pair.key == key { - return pair.valueFn() - } - } - return b.originCtx.Value(key) -} - -func (b *Base) AppendContextValueFunc(key any, valueFn func() any) any { - b.contextValues = append(b.contextValues, contextValuePair{key, valueFn}) - return b -} - -func (b *Base) AppendContextValue(key, value any) any { - b.contextValues = append(b.contextValues, contextValuePair{key, func() any { return value }}) - return b -} - -func (b *Base) GetData() middleware.ContextData { - return b.Data -} - // AppendAccessControlExposeHeaders append headers by name to "Access-Control-Expose-Headers" header func (b *Base) AppendAccessControlExposeHeaders(names ...string) { val := b.RespHeader().Get("Access-Control-Expose-Headers") @@ -295,13 +255,6 @@ func (b *Base) ServeContent(r io.ReadSeeker, opts *ServeHeaderOptions) { http.ServeContent(b.Resp, b.Req, opts.Filename, opts.LastModified, r) } -// Close frees all resources hold by Context -func (b *Base) cleanUp() { - if b.Req != nil && b.Req.MultipartForm != nil { - _ = b.Req.MultipartForm.RemoveAll() // remove the temp files buffered to tmp directory - } -} - func (b *Base) Tr(msg string, args ...any) template.HTML { return b.Locale.Tr(msg, args...) } @@ -310,17 +263,28 @@ func (b *Base) TrN(cnt any, key1, keyN string, args ...any) template.HTML { return b.Locale.TrN(cnt, key1, keyN, args...) } -func NewBaseContext(resp http.ResponseWriter, req *http.Request) (b *Base, closeFunc func()) { - b = &Base{ - originCtx: req.Context(), - Req: req, - Resp: WrapResponseWriter(resp), - Locale: middleware.Locale(resp, req), - Data: middleware.GetContextData(req.Context()), +func NewBaseContext(resp http.ResponseWriter, req *http.Request) *Base { + ds := reqctx.GetRequestDataStore(req.Context()) + b := &Base{ + Context: req.Context(), + RequestDataStore: ds, + Req: req, + Resp: WrapResponseWriter(resp), + Locale: middleware.Locale(resp, req), + Data: ds.GetData(), } b.Req = b.Req.WithContext(b) - b.AppendContextValue(BaseContextKey, b) - b.AppendContextValue(translation.ContextKey, b.Locale) - b.AppendContextValue(httplib.RequestContextKey, b.Req) - return b, b.cleanUp + ds.SetContextValue(BaseContextKey, b) + ds.SetContextValue(translation.ContextKey, b.Locale) + ds.SetContextValue(httplib.RequestContextKey, b.Req) + return b +} + +func NewBaseContextForTest(resp http.ResponseWriter, req *http.Request) *Base { + if !setting.IsInTesting { + panic("This function is only for testing") + } + ctx := reqctx.NewRequestContextForTest(req.Context()) + *req = *req.WithContext(ctx) + return NewBaseContext(resp, req) } diff --git a/services/context/base_test.go b/services/context/base_test.go index 823f20e00b..b936b76f58 100644 --- a/services/context/base_test.go +++ b/services/context/base_test.go @@ -14,6 +14,7 @@ import ( ) func TestRedirect(t *testing.T) { + setting.IsInTesting = true req, _ := http.NewRequest("GET", "/", nil) cases := []struct { @@ -28,10 +29,9 @@ func TestRedirect(t *testing.T) { } for _, c := range cases { resp := httptest.NewRecorder() - b, cleanup := NewBaseContext(resp, req) + b := NewBaseContextForTest(resp, req) resp.Header().Add("Set-Cookie", (&http.Cookie{Name: setting.SessionConfig.CookieName, Value: "dummy"}).String()) b.Redirect(c.url) - cleanup() has := resp.Header().Get("Set-Cookie") == "i_like_gitea=dummy" assert.Equal(t, c.keep, has, "url = %q", c.url) } @@ -39,9 +39,8 @@ func TestRedirect(t *testing.T) { req, _ = http.NewRequest("GET", "/", nil) resp := httptest.NewRecorder() req.Header.Add("HX-Request", "true") - b, cleanup := NewBaseContext(resp, req) + b := NewBaseContextForTest(resp, req) b.Redirect("/other") - cleanup() assert.Equal(t, "/other", resp.Header().Get("HX-Redirect")) assert.Equal(t, http.StatusNoContent, resp.Code) } diff --git a/services/context/context.go b/services/context/context.go index 0d5429e366..6715c5663d 100644 --- a/services/context/context.go +++ b/services/context/context.go @@ -18,7 +18,6 @@ import ( "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/cache" - "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/httpcache" "code.gitea.io/gitea/modules/session" "code.gitea.io/gitea/modules/setting" @@ -153,14 +152,9 @@ func Contexter() func(next http.Handler) http.Handler { } return func(next http.Handler) http.Handler { return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { - base, baseCleanUp := NewBaseContext(resp, req) - defer baseCleanUp() + base := NewBaseContext(resp, req) ctx := NewWebContext(base, rnd, session.GetContextSession(req)) - ctx.Data.MergeFrom(middleware.CommonTemplateContextData()) - if setting.IsProd && !setting.IsInTesting { - ctx.Data["Context"] = ctx // TODO: use "ctx" in template and remove this - } ctx.Data["CurrentURL"] = setting.AppSubURL + req.URL.RequestURI() ctx.Data["Link"] = ctx.Link @@ -168,9 +162,7 @@ func Contexter() func(next http.Handler) http.Handler { ctx.PageData = map[string]any{} ctx.Data["PageData"] = ctx.PageData - ctx.Base.AppendContextValue(WebContextKey, ctx) - ctx.Base.AppendContextValueFunc(gitrepo.RepositoryContextKey, func() any { return ctx.Repo.GitRepo }) - + ctx.Base.SetContextValue(WebContextKey, ctx) ctx.Csrf = NewCSRFProtector(csrfOpts) // Get the last flash message from cookie diff --git a/services/context/context_test.go b/services/context/context_test.go index 984593398d..54044644f0 100644 --- a/services/context/context_test.go +++ b/services/context/context_test.go @@ -26,6 +26,7 @@ func TestRemoveSessionCookieHeader(t *testing.T) { } func TestRedirectToCurrentSite(t *testing.T) { + setting.IsInTesting = true defer test.MockVariableValue(&setting.AppURL, "http://localhost:3000/sub/")() defer test.MockVariableValue(&setting.AppSubURL, "/sub")() cases := []struct { @@ -40,8 +41,7 @@ func TestRedirectToCurrentSite(t *testing.T) { t.Run(c.location, func(t *testing.T) { req := &http.Request{URL: &url.URL{Path: "/"}} resp := httptest.NewRecorder() - base, baseCleanUp := NewBaseContext(resp, req) - defer baseCleanUp() + base := NewBaseContextForTest(resp, req) ctx := NewWebContext(base, nil, nil) ctx.RedirectToCurrentSite(c.location) redirect := test.RedirectURL(resp) diff --git a/services/context/package.go b/services/context/package.go index 271b61e99c..e98e01acbb 100644 --- a/services/context/package.go +++ b/services/context/package.go @@ -153,12 +153,10 @@ func PackageContexter() func(next http.Handler) http.Handler { renderer := templates.HTMLRenderer() return func(next http.Handler) http.Handler { return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { - base, baseCleanUp := NewBaseContext(resp, req) - defer baseCleanUp() - + base := NewBaseContext(resp, req) // it is still needed when rendering 500 page in a package handler ctx := NewWebContext(base, renderer, nil) - ctx.Base.AppendContextValue(WebContextKey, ctx) + ctx.SetContextValue(WebContextKey, ctx) next.ServeHTTP(ctx.Resp, ctx.Req) }) } diff --git a/services/context/private.go b/services/context/private.go index 8b41949f60..51857da8fe 100644 --- a/services/context/private.go +++ b/services/context/private.go @@ -64,11 +64,9 @@ func GetPrivateContext(req *http.Request) *PrivateContext { func PrivateContexter() func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - base, baseCleanUp := NewBaseContext(w, req) + base := NewBaseContext(w, req) ctx := &PrivateContext{Base: base} - defer baseCleanUp() - ctx.Base.AppendContextValue(privateContextKey, ctx) - + ctx.SetContextValue(privateContextKey, ctx) next.ServeHTTP(ctx.Resp, ctx.Req) }) } @@ -78,8 +76,15 @@ func PrivateContexter() func(http.Handler) http.Handler { // This function should be used when there is a need for work to continue even if the request has been cancelled. // Primarily this affects hook/post-receive and hook/proc-receive both of which need to continue working even if // the underlying request has timed out from the ssh/http push -func OverrideContext(ctx *PrivateContext) (cancel context.CancelFunc) { - // We now need to override the request context as the base for our work because even if the request is cancelled we have to continue this work - ctx.Override, _, cancel = process.GetManager().AddTypedContext(graceful.GetManager().HammerContext(), fmt.Sprintf("PrivateContext: %s", ctx.Req.RequestURI), process.RequestProcessType, true) - return cancel +func OverrideContext() func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + // We now need to override the request context as the base for our work because even if the request is cancelled we have to continue this work + ctx := GetPrivateContext(req) + var finished func() + ctx.Override, _, finished = process.GetManager().AddTypedContext(graceful.GetManager().HammerContext(), fmt.Sprintf("PrivateContext: %s", ctx.Req.RequestURI), process.RequestProcessType, true) + defer finished() + next.ServeHTTP(ctx.Resp, ctx.Req) + }) + } } diff --git a/services/context/repo.go b/services/context/repo.go index e96916ca42..db5308415e 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -397,11 +397,13 @@ func repoAssignment(ctx *Context, repo *repo_model.Repository) { } // RepoAssignment returns a middleware to handle repository assignment -func RepoAssignment(ctx *Context) context.CancelFunc { +func RepoAssignment(ctx *Context) { if _, repoAssignmentOnce := ctx.Data["repoAssignmentExecuted"]; repoAssignmentOnce { // FIXME: it should panic in dev/test modes to have a clear behavior - log.Trace("RepoAssignment was exec already, skipping second call ...") - return nil + if !setting.IsProd || setting.IsInTesting { + panic("RepoAssignment should not be executed twice") + } + return } ctx.Data["repoAssignmentExecuted"] = true @@ -429,7 +431,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc { // https://github.com/golang/go/issues/19760 if ctx.FormString("go-get") == "1" { EarlyResponseForGoGetMeta(ctx) - return nil + return } if redirectUserID, err := user_model.LookupUserRedirect(ctx, userName); err == nil { @@ -442,7 +444,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc { } else { ctx.ServerError("GetUserByName", err) } - return nil + return } } ctx.Repo.Owner = owner @@ -467,7 +469,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc { redirectPath += "?" + ctx.Req.URL.RawQuery } ctx.Redirect(path.Join(setting.AppSubURL, redirectPath)) - return nil + return } // Get repository. @@ -480,7 +482,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc { } else if repo_model.IsErrRedirectNotExist(err) { if ctx.FormString("go-get") == "1" { EarlyResponseForGoGetMeta(ctx) - return nil + return } ctx.NotFound("GetRepositoryByName", nil) } else { @@ -489,13 +491,13 @@ func RepoAssignment(ctx *Context) context.CancelFunc { } else { ctx.ServerError("GetRepositoryByName", err) } - return nil + return } repo.Owner = owner repoAssignment(ctx, repo) if ctx.Written() { - return nil + return } ctx.Repo.RepoLink = repo.Link() @@ -520,7 +522,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc { }) if err != nil { ctx.ServerError("GetReleaseCountByRepoID", err) - return nil + return } ctx.Data["NumReleases"], err = db.Count[repo_model.Release](ctx, repo_model.FindReleasesOptions{ // only show draft releases for users who can write, read-only users shouldn't see draft releases. @@ -529,7 +531,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc { }) if err != nil { ctx.ServerError("GetReleaseCountByRepoID", err) - return nil + return } ctx.Data["Title"] = owner.Name + "/" + repo.Name @@ -546,14 +548,14 @@ func RepoAssignment(ctx *Context) context.CancelFunc { canSignedUserFork, err := repo_module.CanUserForkRepo(ctx, ctx.Doer, ctx.Repo.Repository) if err != nil { ctx.ServerError("CanUserForkRepo", err) - return nil + return } ctx.Data["CanSignedUserFork"] = canSignedUserFork userAndOrgForks, err := repo_model.GetForksByUserAndOrgs(ctx, ctx.Doer, ctx.Repo.Repository) if err != nil { ctx.ServerError("GetForksByUserAndOrgs", err) - return nil + return } ctx.Data["UserAndOrgForks"] = userAndOrgForks @@ -587,14 +589,14 @@ func RepoAssignment(ctx *Context) context.CancelFunc { if repo.IsFork { RetrieveBaseRepo(ctx, repo) if ctx.Written() { - return nil + return } } if repo.IsGenerated() { RetrieveTemplateRepo(ctx, repo) if ctx.Written() { - return nil + return } } @@ -609,10 +611,18 @@ func RepoAssignment(ctx *Context) context.CancelFunc { if !isHomeOrSettings { ctx.Redirect(ctx.Repo.RepoLink) } - return nil + return } - gitRepo, err := gitrepo.OpenRepository(ctx, repo) + if ctx.Repo.GitRepo != nil { + if !setting.IsProd || setting.IsInTesting { + panic("RepoAssignment: GitRepo should be nil") + } + _ = ctx.Repo.GitRepo.Close() + ctx.Repo.GitRepo = nil + } + + ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, repo) if err != nil { if strings.Contains(err.Error(), "repository does not exist") || strings.Contains(err.Error(), "no such file or directory") { log.Error("Repository %-v has a broken repository on the file system: %s Error: %v", ctx.Repo.Repository, ctx.Repo.Repository.RepoPath(), err) @@ -622,28 +632,16 @@ func RepoAssignment(ctx *Context) context.CancelFunc { if !isHomeOrSettings { ctx.Redirect(ctx.Repo.RepoLink) } - return nil + return } ctx.ServerError("RepoAssignment Invalid repo "+repo.FullName(), err) - return nil - } - if ctx.Repo.GitRepo != nil { - ctx.Repo.GitRepo.Close() - } - ctx.Repo.GitRepo = gitRepo - - // We opened it, we should close it - cancel := func() { - // If it's been set to nil then assume someone else has closed it. - if ctx.Repo.GitRepo != nil { - ctx.Repo.GitRepo.Close() - } + return } // Stop at this point when the repo is empty. if ctx.Repo.Repository.IsEmpty { ctx.Data["BranchName"] = ctx.Repo.Repository.DefaultBranch - return cancel + return } branchOpts := git_model.FindBranchOptions{ @@ -654,7 +652,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc { branchesTotal, err := db.Count[git_model.Branch](ctx, branchOpts) if err != nil { ctx.ServerError("CountBranches", err) - return cancel + return } // non-empty repo should have at least 1 branch, so this repository's branches haven't been synced yet @@ -662,7 +660,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc { branchesTotal, err = repo_module.SyncRepoBranches(ctx, ctx.Repo.Repository.ID, 0) if err != nil { ctx.ServerError("SyncRepoBranches", err) - return cancel + return } } @@ -670,7 +668,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc { // If no branch is set in the request URL, try to guess a default one. if len(ctx.Repo.BranchName) == 0 { - if len(ctx.Repo.Repository.DefaultBranch) > 0 && gitRepo.IsBranchExist(ctx.Repo.Repository.DefaultBranch) { + if len(ctx.Repo.Repository.DefaultBranch) > 0 && ctx.Repo.GitRepo.IsBranchExist(ctx.Repo.Repository.DefaultBranch) { ctx.Repo.BranchName = ctx.Repo.Repository.DefaultBranch } else { ctx.Repo.BranchName, _ = gitrepo.GetDefaultBranch(ctx, ctx.Repo.Repository) @@ -711,12 +709,12 @@ func RepoAssignment(ctx *Context) context.CancelFunc { repoTransfer, err := repo_model.GetPendingRepositoryTransfer(ctx, ctx.Repo.Repository) if err != nil { ctx.ServerError("GetPendingRepositoryTransfer", err) - return cancel + return } if err := repoTransfer.LoadAttributes(ctx); err != nil { ctx.ServerError("LoadRecipient", err) - return cancel + return } ctx.Data["RepoTransfer"] = repoTransfer @@ -731,7 +729,6 @@ func RepoAssignment(ctx *Context) context.CancelFunc { ctx.Data["GoDocDirectory"] = fullURLPrefix + "{/dir}" ctx.Data["GoDocFile"] = fullURLPrefix + "{/dir}/{file}#L{line}" } - return cancel } // RepoRefType type of repo reference @@ -750,7 +747,7 @@ const headRefName = "HEAD" // RepoRef handles repository reference names when the ref name is not // explicitly given -func RepoRef() func(*Context) context.CancelFunc { +func RepoRef() func(*Context) { // since no ref name is explicitly specified, ok to just use branch return RepoRefByType(RepoRefBranch) } @@ -865,9 +862,9 @@ type RepoRefByTypeOptions struct { // RepoRefByType handles repository reference name for a specific type // of repository reference -func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func(*Context) context.CancelFunc { +func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func(*Context) { opt := util.OptionalArg(opts) - return func(ctx *Context) (cancel context.CancelFunc) { + return func(ctx *Context) { refType := detectRefType // Empty repository does not have reference information. if ctx.Repo.Repository.IsEmpty { @@ -875,7 +872,7 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func ctx.Repo.IsViewBranch = true ctx.Repo.BranchName = ctx.Repo.Repository.DefaultBranch ctx.Data["TreePath"] = "" - return nil + return } var ( @@ -884,17 +881,10 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func ) if ctx.Repo.GitRepo == nil { - ctx.Repo.GitRepo, err = gitrepo.OpenRepository(ctx, ctx.Repo.Repository) + ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, ctx.Repo.Repository) if err != nil { ctx.ServerError(fmt.Sprintf("Open Repository %v failed", ctx.Repo.Repository.FullName()), err) - return nil - } - // We opened it, we should close it - cancel = func() { - // If it's been set to nil then assume someone else has closed it. - if ctx.Repo.GitRepo != nil { - ctx.Repo.GitRepo.Close() - } + return } } @@ -924,7 +914,7 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func ctx.Repo.Repository.MarkAsBrokenEmpty() } else { ctx.ServerError("GetBranchCommit", err) - return cancel + return } ctx.Repo.IsViewBranch = true } else { @@ -941,7 +931,7 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func ctx.Flash.Info(ctx.Tr("repo.branch.renamed", refName, renamedBranchName)) link := setting.AppSubURL + strings.Replace(ctx.Req.URL.EscapedPath(), util.PathEscapeSegments(refName), util.PathEscapeSegments(renamedBranchName), 1) ctx.Redirect(link) - return cancel + return } if refType == RepoRefBranch && ctx.Repo.GitRepo.IsBranchExist(refName) { @@ -951,7 +941,7 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetBranchCommit(refName) if err != nil { ctx.ServerError("GetBranchCommit", err) - return cancel + return } ctx.Repo.CommitID = ctx.Repo.Commit.ID.String() } else if refType == RepoRefTag && ctx.Repo.GitRepo.IsTagExist(refName) { @@ -962,10 +952,10 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func if err != nil { if git.IsErrNotExist(err) { ctx.NotFound("GetTagCommit", err) - return cancel + return } ctx.ServerError("GetTagCommit", err) - return cancel + return } ctx.Repo.CommitID = ctx.Repo.Commit.ID.String() } else if git.IsStringLikelyCommitID(ctx.Repo.GetObjectFormat(), refName, 7) { @@ -975,7 +965,7 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetCommit(refName) if err != nil { ctx.NotFound("GetCommit", err) - return cancel + return } // If short commit ID add canonical link header if len(refName) < ctx.Repo.GetObjectFormat().FullLength() { @@ -984,10 +974,10 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func } } else { if opt.IgnoreNotExistErr { - return cancel + return } ctx.NotFound("RepoRef invalid repo", fmt.Errorf("branch or tag not exist: %s", refName)) - return cancel + return } if guessLegacyPath { @@ -999,7 +989,7 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func ctx.Repo.BranchNameSubURL(), util.PathEscapeSegments(ctx.Repo.TreePath)) ctx.Redirect(redirect) - return cancel + return } } @@ -1017,12 +1007,10 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func ctx.Repo.CommitsCount, err = ctx.Repo.GetCommitsCount() if err != nil { ctx.ServerError("GetCommitsCount", err) - return cancel + return } ctx.Data["CommitsCount"] = ctx.Repo.CommitsCount ctx.Repo.GitRepo.LastCommitCache = git.NewLastCommitCache(ctx.Repo.CommitsCount, ctx.Repo.Repository.FullName(), ctx.Repo.GitRepo, cache.GetCache()) - - return cancel } } diff --git a/services/contexttest/context_tests.go b/services/contexttest/context_tests.go index 39ad5a362f..b0f71cad20 100644 --- a/services/contexttest/context_tests.go +++ b/services/contexttest/context_tests.go @@ -21,6 +21,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/gitrepo" + "code.gitea.io/gitea/modules/reqctx" "code.gitea.io/gitea/modules/session" "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/modules/translation" @@ -40,7 +41,7 @@ func mockRequest(t *testing.T, reqPath string) *http.Request { requestURL, err := url.Parse(path) assert.NoError(t, err) req := &http.Request{Method: method, Host: requestURL.Host, URL: requestURL, Form: maps.Clone(requestURL.Query()), Header: http.Header{}} - req = req.WithContext(middleware.WithContextData(req.Context())) + req = req.WithContext(reqctx.NewRequestContextForTest(req.Context())) return req } @@ -60,17 +61,16 @@ func MockContext(t *testing.T, reqPath string, opts ...MockContextOption) (*cont } resp := httptest.NewRecorder() req := mockRequest(t, reqPath) - base, baseCleanUp := context.NewBaseContext(resp, req) - _ = baseCleanUp // during test, it doesn't need to do clean up. TODO: this can be improved later + base := context.NewBaseContext(resp, req) base.Data = middleware.GetContextData(req.Context()) base.Locale = &translation.MockLocale{} chiCtx := chi.NewRouteContext() ctx := context.NewWebContext(base, opt.Render, nil) - ctx.AppendContextValue(context.WebContextKey, ctx) - ctx.AppendContextValue(chi.RouteCtxKey, chiCtx) + ctx.SetContextValue(context.WebContextKey, ctx) + ctx.SetContextValue(chi.RouteCtxKey, chiCtx) if opt.SessionStore != nil { - ctx.AppendContextValue(session.MockStoreContextKey, opt.SessionStore) + ctx.SetContextValue(session.MockStoreContextKey, opt.SessionStore) ctx.Session = opt.SessionStore } ctx.Cache = cache.GetCache() @@ -83,27 +83,24 @@ func MockContext(t *testing.T, reqPath string, opts ...MockContextOption) (*cont func MockAPIContext(t *testing.T, reqPath string) (*context.APIContext, *httptest.ResponseRecorder) { resp := httptest.NewRecorder() req := mockRequest(t, reqPath) - base, baseCleanUp := context.NewBaseContext(resp, req) + base := context.NewBaseContext(resp, req) base.Data = middleware.GetContextData(req.Context()) base.Locale = &translation.MockLocale{} ctx := &context.APIContext{Base: base} - _ = baseCleanUp // during test, it doesn't need to do clean up. TODO: this can be improved later - chiCtx := chi.NewRouteContext() - ctx.Base.AppendContextValue(chi.RouteCtxKey, chiCtx) + ctx.SetContextValue(chi.RouteCtxKey, chiCtx) return ctx, resp } func MockPrivateContext(t *testing.T, reqPath string) (*context.PrivateContext, *httptest.ResponseRecorder) { resp := httptest.NewRecorder() req := mockRequest(t, reqPath) - base, baseCleanUp := context.NewBaseContext(resp, req) + base := context.NewBaseContext(resp, req) base.Data = middleware.GetContextData(req.Context()) base.Locale = &translation.MockLocale{} ctx := &context.PrivateContext{Base: base} - _ = baseCleanUp // during test, it doesn't need to do clean up. TODO: this can be improved later chiCtx := chi.NewRouteContext() - ctx.Base.AppendContextValue(chi.RouteCtxKey, chiCtx) + ctx.SetContextValue(chi.RouteCtxKey, chiCtx) return ctx, resp } diff --git a/services/markup/renderhelper_mention_test.go b/services/markup/renderhelper_mention_test.go index f0c0eb9926..c244fa3d21 100644 --- a/services/markup/renderhelper_mention_test.go +++ b/services/markup/renderhelper_mention_test.go @@ -40,8 +40,7 @@ func TestRenderHelperMention(t *testing.T) { // when using web context, use user.IsUserVisibleToViewer to check req, err := http.NewRequest("GET", "/", nil) assert.NoError(t, err) - base, baseCleanUp := gitea_context.NewBaseContext(httptest.NewRecorder(), req) - defer baseCleanUp() + base := gitea_context.NewBaseContextForTest(httptest.NewRecorder(), req) giteaCtx := gitea_context.NewWebContext(base, &contexttest.MockRender{}, nil) assert.True(t, FormalRenderHelperFuncs().IsUsernameMentionable(giteaCtx, userPublic)) diff --git a/services/pull/merge_squash.go b/services/pull/merge_squash.go index 197d8102dd..8f8a5d82e7 100644 --- a/services/pull/merge_squash.go +++ b/services/pull/merge_squash.go @@ -10,7 +10,6 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/git" - "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" ) @@ -25,12 +24,12 @@ func getAuthorSignatureSquash(ctx *mergeContext) (*git.Signature, error) { // Try to get an signature from the same user in one of the commits, as the // poster email might be private or commits might have a different signature // than the primary email address of the poster. - gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpenPath(ctx, ctx.tmpBasePath) + gitRepo, err := git.OpenRepository(ctx, ctx.tmpBasePath) if err != nil { log.Error("%-v Unable to open base repository: %v", ctx.pr, err) return nil, err } - defer closer.Close() + defer gitRepo.Close() commits, err := gitRepo.CommitsBetweenIDs(trackingBranch, "HEAD") if err != nil { |