diff options
author | wxiaoguang <wxiaoguang@gmail.com> | 2023-05-04 14:36:34 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-05-04 14:36:34 +0800 |
commit | 5d77691d428d5302ee4df6c2a936b8e2ea9dca7e (patch) | |
tree | 4fa6b13f2ae81c5de94d84f2288ae1f5653c011f /modules | |
parent | 75ea0d5dba5dbf2f84cef2d12460fdd566d43e62 (diff) | |
download | gitea-5d77691d428d5302ee4df6c2a936b8e2ea9dca7e.tar.gz gitea-5d77691d428d5302ee4df6c2a936b8e2ea9dca7e.zip |
Improve template system and panic recovery (#24461)
Partially for #24457
Major changes:
1. The old `signedUserNameStringPointerKey` is quite hacky, use
`ctx.Data[SignedUser]` instead
2. Move duplicate code from `Contexter` to `CommonTemplateContextData`
3. Remove incorrect copying&pasting code `ctx.Data["Err_Password"] =
true` in API handlers
4. Use one unique `RenderPanicErrorPage` for panic error page rendering
5. Move `stripSlashesMiddleware` to be the first middleware
6. Install global panic recovery handler, it works for both `install`
and `web`
7. Make `500.tmpl` only depend minimal template functions/variables,
avoid triggering new panics
Screenshot:
<details>
![image](https://user-images.githubusercontent.com/2114189/235444895-cecbabb8-e7dc-4360-a31c-b982d11946a7.png)
</details>
Diffstat (limited to 'modules')
-rw-r--r-- | modules/context/access_log.go | 14 | ||||
-rw-r--r-- | modules/context/api.go | 13 | ||||
-rw-r--r-- | modules/context/context.go | 73 | ||||
-rw-r--r-- | modules/context/package.go | 3 | ||||
-rw-r--r-- | modules/context/private.go | 3 | ||||
-rw-r--r-- | modules/templates/base.go | 31 | ||||
-rw-r--r-- | modules/test/context_tests.go | 2 | ||||
-rw-r--r-- | modules/web/middleware/data.go | 62 | ||||
-rw-r--r-- | modules/web/middleware/flash.go | 4 | ||||
-rw-r--r-- | modules/web/middleware/request.go | 5 | ||||
-rw-r--r-- | modules/web/route.go | 4 |
11 files changed, 104 insertions, 110 deletions
diff --git a/modules/context/access_log.go b/modules/context/access_log.go index 64d204733b..b6468d139b 100644 --- a/modules/context/access_log.go +++ b/modules/context/access_log.go @@ -5,7 +5,6 @@ package context import ( "bytes" - "context" "fmt" "net" "net/http" @@ -13,8 +12,10 @@ import ( "text/template" "time" + user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/web/middleware" ) type routerLoggerOptions struct { @@ -26,8 +27,6 @@ type routerLoggerOptions struct { RequestID *string } -var signedUserNameStringPointerKey interface{} = "signedUserNameStringPointerKey" - const keyOfRequestIDInTemplate = ".RequestID" // According to: @@ -60,8 +59,6 @@ func AccessLogger() func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { start := time.Now() - identity := "-" - r := req.WithContext(context.WithValue(req.Context(), signedUserNameStringPointerKey, &identity)) var requestID string if needRequestID { @@ -73,9 +70,14 @@ func AccessLogger() func(http.Handler) http.Handler { reqHost = req.RemoteAddr } - next.ServeHTTP(w, r) + next.ServeHTTP(w, req) rw := w.(ResponseWriter) + identity := "-" + data := middleware.GetContextData(req.Context()) + if signedUser, ok := data[middleware.ContextDataKeySignedUser].(*user_model.User); ok { + identity = signedUser.Name + } buf := bytes.NewBuffer([]byte{}) err = logTemplate.Execute(buf, routerLoggerOptions{ req: req, diff --git a/modules/context/api.go b/modules/context/api.go index ae245ec1cb..e263dcbe8d 100644 --- a/modules/context/api.go +++ b/modules/context/api.go @@ -222,7 +222,7 @@ func APIContexter() func(http.Handler) http.Handler { ctx := APIContext{ Context: &Context{ Resp: NewResponse(w), - Data: map[string]interface{}{}, + Data: middleware.GetContextData(req.Context()), Locale: locale, Cache: cache.GetCache(), Repo: &Repository{ @@ -250,17 +250,6 @@ func APIContexter() func(http.Handler) http.Handler { ctx.Data["Context"] = &ctx next.ServeHTTP(ctx.Resp, ctx.Req) - - // Handle adding signedUserName to the context for the AccessLogger - usernameInterface := ctx.Data["SignedUserName"] - identityPtrInterface := ctx.Req.Context().Value(signedUserNameStringPointerKey) - if usernameInterface != nil && identityPtrInterface != nil { - username := usernameInterface.(string) - identityPtr := identityPtrInterface.(*string) - if identityPtr != nil && username != "" { - *identityPtr = username - } - } }) } } diff --git a/modules/context/context.go b/modules/context/context.go index cd7fcebe55..d73a26e5b6 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -55,7 +55,7 @@ type Render interface { type Context struct { Resp ResponseWriter Req *http.Request - Data map[string]interface{} // data used by MVC templates + Data middleware.ContextData // data used by MVC templates PageData map[string]interface{} // data used by JavaScript modules in one page, it's `window.config.pageData` Render Render translation.Locale @@ -97,7 +97,7 @@ func (ctx *Context) TrHTMLEscapeArgs(msg string, args ...string) string { } // GetData returns the data -func (ctx *Context) GetData() map[string]interface{} { +func (ctx *Context) GetData() middleware.ContextData { return ctx.Data } @@ -219,6 +219,7 @@ const tplStatus500 base.TplName = "status/500" // HTML calls Context.HTML and renders the template to HTTP response func (ctx *Context) HTML(status int, name base.TplName) { log.Debug("Template: %s", name) + tmplStartTime := time.Now() if !setting.IsProd { ctx.Data["TemplateName"] = name @@ -226,13 +227,19 @@ func (ctx *Context) HTML(status int, name base.TplName) { ctx.Data["TemplateLoadTimes"] = func() string { return strconv.FormatInt(time.Since(tmplStartTime).Nanoseconds()/1e6, 10) + "ms" } - if err := ctx.Render.HTML(ctx.Resp, status, string(name), templates.BaseVars().Merge(ctx.Data)); err != nil { - if status == http.StatusInternalServerError && name == tplStatus500 { - ctx.PlainText(http.StatusInternalServerError, "Unable to find HTML templates, the template system is not initialized, or Gitea can't find your template files.") - return - } + + err := ctx.Render.HTML(ctx.Resp, status, string(name), ctx.Data) + if err == nil { + return + } + + // if rendering fails, show error page + if name != tplStatus500 { err = fmt.Errorf("failed to render template: %s, error: %s", name, templates.HandleTemplateRenderingError(err)) - ctx.ServerError("Render failed", err) + ctx.ServerError("Render failed", err) // show the 500 error page + } else { + ctx.PlainText(http.StatusInternalServerError, "Unable to render status/500 page, the template system is broken, or Gitea can't find your template files.") + return } } @@ -676,7 +683,7 @@ func getCsrfOpts() CsrfOptions { } // Contexter initializes a classic context for a request. -func Contexter(ctx context.Context) func(next http.Handler) http.Handler { +func Contexter() func(next http.Handler) http.Handler { rnd := templates.HTMLRenderer() csrfOpts := getCsrfOpts() if !setting.IsProd { @@ -684,34 +691,30 @@ func Contexter(ctx context.Context) func(next http.Handler) http.Handler { } return func(next http.Handler) http.Handler { return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { - locale := middleware.Locale(resp, req) - startTime := time.Now() - link := setting.AppSubURL + strings.TrimSuffix(req.URL.EscapedPath(), "/") - ctx := Context{ Resp: NewResponse(resp), Cache: mc.GetCache(), - Locale: locale, - Link: link, + Locale: middleware.Locale(resp, req), + Link: setting.AppSubURL + strings.TrimSuffix(req.URL.EscapedPath(), "/"), Render: rnd, Session: session.GetSession(req), Repo: &Repository{ PullRequest: &PullRequest{}, }, - Org: &Organization{}, - Data: map[string]interface{}{ - "CurrentURL": setting.AppSubURL + req.URL.RequestURI(), - "PageStartTime": startTime, - "Link": link, - "RunModeIsProd": setting.IsProd, - }, + Org: &Organization{}, + Data: middleware.GetContextData(req.Context()), } defer ctx.Close() + ctx.Data.MergeFrom(middleware.CommonTemplateContextData()) + ctx.Data["Context"] = &ctx + ctx.Data["CurrentURL"] = setting.AppSubURL + req.URL.RequestURI() + ctx.Data["Link"] = ctx.Link + ctx.Data["locale"] = ctx.Locale + // PageData is passed by reference, and it will be rendered to `window.config.pageData` in `head.tmpl` for JavaScript modules - ctx.PageData = map[string]interface{}{} + ctx.PageData = map[string]any{} ctx.Data["PageData"] = ctx.PageData - ctx.Data["Context"] = &ctx ctx.Req = WithContext(req, &ctx) ctx.Csrf = PrepareCSRFProtector(csrfOpts, &ctx) @@ -755,16 +758,6 @@ func Contexter(ctx context.Context) func(next http.Handler) http.Handler { ctx.Data["CsrfTokenHtml"] = template.HTML(`<input type="hidden" name="_csrf" value="` + ctx.Data["CsrfToken"].(string) + `">`) // FIXME: do we really always need these setting? There should be someway to have to avoid having to always set these - ctx.Data["IsLandingPageHome"] = setting.LandingPageURL == setting.LandingPageHome - ctx.Data["IsLandingPageExplore"] = setting.LandingPageURL == setting.LandingPageExplore - ctx.Data["IsLandingPageOrganizations"] = setting.LandingPageURL == setting.LandingPageOrganizations - - ctx.Data["ShowRegistrationButton"] = setting.Service.ShowRegistrationButton - ctx.Data["ShowMilestonesDashboardPage"] = setting.Service.ShowMilestonesDashboardPage - ctx.Data["ShowFooterVersion"] = setting.Other.ShowFooterVersion - - ctx.Data["EnableSwagger"] = setting.API.EnableSwagger - ctx.Data["EnableOpenIDSignIn"] = setting.Service.EnableOpenIDSignIn ctx.Data["DisableMigrations"] = setting.Repository.DisableMigrations ctx.Data["DisableStars"] = setting.Repository.DisableStars ctx.Data["EnableActions"] = setting.Actions.Enabled @@ -777,21 +770,9 @@ func Contexter(ctx context.Context) func(next http.Handler) http.Handler { ctx.Data["UnitProjectsGlobalDisabled"] = unit.TypeProjects.UnitGlobalDisabled() ctx.Data["UnitActionsGlobalDisabled"] = unit.TypeActions.UnitGlobalDisabled() - ctx.Data["locale"] = locale ctx.Data["AllLangs"] = translation.AllLangs() next.ServeHTTP(ctx.Resp, ctx.Req) - - // Handle adding signedUserName to the context for the AccessLogger - usernameInterface := ctx.Data["SignedUserName"] - identityPtrInterface := ctx.Req.Context().Value(signedUserNameStringPointerKey) - if usernameInterface != nil && identityPtrInterface != nil { - username := usernameInterface.(string) - identityPtr := identityPtrInterface.(*string) - if identityPtr != nil && username != "" { - *identityPtr = username - } - } }) } } diff --git a/modules/context/package.go b/modules/context/package.go index 6c418b3164..fe5bdac19d 100644 --- a/modules/context/package.go +++ b/modules/context/package.go @@ -16,6 +16,7 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/templates" + "code.gitea.io/gitea/modules/web/middleware" ) // Package contains owner, access mode and optional the package descriptor @@ -136,7 +137,7 @@ func PackageContexter(ctx gocontext.Context) func(next http.Handler) http.Handle return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { ctx := Context{ Resp: NewResponse(resp), - Data: map[string]interface{}{}, + Data: middleware.GetContextData(req.Context()), Render: rnd, } defer ctx.Close() diff --git a/modules/context/private.go b/modules/context/private.go index 24f50fa471..f621dd6839 100644 --- a/modules/context/private.go +++ b/modules/context/private.go @@ -11,6 +11,7 @@ import ( "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/process" + "code.gitea.io/gitea/modules/web/middleware" ) // PrivateContext represents a context for private routes @@ -62,7 +63,7 @@ func PrivateContexter() func(http.Handler) http.Handler { ctx := &PrivateContext{ Context: &Context{ Resp: NewResponse(w), - Data: map[string]interface{}{}, + Data: middleware.GetContextData(req.Context()), }, } defer ctx.Close() diff --git a/modules/templates/base.go b/modules/templates/base.go index 4254a56976..ef28cc03f4 100644 --- a/modules/templates/base.go +++ b/modules/templates/base.go @@ -5,43 +5,12 @@ package templates import ( "strings" - "time" "code.gitea.io/gitea/modules/assetfs" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" ) -// Vars represents variables to be render in golang templates -type Vars map[string]interface{} - -// Merge merges another vars to the current, another Vars will override the current -func (vars Vars) Merge(another map[string]interface{}) Vars { - for k, v := range another { - vars[k] = v - } - return vars -} - -// BaseVars returns all basic vars -func BaseVars() Vars { - startTime := time.Now() - return map[string]interface{}{ - "IsLandingPageHome": setting.LandingPageURL == setting.LandingPageHome, - "IsLandingPageExplore": setting.LandingPageURL == setting.LandingPageExplore, - "IsLandingPageOrganizations": setting.LandingPageURL == setting.LandingPageOrganizations, - - "ShowRegistrationButton": setting.Service.ShowRegistrationButton, - "ShowMilestonesDashboardPage": setting.Service.ShowMilestonesDashboardPage, - "ShowFooterVersion": setting.Other.ShowFooterVersion, - "DisableDownloadSourceArchives": setting.Repository.DisableDownloadSourceArchives, - - "EnableSwagger": setting.API.EnableSwagger, - "EnableOpenIDSignIn": setting.Service.EnableOpenIDSignIn, - "PageStartTime": startTime, - } -} - func AssetFS() *assetfs.LayeredFS { return assetfs.Layered(CustomAssets(), BuiltinAssets()) } diff --git a/modules/test/context_tests.go b/modules/test/context_tests.go index 4af7651250..35dd920bb9 100644 --- a/modules/test/context_tests.go +++ b/modules/test/context_tests.go @@ -30,7 +30,7 @@ func MockContext(t *testing.T, path string) *context.Context { resp := &mockResponseWriter{} ctx := context.Context{ Render: &mockRender{}, - Data: make(map[string]interface{}), + Data: make(middleware.ContextData), Flash: &middleware.Flash{ Values: make(url.Values), }, diff --git a/modules/web/middleware/data.go b/modules/web/middleware/data.go index 43189940ee..c1f0516d7d 100644 --- a/modules/web/middleware/data.go +++ b/modules/web/middleware/data.go @@ -3,7 +3,63 @@ package middleware -// DataStore represents a data store -type DataStore interface { - GetData() map[string]interface{} +import ( + "context" + "time" + + "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() map[string]any { + 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 + } + return nil +} + +func CommonTemplateContextData() ContextData { + return ContextData{ + "IsLandingPageHome": setting.LandingPageURL == setting.LandingPageHome, + "IsLandingPageExplore": setting.LandingPageURL == setting.LandingPageExplore, + "IsLandingPageOrganizations": setting.LandingPageURL == setting.LandingPageOrganizations, + + "ShowRegistrationButton": setting.Service.ShowRegistrationButton, + "ShowMilestonesDashboardPage": setting.Service.ShowMilestonesDashboardPage, + "ShowFooterVersion": setting.Other.ShowFooterVersion, + "DisableDownloadSourceArchives": setting.Repository.DisableDownloadSourceArchives, + + "EnableSwagger": setting.API.EnableSwagger, + "EnableOpenIDSignIn": setting.Service.EnableOpenIDSignIn, + "PageStartTime": time.Now(), + + "RunModeIsProd": setting.IsProd, + } } diff --git a/modules/web/middleware/flash.go b/modules/web/middleware/flash.go index f2d7cc692d..fa29ddeffc 100644 --- a/modules/web/middleware/flash.go +++ b/modules/web/middleware/flash.go @@ -18,7 +18,7 @@ var FlashNow bool // Flash represents a one time data transfer between two requests. type Flash struct { - DataStore + DataStore ContextDataStore url.Values ErrorMsg, WarningMsg, InfoMsg, SuccessMsg string } @@ -34,7 +34,7 @@ func (f *Flash) set(name, msg string, current ...bool) { } if isShow { - f.GetData()["Flash"] = f + f.DataStore.GetData()["Flash"] = f } else { f.Set(name, msg) } diff --git a/modules/web/middleware/request.go b/modules/web/middleware/request.go index 34add27214..0bb155df70 100644 --- a/modules/web/middleware/request.go +++ b/modules/web/middleware/request.go @@ -12,8 +12,3 @@ import ( func IsAPIPath(req *http.Request) bool { return strings.HasPrefix(req.URL.Path, "/api/") } - -// IsInternalPath returns true if the specified URL is an internal API path -func IsInternalPath(req *http.Request) bool { - return strings.HasPrefix(req.URL.Path, "/api/internal/") -} diff --git a/modules/web/route.go b/modules/web/route.go index 6fd60f4ca7..d801f1025c 100644 --- a/modules/web/route.go +++ b/modules/web/route.go @@ -25,12 +25,12 @@ func Bind[T any](_ T) any { } // SetForm set the form object -func SetForm(data middleware.DataStore, obj interface{}) { +func SetForm(data middleware.ContextDataStore, obj interface{}) { data.GetData()["__form"] = obj } // GetForm returns the validate form information -func GetForm(data middleware.DataStore) interface{} { +func GetForm(data middleware.ContextDataStore) interface{} { return data.GetData()["__form"] } |