From 0aedb03996d7bdce88b1f0086151f8778b10c1a4 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 13 Nov 2024 16:58:09 +0800 Subject: [PATCH 1/5] Fix LFS route mock, realm, middleware names (#32488) 1. move "internal-lfs" route mock to "common-lfs" 2. fine tune tests 3. fix "realm" strings, according to RFC: https://datatracker.ietf.org/doc/html/rfc2617: * realm = "realm" "=" realm-value * realm-value = quoted-string 4. clarify some names of the middlewares, rename `ignXxx` to `optXxx` to match `reqXxx`, and rename ambiguous `requireSignIn` to `reqGitSignIn` --- go.mod | 1 + routers/common/lfs.go | 4 +- routers/private/internal.go | 5 +- routers/web/auth/oauth2_provider.go | 4 +- routers/web/githttp.go | 28 +++++----- routers/web/web.go | 73 +++++++++++++-------------- services/context/base.go | 5 ++ services/context/context.go | 3 ++ services/lfs/locks.go | 20 ++++---- services/lfs/server.go | 22 ++++---- tests/integration/git_lfs_ssh_test.go | 30 ++++++----- 11 files changed, 102 insertions(+), 93 deletions(-) diff --git a/go.mod b/go.mod index 60deb90222921..bbd81868684f8 100644 --- a/go.mod +++ b/go.mod @@ -330,6 +330,7 @@ replace github.com/shurcooL/vfsgen => github.com/lunny/vfsgen v0.0.0-20220105142 replace github.com/nektos/act => gitea.com/gitea/act v0.261.3 +// TODO: the only difference is in `PutObject`: the fork doesn't use `NewVerifyingReader(r, sha256.New(), oid, expectedSize)`, need to figure out why replace github.com/charmbracelet/git-lfs-transfer => gitea.com/gitea/git-lfs-transfer v0.2.0 // TODO: This could be removed after https://github.com/mholt/archiver/pull/396 merged diff --git a/routers/common/lfs.go b/routers/common/lfs.go index ba6e1163f1b89..1d2b71394bf0e 100644 --- a/routers/common/lfs.go +++ b/routers/common/lfs.go @@ -10,6 +10,8 @@ import ( "code.gitea.io/gitea/services/lfs" ) +const RouterMockPointCommonLFS = "common-lfs" + func AddOwnerRepoGitLFSRoutes(m *web.Router, middlewares ...any) { // shared by web and internal routers m.Group("/{username}/{reponame}/info/lfs", func() { @@ -25,5 +27,5 @@ func AddOwnerRepoGitLFSRoutes(m *web.Router, middlewares ...any) { m.Post("/{lid}/unlock", lfs.UnLockHandler) }, lfs.CheckAcceptMediaType) m.Any("/*", http.NotFound) - }, middlewares...) + }, append([]any{web.RouterMockPoint(RouterMockPointCommonLFS)}, middlewares...)...) } diff --git a/routers/private/internal.go b/routers/private/internal.go index db074238c6eb9..1fb72f13d9cc1 100644 --- a/routers/private/internal.go +++ b/routers/private/internal.go @@ -20,8 +20,6 @@ import ( chi_middleware "github.com/go-chi/chi/v5/middleware" ) -const RouterMockPointInternalLFS = "internal-lfs" - func authInternal(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { if setting.InternalToken == "" { @@ -87,10 +85,11 @@ func Routes() *web.Router { r.Group("/repo", func() { // FIXME: it is not right to use context.Contexter here because all routes here should use PrivateContext + // 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) - }, web.RouterMockPoint(RouterMockPointInternalLFS)) + }) }) return r diff --git a/routers/web/auth/oauth2_provider.go b/routers/web/auth/oauth2_provider.go index 29827b062de84..faea34959fb5b 100644 --- a/routers/web/auth/oauth2_provider.go +++ b/routers/web/auth/oauth2_provider.go @@ -91,7 +91,7 @@ type userInfoResponse struct { // InfoOAuth manages request for userinfo endpoint func InfoOAuth(ctx *context.Context) { if ctx.Doer == nil || ctx.Data["AuthedMethod"] != (&auth_service.OAuth2{}).Name() { - ctx.Resp.Header().Set("WWW-Authenticate", `Bearer realm=""`) + ctx.Resp.Header().Set("WWW-Authenticate", `Bearer realm="Gitea OAuth2"`) ctx.PlainText(http.StatusUnauthorized, "no valid authorization") return } @@ -136,7 +136,7 @@ func IntrospectOAuth(ctx *context.Context) { clientIDValid = err == nil && app.ValidateClientSecret([]byte(clientSecret)) } if !clientIDValid { - ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm=""`) + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="Gitea OAuth2"`) ctx.PlainText(http.StatusUnauthorized, "no valid authorization") return } diff --git a/routers/web/githttp.go b/routers/web/githttp.go index 102c92e12094d..5831e6f52366d 100644 --- a/routers/web/githttp.go +++ b/routers/web/githttp.go @@ -12,21 +12,19 @@ import ( "code.gitea.io/gitea/services/context" ) -func requireSignIn(ctx *context.Context) { - if !setting.Service.RequireSignInView { - return +func addOwnerRepoGitHTTPRouters(m *web.Router) { + reqGitSignIn := func(ctx *context.Context) { + if !setting.Service.RequireSignInView { + return + } + // rely on the results of Contexter + if !ctx.IsSigned { + // TODO: support digit auth - which would be Authorization header with digit + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="Gitea"`) + ctx.Error(http.StatusUnauthorized) + } } - - // rely on the results of Contexter - if !ctx.IsSigned { - // TODO: support digit auth - which would be Authorization header with digit - ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="Gitea"`) - ctx.Error(http.StatusUnauthorized) - } -} - -func gitHTTPRouters(m *web.Router) { - m.Group("", func() { + m.Group("/{username}/{reponame}", func() { m.Methods("POST,OPTIONS", "/git-upload-pack", repo.ServiceUploadPack) m.Methods("POST,OPTIONS", "/git-receive-pack", repo.ServiceReceivePack) m.Methods("GET,OPTIONS", "/info/refs", repo.GetInfoRefs) @@ -38,5 +36,5 @@ func gitHTTPRouters(m *web.Router) { m.Methods("GET,OPTIONS", "/objects/{head:[0-9a-f]{2}}/{hash:[0-9a-f]{38,62}}", repo.GetLooseObject) m.Methods("GET,OPTIONS", "/objects/pack/pack-{file:[0-9a-f]{40,64}}.pack", repo.GetPackFile) m.Methods("GET,OPTIONS", "/objects/pack/pack-{file:[0-9a-f]{40,64}}.idx", repo.GetIdxFile) - }, ignSignInAndCsrf, requireSignIn, repo.HTTPGitEnabledHandler, repo.CorsHandler(), context.UserAssignmentWeb()) + }, optSignInIgnoreCsrf, reqGitSignIn, repo.HTTPGitEnabledHandler, repo.CorsHandler(), context.UserAssignmentWeb()) } diff --git a/routers/web/web.go b/routers/web/web.go index c56906c10de8c..137c67730652d 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -291,15 +291,16 @@ func Routes() *web.Router { return routes } -var ignSignInAndCsrf = verifyAuthWithOptions(&common.VerifyOptions{DisableCSRF: true}) +var optSignInIgnoreCsrf = verifyAuthWithOptions(&common.VerifyOptions{DisableCSRF: true}) // registerRoutes register routes func registerRoutes(m *web.Router) { + // required to be signed in or signed out reqSignIn := verifyAuthWithOptions(&common.VerifyOptions{SignInRequired: true}) reqSignOut := verifyAuthWithOptions(&common.VerifyOptions{SignOutRequired: true}) - // TODO: rename them to "optSignIn", which means that the "sign-in" could be optional, depends on the VerifyOptions (RequireSignInView) - ignSignIn := verifyAuthWithOptions(&common.VerifyOptions{SignInRequired: setting.Service.RequireSignInView}) - ignExploreSignIn := verifyAuthWithOptions(&common.VerifyOptions{SignInRequired: setting.Service.RequireSignInView || setting.Service.Explore.RequireSigninView}) + // optional sign in (if signed in, use the user as doer, if not, no doer) + optSignIn := verifyAuthWithOptions(&common.VerifyOptions{SignInRequired: setting.Service.RequireSignInView}) + optExploreSignIn := verifyAuthWithOptions(&common.VerifyOptions{SignInRequired: setting.Service.RequireSignInView || setting.Service.Explore.RequireSigninView}) validation.AddBindingRules() @@ -470,7 +471,7 @@ func registerRoutes(m *web.Router) { // Especially some AJAX requests, we can reduce middleware number to improve performance. m.Get("/", Home) - m.Get("/sitemap.xml", sitemapEnabled, ignExploreSignIn, HomeSitemap) + m.Get("/sitemap.xml", sitemapEnabled, optExploreSignIn, HomeSitemap) m.Group("/.well-known", func() { m.Get("/openid-configuration", auth.OIDCWellKnown) m.Group("", func() { @@ -500,7 +501,7 @@ func registerRoutes(m *web.Router) { } }, explore.Code) m.Get("/topics/search", explore.TopicSearch) - }, ignExploreSignIn) + }, optExploreSignIn) m.Group("/issues", func() { m.Get("", user.Issues) @@ -558,12 +559,12 @@ func registerRoutes(m *web.Router) { m.Post("/grant", web.Bind(forms.GrantApplicationForm{}), auth.GrantApplicationOAuth) // TODO manage redirection m.Post("/authorize", web.Bind(forms.AuthorizationForm{}), auth.AuthorizeOAuth) - }, ignSignInAndCsrf, reqSignIn) + }, optSignInIgnoreCsrf, reqSignIn) - m.Methods("GET, OPTIONS", "/userinfo", optionsCorsHandler(), ignSignInAndCsrf, auth.InfoOAuth) - m.Methods("POST, OPTIONS", "/access_token", optionsCorsHandler(), web.Bind(forms.AccessTokenForm{}), ignSignInAndCsrf, auth.AccessTokenOAuth) - m.Methods("GET, OPTIONS", "/keys", optionsCorsHandler(), ignSignInAndCsrf, auth.OIDCKeys) - m.Methods("POST, OPTIONS", "/introspect", optionsCorsHandler(), web.Bind(forms.IntrospectTokenForm{}), ignSignInAndCsrf, auth.IntrospectOAuth) + m.Methods("GET, OPTIONS", "/userinfo", optionsCorsHandler(), optSignInIgnoreCsrf, auth.InfoOAuth) + m.Methods("POST, OPTIONS", "/access_token", optionsCorsHandler(), web.Bind(forms.AccessTokenForm{}), optSignInIgnoreCsrf, auth.AccessTokenOAuth) + m.Methods("GET, OPTIONS", "/keys", optionsCorsHandler(), optSignInIgnoreCsrf, auth.OIDCKeys) + m.Methods("POST, OPTIONS", "/introspect", optionsCorsHandler(), web.Bind(forms.IntrospectTokenForm{}), optSignInIgnoreCsrf, auth.IntrospectOAuth) }, oauth2Enabled) m.Group("/user/settings", func() { @@ -685,7 +686,7 @@ func registerRoutes(m *web.Router) { m.Post("/forgot_password", auth.ForgotPasswdPost) m.Post("/logout", auth.SignOut) m.Get("/stopwatches", reqSignIn, user.GetStopwatches) - m.Get("/search_candidates", ignExploreSignIn, user.SearchCandidates) + m.Get("/search_candidates", optExploreSignIn, user.SearchCandidates) m.Group("/oauth2", func() { m.Get("/{provider}", auth.SignInOAuth) m.Get("/{provider}/callback", auth.SignInOAuthCallback) @@ -809,7 +810,7 @@ func registerRoutes(m *web.Router) { m.Group("", func() { m.Get("/{username}", user.UsernameSubRoute) m.Methods("GET, OPTIONS", "/attachments/{uuid}", optionsCorsHandler(), repo.GetAttachment) - }, ignSignIn) + }, optSignIn) m.Post("/{username}", reqSignIn, context.UserAssignmentWeb(), user.Action) @@ -860,7 +861,7 @@ func registerRoutes(m *web.Router) { m.Group("/{org}", func() { m.Get("/members", org.Members) }, context.OrgAssignment()) - }, ignSignIn) + }, optSignIn) // end "/org": members m.Group("/org", func() { @@ -1043,14 +1044,14 @@ func registerRoutes(m *web.Router) { m.Group("", func() { m.Get("/code", user.CodeSearch) }, reqUnitAccess(unit.TypeCode, perm.AccessModeRead, false), individualPermsChecker) - }, ignSignIn, context.UserAssignmentWeb(), context.OrgAssignment()) + }, optSignIn, context.UserAssignmentWeb(), context.OrgAssignment()) // end "/{username}/-": packages, projects, code m.Group("/{username}/{reponame}/-", func() { m.Group("/migrate", func() { m.Get("/status", repo.MigrateStatus) }) - }, ignSignIn, context.RepoAssignment, reqRepoCodeReader) + }, optSignIn, context.RepoAssignment, reqRepoCodeReader) // end "/{username}/{reponame}/-": migrate m.Group("/{username}/{reponame}/settings", func() { @@ -1145,10 +1146,10 @@ func registerRoutes(m *web.Router) { // end "/{username}/{reponame}/settings" // user/org home, including rss feeds - m.Get("/{username}/{reponame}", ignSignIn, context.RepoAssignment, context.RepoRef(), repo.SetEditorconfigIfExists, repo.Home) + m.Get("/{username}/{reponame}", optSignIn, context.RepoAssignment, context.RepoRef(), repo.SetEditorconfigIfExists, repo.Home) // TODO: maybe it should relax the permission to allow "any access" - m.Post("/{username}/{reponame}/markup", ignSignIn, context.RepoAssignment, context.RequireRepoReaderOr(unit.TypeCode, unit.TypeIssues, unit.TypePullRequests, unit.TypeReleases, unit.TypeWiki), web.Bind(structs.MarkupOption{}), misc.Markup) + m.Post("/{username}/{reponame}/markup", optSignIn, context.RepoAssignment, context.RequireRepoReaderOr(unit.TypeCode, unit.TypeIssues, unit.TypePullRequests, unit.TypeReleases, unit.TypeWiki), web.Bind(structs.MarkupOption{}), misc.Markup) m.Group("/{username}/{reponame}", func() { m.Get("/find/*", repo.FindFiles) @@ -1161,7 +1162,7 @@ func registerRoutes(m *web.Router) { m.Combo("/compare/*", repo.MustBeNotEmpty, repo.SetEditorconfigIfExists). Get(repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.CompareDiff). Post(reqSignIn, context.RepoMustNotBeArchived(), reqRepoPullsReader, repo.MustAllowPulls, web.Bind(forms.CreateIssueForm{}), repo.SetWhitespaceBehavior, repo.CompareAndPullRequestPost) - }, ignSignIn, context.RepoAssignment, reqRepoCodeReader) + }, optSignIn, context.RepoAssignment, reqRepoCodeReader) // end "/{username}/{reponame}": find, compare, list (code related) m.Group("/{username}/{reponame}", func() { @@ -1184,7 +1185,7 @@ func registerRoutes(m *web.Router) { }) }, context.RepoRef()) m.Get("/issues/suggestions", repo.IssueSuggestions) - }, ignSignIn, context.RepoAssignment, reqRepoIssuesOrPullsReader) + }, optSignIn, context.RepoAssignment, reqRepoIssuesOrPullsReader) // end "/{username}/{reponame}": view milestone, label, issue, pull, etc m.Group("/{username}/{reponame}", func() { @@ -1194,7 +1195,7 @@ func registerRoutes(m *web.Router) { m.Get("", repo.ViewIssue) }) }) - }, ignSignIn, context.RepoAssignment, context.RequireRepoReaderOr(unit.TypeIssues, unit.TypePullRequests, unit.TypeExternalTracker)) + }, optSignIn, context.RepoAssignment, context.RequireRepoReaderOr(unit.TypeIssues, unit.TypePullRequests, unit.TypeExternalTracker)) // end "/{username}/{reponame}": issue/pull list, issue/pull view, external tracker m.Group("/{username}/{reponame}", func() { // edit issues, pulls, labels, milestones, etc @@ -1331,7 +1332,7 @@ func registerRoutes(m *web.Router) { repo.MustBeNotEmpty, context.RepoRefByType(context.RepoRefTag, context.RepoRefByTypeOptions{IgnoreNotExistErr: true})) m.Post("/tags/delete", repo.DeleteTag, reqSignIn, repo.MustBeNotEmpty, context.RepoMustNotBeArchived(), reqRepoCodeWriter, context.RepoRef()) - }, ignSignIn, context.RepoAssignment, reqRepoCodeReader) + }, optSignIn, context.RepoAssignment, reqRepoCodeReader) // end "/{username}/{reponame}": repo tags m.Group("/{username}/{reponame}", func() { // repo releases @@ -1356,12 +1357,12 @@ func registerRoutes(m *web.Router) { m.Get("/edit/*", repo.EditRelease) m.Post("/edit/*", web.Bind(forms.EditReleaseForm{}), repo.EditReleasePost) }, reqSignIn, repo.MustBeNotEmpty, context.RepoMustNotBeArchived(), reqRepoReleaseWriter, repo.CommitInfoCache) - }, ignSignIn, context.RepoAssignment, reqRepoReleaseReader) + }, optSignIn, context.RepoAssignment, reqRepoReleaseReader) // end "/{username}/{reponame}": repo releases m.Group("/{username}/{reponame}", func() { // to maintain compatibility with old attachments m.Get("/attachments/{uuid}", repo.GetAttachment) - }, ignSignIn, context.RepoAssignment) + }, optSignIn, context.RepoAssignment) // end "/{username}/{reponame}": compatibility with old attachments m.Group("/{username}/{reponame}", func() { @@ -1372,7 +1373,7 @@ func registerRoutes(m *web.Router) { if setting.Packages.Enabled { m.Get("/packages", repo.Packages) } - }, ignSignIn, context.RepoAssignment) + }, optSignIn, context.RepoAssignment) m.Group("/{username}/{reponame}/projects", func() { m.Get("", repo.Projects) @@ -1397,7 +1398,7 @@ func registerRoutes(m *web.Router) { }) }) }, reqRepoProjectsWriter, context.RepoMustNotBeArchived()) - }, ignSignIn, context.RepoAssignment, reqRepoProjectsReader, repo.MustEnableRepoProjects) + }, optSignIn, context.RepoAssignment, reqRepoProjectsReader, repo.MustEnableRepoProjects) // end "/{username}/{reponame}/projects" m.Group("/{username}/{reponame}/actions", func() { @@ -1427,7 +1428,7 @@ func registerRoutes(m *web.Router) { m.Group("/workflows/{workflow_name}", func() { m.Get("/badge.svg", actions.GetWorkflowBadge) }) - }, ignSignIn, context.RepoAssignment, reqRepoActionsReader, actions.MustEnableActions) + }, optSignIn, context.RepoAssignment, reqRepoActionsReader, actions.MustEnableActions) // end "/{username}/{reponame}/actions" m.Group("/{username}/{reponame}/wiki", func() { @@ -1440,7 +1441,7 @@ func registerRoutes(m *web.Router) { m.Get("/commit/{sha:[a-f0-9]{7,64}}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.Diff) m.Get("/commit/{sha:[a-f0-9]{7,64}}.{ext:patch|diff}", repo.RawDiff) m.Get("/raw/*", repo.WikiRaw) - }, ignSignIn, context.RepoAssignment, repo.MustEnableWiki, reqRepoWikiReader, func(ctx *context.Context) { + }, optSignIn, context.RepoAssignment, repo.MustEnableWiki, reqRepoWikiReader, func(ctx *context.Context) { ctx.Data["PageIsWiki"] = true ctx.Data["CloneButtonOriginLink"] = ctx.Repo.Repository.WikiCloneLink() }) @@ -1462,7 +1463,7 @@ func registerRoutes(m *web.Router) { m.Get("/data", repo.RecentCommitsData) }) }, - ignSignIn, context.RepoAssignment, context.RequireRepoReaderOr(unit.TypePullRequests, unit.TypeIssues, unit.TypeReleases), + optSignIn, context.RepoAssignment, context.RequireRepoReaderOr(unit.TypePullRequests, unit.TypeIssues, unit.TypeReleases), context.RepoRef(), repo.MustBeNotEmpty, ) // end "/{username}/{reponame}/activity" @@ -1493,7 +1494,7 @@ func registerRoutes(m *web.Router) { }, context.RepoMustNotBeArchived()) }) }) - }, ignSignIn, context.RepoAssignment, repo.MustAllowPulls, reqRepoPullsReader) + }, optSignIn, context.RepoAssignment, repo.MustAllowPulls, reqRepoPullsReader) // end "/{username}/{reponame}/pulls/{index}": repo pull request m.Group("/{username}/{reponame}", func() { @@ -1593,7 +1594,7 @@ func registerRoutes(m *web.Router) { m.Get("/forks", context.RepoRef(), repo.Forks) m.Get("/commit/{sha:([a-f0-9]{7,64})}.{ext:patch|diff}", repo.MustBeNotEmpty, repo.RawDiff) m.Post("/lastcommit/*", context.RepoRefByType(context.RepoRefCommit), repo.LastCommit) - }, ignSignIn, context.RepoAssignment, reqRepoCodeReader) + }, optSignIn, context.RepoAssignment, reqRepoCodeReader) // end "/{username}/{reponame}": repo code m.Group("/{username}/{reponame}", func() { @@ -1601,13 +1602,11 @@ func registerRoutes(m *web.Router) { m.Get("/watchers", repo.Watchers) m.Get("/search", reqRepoCodeReader, repo.Search) m.Post("/action/{action}", reqSignIn, repo.Action) - }, ignSignIn, context.RepoAssignment, context.RepoRef()) + }, optSignIn, context.RepoAssignment, context.RepoRef()) - common.AddOwnerRepoGitLFSRoutes(m, ignSignInAndCsrf, lfsServerEnabled) - m.Group("/{username}/{reponame}", func() { - gitHTTPRouters(m) - }) - // end "/{username}/{reponame}.git": git support + common.AddOwnerRepoGitLFSRoutes(m, optSignInIgnoreCsrf, lfsServerEnabled) // "/{username}/{reponame}/{lfs-paths}": git-lfs support + + addOwnerRepoGitHTTPRouters(m) // "/{username}/{reponame}/{git-paths}": git http support m.Group("/notifications", func() { m.Get("", user.Notifications) diff --git a/services/context/base.go b/services/context/base.go index 68619bf067800..d62709558436e 100644 --- a/services/context/base.go +++ b/services/context/base.go @@ -30,6 +30,10 @@ type contextValuePair struct { valueFn func() any } +type BaseContextKeyType struct{} + +var BaseContextKey BaseContextKeyType + type Base struct { originCtx context.Context contextValues []contextValuePair @@ -315,6 +319,7 @@ func NewBaseContext(resp http.ResponseWriter, req *http.Request) (b *Base, close Data: middleware.GetContextData(req.Context()), } 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 diff --git a/services/context/context.go b/services/context/context.go index 6c7128ef6866c..812a8c27eeb53 100644 --- a/services/context/context.go +++ b/services/context/context.go @@ -65,6 +65,9 @@ type Context struct { type TemplateContext map[string]any func init() { + web.RegisterResponseStatusProvider[*Base](func(req *http.Request) web_types.ResponseStatusProvider { + return req.Context().Value(BaseContextKey).(*Base) + }) web.RegisterResponseStatusProvider[*Context](func(req *http.Request) web_types.ResponseStatusProvider { return req.Context().Value(WebContextKey).(*Context) }) diff --git a/services/lfs/locks.go b/services/lfs/locks.go index 4254c99383fa1..1d464f4a669ac 100644 --- a/services/lfs/locks.go +++ b/services/lfs/locks.go @@ -51,7 +51,7 @@ func GetListLockHandler(ctx *context.Context) { repository, err := repo_model.GetRepositoryByOwnerAndName(ctx, rv.User, rv.Repo) if err != nil { log.Debug("Could not find repository: %s/%s - %s", rv.User, rv.Repo, err) - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ Message: "You must have pull access to list locks", }) @@ -66,7 +66,7 @@ func GetListLockHandler(ctx *context.Context) { authenticated := authenticate(ctx, repository, rv.Authorization, true, false) if !authenticated { - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ Message: "You must have pull access to list locks", }) @@ -143,7 +143,7 @@ func PostLockHandler(ctx *context.Context) { repository, err := repo_model.GetRepositoryByOwnerAndName(ctx, userName, repoName) if err != nil { log.Error("Unable to get repository: %s/%s Error: %v", userName, repoName, err) - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ Message: "You must have push access to create locks", }) @@ -158,7 +158,7 @@ func PostLockHandler(ctx *context.Context) { authenticated := authenticate(ctx, repository, authorization, true, true) if !authenticated { - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ Message: "You must have push access to create locks", }) @@ -191,7 +191,7 @@ func PostLockHandler(ctx *context.Context) { return } if git_model.IsErrLFSUnauthorizedAction(err) { - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ Message: "You must have push access to create locks : " + err.Error(), }) @@ -215,7 +215,7 @@ func VerifyLockHandler(ctx *context.Context) { repository, err := repo_model.GetRepositoryByOwnerAndName(ctx, userName, repoName) if err != nil { log.Error("Unable to get repository: %s/%s Error: %v", userName, repoName, err) - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ Message: "You must have push access to verify locks", }) @@ -230,7 +230,7 @@ func VerifyLockHandler(ctx *context.Context) { authenticated := authenticate(ctx, repository, authorization, true, true) if !authenticated { - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ Message: "You must have push access to verify locks", }) @@ -286,7 +286,7 @@ func UnLockHandler(ctx *context.Context) { repository, err := repo_model.GetRepositoryByOwnerAndName(ctx, userName, repoName) if err != nil { log.Error("Unable to get repository: %s/%s Error: %v", userName, repoName, err) - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ Message: "You must have push access to delete locks", }) @@ -301,7 +301,7 @@ func UnLockHandler(ctx *context.Context) { authenticated := authenticate(ctx, repository, authorization, true, true) if !authenticated { - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ Message: "You must have push access to delete locks", }) @@ -324,7 +324,7 @@ func UnLockHandler(ctx *context.Context) { lock, err := git_model.DeleteLFSLockByID(ctx, ctx.PathParamInt64("lid"), repository, ctx.Doer, req.Force) if err != nil { if git_model.IsErrLFSUnauthorizedAction(err) { - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ Message: "You must have push access to delete locks : " + err.Error(), }) diff --git a/services/lfs/server.go b/services/lfs/server.go index f8ef177387026..a77623fdc196c 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -21,7 +21,7 @@ import ( actions_model "code.gitea.io/gitea/models/actions" auth_model "code.gitea.io/gitea/models/auth" git_model "code.gitea.io/gitea/models/git" - "code.gitea.io/gitea/models/perm" + perm_model "code.gitea.io/gitea/models/perm" access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" @@ -77,7 +77,7 @@ func CheckAcceptMediaType(ctx *context.Context) { } } -var rangeHeaderRegexp = regexp.MustCompile(`bytes=(\d+)\-(\d*).*`) +var rangeHeaderRegexp = regexp.MustCompile(`bytes=(\d+)-(\d*).*`) // DownloadHandler gets the content from the content store func DownloadHandler(ctx *context.Context) { @@ -507,11 +507,11 @@ func writeStatusMessage(ctx *context.Context, status int, message string) { } // authenticate uses the authorization string to determine whether -// or not to proceed. This server assumes an HTTP Basic auth format. +// to proceed. This server assumes an HTTP Basic auth format. func authenticate(ctx *context.Context, repository *repo_model.Repository, authorization string, requireSigned, requireWrite bool) bool { - accessMode := perm.AccessModeRead + accessMode := perm_model.AccessModeRead if requireWrite { - accessMode = perm.AccessModeWrite + accessMode = perm_model.AccessModeWrite } if ctx.Data["IsActionsToken"] == true { @@ -526,9 +526,9 @@ func authenticate(ctx *context.Context, repository *repo_model.Repository, autho } if task.IsForkPullRequest { - return accessMode <= perm.AccessModeRead + return accessMode <= perm_model.AccessModeRead } - return accessMode <= perm.AccessModeWrite + return accessMode <= perm_model.AccessModeWrite } // ctx.IsSigned is unnecessary here, this will be checked in perm.CanAccess @@ -553,7 +553,7 @@ func authenticate(ctx *context.Context, repository *repo_model.Repository, autho return true } -func handleLFSToken(ctx stdCtx.Context, tokenSHA string, target *repo_model.Repository, mode perm.AccessMode) (*user_model.User, error) { +func handleLFSToken(ctx stdCtx.Context, tokenSHA string, target *repo_model.Repository, mode perm_model.AccessMode) (*user_model.User, error) { if !strings.Contains(tokenSHA, ".") { return nil, nil } @@ -576,7 +576,7 @@ func handleLFSToken(ctx stdCtx.Context, tokenSHA string, target *repo_model.Repo return nil, fmt.Errorf("invalid token claim") } - if mode == perm.AccessModeWrite && claims.Op != "upload" { + if mode == perm_model.AccessModeWrite && claims.Op != "upload" { return nil, fmt.Errorf("invalid token claim") } @@ -588,7 +588,7 @@ func handleLFSToken(ctx stdCtx.Context, tokenSHA string, target *repo_model.Repo return u, nil } -func parseToken(ctx stdCtx.Context, authorization string, target *repo_model.Repository, mode perm.AccessMode) (*user_model.User, error) { +func parseToken(ctx stdCtx.Context, authorization string, target *repo_model.Repository, mode perm_model.AccessMode) (*user_model.User, error) { if authorization == "" { return nil, fmt.Errorf("no token") } @@ -608,6 +608,6 @@ func parseToken(ctx stdCtx.Context, authorization string, target *repo_model.Rep } func requireAuth(ctx *context.Context) { - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) writeStatus(ctx, http.StatusUnauthorized) } diff --git a/tests/integration/git_lfs_ssh_test.go b/tests/integration/git_lfs_ssh_test.go index 33c2fba6208c4..9cb7fd089bdf9 100644 --- a/tests/integration/git_lfs_ssh_test.go +++ b/tests/integration/git_lfs_ssh_test.go @@ -4,14 +4,18 @@ package integration import ( + gocontext "context" "net/url" + "slices" + "strings" "sync" "testing" auth_model "code.gitea.io/gitea/models/auth" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/web" - "code.gitea.io/gitea/routers/private" + "code.gitea.io/gitea/routers/common" "code.gitea.io/gitea/services/context" "github.com/stretchr/testify/assert" @@ -25,7 +29,7 @@ func TestGitLFSSSH(t *testing.T) { var mu sync.Mutex var routerCalls []string - web.RouteMock(private.RouterMockPointInternalLFS, func(ctx *context.PrivateContext) { + web.RouteMock(common.RouterMockPointCommonLFS, func(ctx *context.Base) { mu.Lock() routerCalls = append(routerCalls, ctx.Req.Method+" "+ctx.Req.URL.Path) mu.Unlock() @@ -42,20 +46,18 @@ func TestGitLFSSSH(t *testing.T) { setting.LFS.AllowPureSSH = true require.NoError(t, cfg.Save()) - // do LFS SSH transfer? + _, _, cmdErr := git.NewCommand(gocontext.Background(), "config", "lfs.sshtransfer", "always").RunStdString(&git.RunOpts{Dir: dstPath}) + assert.NoError(t, cmdErr) lfsCommitAndPushTest(t, dstPath, 10) }) - // FIXME: Here we only see the following calls, but actually there should be calls to "PUT"? - // 0 = {string} "GET /api/internal/repo/user2/repo1.git/info/lfs/locks" - // 1 = {string} "POST /api/internal/repo/user2/repo1.git/info/lfs/objects/batch" - // 2 = {string} "GET /api/internal/repo/user2/repo1.git/info/lfs/locks" - // 3 = {string} "POST /api/internal/repo/user2/repo1.git/info/lfs/locks" - // 4 = {string} "GET /api/internal/repo/user2/repo1.git/info/lfs/locks" - // 5 = {string} "GET /api/internal/repo/user2/repo1.git/info/lfs/locks" - // 6 = {string} "GET /api/internal/repo/user2/repo1.git/info/lfs/locks" - // 7 = {string} "POST /api/internal/repo/user2/repo1.git/info/lfs/locks/24/unlock" - assert.NotEmpty(t, routerCalls) - // assert.Contains(t, routerCalls, "PUT /api/internal/repo/user2/repo1.git/info/lfs/objects/....") + countBatch := slices.ContainsFunc(routerCalls, func(s string) bool { + return strings.Contains(s, "POST /api/internal/repo/user2/repo1.git/info/lfs/objects/batch") + }) + countUpload := slices.ContainsFunc(routerCalls, func(s string) bool { + return strings.Contains(s, "PUT /user2/repo1.git/info/lfs/objects/") + }) + assert.NotZero(t, countBatch) + assert.NotZero(t, countUpload) }) } From ad223000d42782d1cd5a2ef9e8cb3a51f76739f9 Mon Sep 17 00:00:00 2001 From: BoYanZh Date: Wed, 13 Nov 2024 13:17:54 -0500 Subject: [PATCH 2/5] Perf: add extra index to notification table (#32395) Index SQL: `CREATE INDEX u_s_uu ON notification(user_id, status, updated_unix);` The naming follows `action.go` in the same dir. I am unsure which version I should add SQL to the migration folder, so I have not modified it. Fix #32390 --- models/activities/notification.go | 58 +++++++++++++++++++---- models/migrations/migrations.go | 1 + models/migrations/v1_23/v309.go | 77 +++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 9 deletions(-) create mode 100644 models/migrations/v1_23/v309.go diff --git a/models/activities/notification.go b/models/activities/notification.go index b888adeb60fe5..6dde26fd53e5b 100644 --- a/models/activities/notification.go +++ b/models/activities/notification.go @@ -18,6 +18,7 @@ import ( "code.gitea.io/gitea/modules/timeutil" "xorm.io/builder" + "xorm.io/xorm/schemas" ) type ( @@ -50,25 +51,64 @@ const ( // Notification represents a notification type Notification struct { ID int64 `xorm:"pk autoincr"` - UserID int64 `xorm:"INDEX NOT NULL"` - RepoID int64 `xorm:"INDEX NOT NULL"` + UserID int64 `xorm:"NOT NULL"` + RepoID int64 `xorm:"NOT NULL"` - Status NotificationStatus `xorm:"SMALLINT INDEX NOT NULL"` - Source NotificationSource `xorm:"SMALLINT INDEX NOT NULL"` + Status NotificationStatus `xorm:"SMALLINT NOT NULL"` + Source NotificationSource `xorm:"SMALLINT NOT NULL"` - IssueID int64 `xorm:"INDEX NOT NULL"` - CommitID string `xorm:"INDEX"` + IssueID int64 `xorm:"NOT NULL"` + CommitID string CommentID int64 - UpdatedBy int64 `xorm:"INDEX NOT NULL"` + UpdatedBy int64 `xorm:"NOT NULL"` Issue *issues_model.Issue `xorm:"-"` Repository *repo_model.Repository `xorm:"-"` Comment *issues_model.Comment `xorm:"-"` User *user_model.User `xorm:"-"` - CreatedUnix timeutil.TimeStamp `xorm:"created INDEX NOT NULL"` - UpdatedUnix timeutil.TimeStamp `xorm:"updated INDEX NOT NULL"` + CreatedUnix timeutil.TimeStamp `xorm:"created NOT NULL"` + UpdatedUnix timeutil.TimeStamp `xorm:"updated NOT NULL"` +} + +// TableIndices implements xorm's TableIndices interface +func (n *Notification) TableIndices() []*schemas.Index { + indices := make([]*schemas.Index, 0, 8) + usuuIndex := schemas.NewIndex("u_s_uu", schemas.IndexType) + usuuIndex.AddColumn("user_id", "status", "updated_unix") + indices = append(indices, usuuIndex) + + // Add the individual indices that were previously defined in struct tags + userIDIndex := schemas.NewIndex("idx_notification_user_id", schemas.IndexType) + userIDIndex.AddColumn("user_id") + indices = append(indices, userIDIndex) + + repoIDIndex := schemas.NewIndex("idx_notification_repo_id", schemas.IndexType) + repoIDIndex.AddColumn("repo_id") + indices = append(indices, repoIDIndex) + + statusIndex := schemas.NewIndex("idx_notification_status", schemas.IndexType) + statusIndex.AddColumn("status") + indices = append(indices, statusIndex) + + sourceIndex := schemas.NewIndex("idx_notification_source", schemas.IndexType) + sourceIndex.AddColumn("source") + indices = append(indices, sourceIndex) + + issueIDIndex := schemas.NewIndex("idx_notification_issue_id", schemas.IndexType) + issueIDIndex.AddColumn("issue_id") + indices = append(indices, issueIDIndex) + + commitIDIndex := schemas.NewIndex("idx_notification_commit_id", schemas.IndexType) + commitIDIndex.AddColumn("commit_id") + indices = append(indices, commitIDIndex) + + updatedByIndex := schemas.NewIndex("idx_notification_updated_by", schemas.IndexType) + updatedByIndex.AddColumn("updated_by") + indices = append(indices, updatedByIndex) + + return indices } func init() { diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 0333e7e204aad..e0361af86ba8e 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -366,6 +366,7 @@ func prepareMigrationTasks() []*migration { newMigration(306, "Add BlockAdminMergeOverride to ProtectedBranch", v1_23.AddBlockAdminMergeOverrideBranchProtection), newMigration(307, "Fix milestone deadline_unix when there is no due date", v1_23.FixMilestoneNoDueDate), newMigration(308, "Add index(user_id, is_deleted) for action table", v1_23.AddNewIndexForUserDashboard), + newMigration(309, "Improve Notification table indices", v1_23.ImproveNotificationTableIndices), } return preparedMigrations } diff --git a/models/migrations/v1_23/v309.go b/models/migrations/v1_23/v309.go new file mode 100644 index 0000000000000..5b39398443ff1 --- /dev/null +++ b/models/migrations/v1_23/v309.go @@ -0,0 +1,77 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_23 //nolint + +import ( + "code.gitea.io/gitea/modules/timeutil" + + "xorm.io/xorm" + "xorm.io/xorm/schemas" +) + +type improveNotificationTableIndicesAction struct { + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"NOT NULL"` + RepoID int64 `xorm:"NOT NULL"` + + Status uint8 `xorm:"SMALLINT NOT NULL"` + Source uint8 `xorm:"SMALLINT NOT NULL"` + + IssueID int64 `xorm:"NOT NULL"` + CommitID string + CommentID int64 + + UpdatedBy int64 `xorm:"NOT NULL"` + + CreatedUnix timeutil.TimeStamp `xorm:"created NOT NULL"` + UpdatedUnix timeutil.TimeStamp `xorm:"updated NOT NULL"` +} + +// TableName sets the name of this table +func (*improveNotificationTableIndicesAction) TableName() string { + return "notification" +} + +// TableIndices implements xorm's TableIndices interface +func (*improveNotificationTableIndicesAction) TableIndices() []*schemas.Index { + indices := make([]*schemas.Index, 0, 8) + usuuIndex := schemas.NewIndex("u_s_uu", schemas.IndexType) + usuuIndex.AddColumn("user_id", "status", "updated_unix") + indices = append(indices, usuuIndex) + + // Add the individual indices that were previously defined in struct tags + userIDIndex := schemas.NewIndex("idx_notification_user_id", schemas.IndexType) + userIDIndex.AddColumn("user_id") + indices = append(indices, userIDIndex) + + repoIDIndex := schemas.NewIndex("idx_notification_repo_id", schemas.IndexType) + repoIDIndex.AddColumn("repo_id") + indices = append(indices, repoIDIndex) + + statusIndex := schemas.NewIndex("idx_notification_status", schemas.IndexType) + statusIndex.AddColumn("status") + indices = append(indices, statusIndex) + + sourceIndex := schemas.NewIndex("idx_notification_source", schemas.IndexType) + sourceIndex.AddColumn("source") + indices = append(indices, sourceIndex) + + issueIDIndex := schemas.NewIndex("idx_notification_issue_id", schemas.IndexType) + issueIDIndex.AddColumn("issue_id") + indices = append(indices, issueIDIndex) + + commitIDIndex := schemas.NewIndex("idx_notification_commit_id", schemas.IndexType) + commitIDIndex.AddColumn("commit_id") + indices = append(indices, commitIDIndex) + + updatedByIndex := schemas.NewIndex("idx_notification_updated_by", schemas.IndexType) + updatedByIndex.AddColumn("updated_by") + indices = append(indices, updatedByIndex) + + return indices +} + +func ImproveNotificationTableIndices(x *xorm.Engine) error { + return x.Sync(&improveNotificationTableIndicesAction{}) +} From 9880c1372e6db6dd9cf28ae240af6bd52164459d Mon Sep 17 00:00:00 2001 From: silverwind Date: Wed, 13 Nov 2024 22:39:55 +0100 Subject: [PATCH 3/5] Bump CI,Flake and Snap to Node 22 (#32487) Node 22 is LTS since 2024-10-29. Updated it everywhere. --------- Co-authored-by: techknowlogick --- .github/workflows/pull-compliance.yml | 8 ++++---- .github/workflows/pull-e2e-tests.yml | 2 +- .github/workflows/release-nightly.yml | 2 +- .github/workflows/release-tag-rc.yml | 2 +- .github/workflows/release-tag-version.yml | 2 +- flake.lock | 12 ++++++------ flake.nix | 2 +- snap/snapcraft.yaml | 2 +- 8 files changed, 16 insertions(+), 16 deletions(-) diff --git a/.github/workflows/pull-compliance.yml b/.github/workflows/pull-compliance.yml index 429829dfe0fc9..7e988e04492ea 100644 --- a/.github/workflows/pull-compliance.yml +++ b/.github/workflows/pull-compliance.yml @@ -37,7 +37,7 @@ jobs: python-version: "3.12" - uses: actions/setup-node@v4 with: - node-version: 20 + node-version: 22 cache: npm cache-dependency-path: package-lock.json - run: pip install poetry @@ -66,7 +66,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-node@v4 with: - node-version: 20 + node-version: 22 cache: npm cache-dependency-path: package-lock.json - run: make deps-frontend @@ -137,7 +137,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-node@v4 with: - node-version: 20 + node-version: 22 cache: npm cache-dependency-path: package-lock.json - run: make deps-frontend @@ -186,7 +186,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-node@v4 with: - node-version: 20 + node-version: 22 cache: npm cache-dependency-path: package-lock.json - run: make deps-frontend diff --git a/.github/workflows/pull-e2e-tests.yml b/.github/workflows/pull-e2e-tests.yml index 35ac7598f6b75..b84c69e4a0f5d 100644 --- a/.github/workflows/pull-e2e-tests.yml +++ b/.github/workflows/pull-e2e-tests.yml @@ -23,7 +23,7 @@ jobs: check-latest: true - uses: actions/setup-node@v4 with: - node-version: 20 + node-version: 22 cache: npm cache-dependency-path: package-lock.json - run: make deps-frontend frontend deps-backend diff --git a/.github/workflows/release-nightly.yml b/.github/workflows/release-nightly.yml index 10fe94b2965a7..6e1b6e0758417 100644 --- a/.github/workflows/release-nightly.yml +++ b/.github/workflows/release-nightly.yml @@ -22,7 +22,7 @@ jobs: check-latest: true - uses: actions/setup-node@v4 with: - node-version: 20 + node-version: 22 cache: npm cache-dependency-path: package-lock.json - run: make deps-frontend deps-backend diff --git a/.github/workflows/release-tag-rc.yml b/.github/workflows/release-tag-rc.yml index 55908d3657ac8..41037df29cbc0 100644 --- a/.github/workflows/release-tag-rc.yml +++ b/.github/workflows/release-tag-rc.yml @@ -23,7 +23,7 @@ jobs: check-latest: true - uses: actions/setup-node@v4 with: - node-version: 20 + node-version: 22 cache: npm cache-dependency-path: package-lock.json - run: make deps-frontend deps-backend diff --git a/.github/workflows/release-tag-version.yml b/.github/workflows/release-tag-version.yml index edf7ea1270df7..a23e6982000ae 100644 --- a/.github/workflows/release-tag-version.yml +++ b/.github/workflows/release-tag-version.yml @@ -25,7 +25,7 @@ jobs: check-latest: true - uses: actions/setup-node@v4 with: - node-version: 20 + node-version: 22 cache: npm cache-dependency-path: package-lock.json - run: make deps-frontend deps-backend diff --git a/flake.lock b/flake.lock index 9eadad2b94452..1890b82dcfa7b 100644 --- a/flake.lock +++ b/flake.lock @@ -5,11 +5,11 @@ "systems": "systems" }, "locked": { - "lastModified": 1710146030, - "narHash": "sha256-SZ5L6eA7HJ/nmkzGG7/ISclqe6oZdOZTNoesiInkXPQ=", + "lastModified": 1726560853, + "narHash": "sha256-X6rJYSESBVr3hBoH0WbKE5KvhPU5bloyZ2L4K60/fPQ=", "owner": "numtide", "repo": "flake-utils", - "rev": "b1d9ab70662946ef0850d488da1c9019f3a9752a", + "rev": "c1dfcf08411b08f6b8615f7d8971a2bfa81d5e8a", "type": "github" }, "original": { @@ -20,11 +20,11 @@ }, "nixpkgs": { "locked": { - "lastModified": 1720542800, - "narHash": "sha256-ZgnNHuKV6h2+fQ5LuqnUaqZey1Lqqt5dTUAiAnqH0QQ=", + "lastModified": 1731139594, + "narHash": "sha256-IigrKK3vYRpUu+HEjPL/phrfh7Ox881er1UEsZvw9Q4=", "owner": "nixos", "repo": "nixpkgs", - "rev": "feb2849fdeb70028c70d73b848214b00d324a497", + "rev": "76612b17c0ce71689921ca12d9ffdc9c23ce40b2", "type": "github" }, "original": { diff --git a/flake.nix b/flake.nix index e2f273e3418d9..2c9d74c137ecb 100644 --- a/flake.nix +++ b/flake.nix @@ -22,7 +22,7 @@ gzip # frontend - nodejs_20 + nodejs_22 # linting python312 diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index a0a85d85af0c9..c4c31968e0e53 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -44,7 +44,7 @@ parts: source: . stage-packages: [ git, sqlite3, openssh-client ] build-packages: [ git, libpam0g-dev, libsqlite3-dev, build-essential] - build-snaps: [ go/1.23/stable, node/20/stable ] + build-snaps: [ go/1.23/stable, node/22/stable ] build-environment: - LDFLAGS: "" override-pull: | From 985e2a8af3d6468bac3ab178148c38bdbd8414f5 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 14 Nov 2024 12:17:58 +0800 Subject: [PATCH 4/5] Fix nil panic if repo doesn't exist (#32501) fix #32496 --- models/activities/action.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/models/activities/action.go b/models/activities/action.go index c83dba9d4697b..43da557fff398 100644 --- a/models/activities/action.go +++ b/models/activities/action.go @@ -251,6 +251,9 @@ func (a *Action) GetActDisplayNameTitle(ctx context.Context) string { // GetRepoUserName returns the name of the action repository owner. func (a *Action) GetRepoUserName(ctx context.Context) string { a.loadRepo(ctx) + if a.Repo == nil { + return "(non-existing-repo)" + } return a.Repo.OwnerName } @@ -263,6 +266,9 @@ func (a *Action) ShortRepoUserName(ctx context.Context) string { // GetRepoName returns the name of the action repository. func (a *Action) GetRepoName(ctx context.Context) string { a.loadRepo(ctx) + if a.Repo == nil { + return "(non-existing-repo)" + } return a.Repo.Name } From 3f9c3e7bc394c115ccc4818d6505f1f68de350d2 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 14 Nov 2024 13:02:11 +0800 Subject: [PATCH 5/5] Refactor render system (#32492) There were too many patches to the Render system, it's really difficult to make further improvements. This PR clears the legacy problems and fix TODOs. 1. Rename `RenderContext.Type` to `RenderContext.MarkupType` to clarify its usage. 2. Use `ContentMode` to replace `meta["mode"]` and `IsWiki`, to clarify the rendering behaviors. 3. Use "wiki" mode instead of "mode=gfm + wiki=true" 4. Merge `renderByType` and `renderByFile` 5. Add more comments ---- The problem of "mode=document": in many cases it is not set, so many non-comment places use comment's hard line break incorrectly --- models/repo/repo.go | 2 - modules/markup/console/console.go | 23 +--- modules/markup/html.go | 9 +- modules/markup/html_codepreview_test.go | 5 +- modules/markup/html_internal_test.go | 22 ++-- modules/markup/html_issue.go | 5 +- modules/markup/html_link.go | 4 +- modules/markup/html_node.go | 4 +- modules/markup/html_test.go | 52 ++++----- modules/markup/markdown/goldmark.go | 7 +- modules/markup/markdown/markdown.go | 4 +- modules/markup/markdown/markdown_test.go | 40 ++++--- modules/markup/markdown/transform_image.go | 2 +- modules/markup/orgmode/orgmode.go | 5 +- modules/markup/orgmode/orgmode_test.go | 3 +- modules/markup/render.go | 108 ++++++++++-------- modules/structs/miscellaneous.go | 10 +- modules/templates/util_render.go | 10 +- modules/templates/util_render_test.go | 15 ++- routers/api/v1/misc/markup.go | 12 +- routers/api/v1/misc/markup_test.go | 13 ++- routers/common/markup.go | 70 +++++------- routers/web/feed/convert.go | 1 - routers/web/misc/markup.go | 4 +- routers/web/repo/view.go | 12 +- routers/web/repo/wiki.go | 6 +- routers/web/user/profile.go | 1 - services/context/org.go | 3 +- templates/swagger/v1_json.tmpl | 8 +- tests/integration/markup_external_test.go | 27 ++++- .../js/features/comp/ComboMarkdownEditor.ts | 3 - web_src/js/features/repo-wiki.ts | 4 +- 32 files changed, 237 insertions(+), 257 deletions(-) diff --git a/models/repo/repo.go b/models/repo/repo.go index 68f8e16a21d58..4776ff0b9ca25 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -479,7 +479,6 @@ func (repo *Repository) ComposeMetas(ctx context.Context) map[string]string { metas := map[string]string{ "user": repo.OwnerName, "repo": repo.Name, - "mode": "comment", } unit, err := repo.GetUnit(ctx, unit.TypeExternalTracker) @@ -521,7 +520,6 @@ func (repo *Repository) ComposeDocumentMetas(ctx context.Context) map[string]str for k, v := range repo.ComposeMetas(ctx) { metas[k] = v } - metas["mode"] = "document" repo.DocumentRenderingMetas = metas } return repo.DocumentRenderingMetas diff --git a/modules/markup/console/console.go b/modules/markup/console/console.go index 01653565fe297..d991527b80f59 100644 --- a/modules/markup/console/console.go +++ b/modules/markup/console/console.go @@ -8,7 +8,6 @@ import ( "io" "path/filepath" "regexp" - "strings" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/setting" @@ -17,9 +16,6 @@ import ( "github.com/go-enry/go-enry/v2" ) -// MarkupName describes markup's name -var MarkupName = "console" - func init() { markup.RegisterRenderer(Renderer{}) } @@ -29,7 +25,7 @@ type Renderer struct{} // Name implements markup.Renderer func (Renderer) Name() string { - return MarkupName + return "console" } // Extensions implements markup.Renderer @@ -67,20 +63,3 @@ func (Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.Wri _, err = output.Write(buf) return err } - -// Render renders terminal colors to HTML with all specific handling stuff. -func Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error { - if ctx.Type == "" { - ctx.Type = MarkupName - } - return markup.Render(ctx, input, output) -} - -// RenderString renders terminal colors in string to HTML with all specific handling stuff and return string -func RenderString(ctx *markup.RenderContext, content string) (string, error) { - var buf strings.Builder - if err := Render(ctx, strings.NewReader(content), &buf); err != nil { - return "", err - } - return buf.String(), nil -} diff --git a/modules/markup/html.go b/modules/markup/html.go index e2eefefc4bafd..54c65c95d2723 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -442,12 +442,11 @@ func createLink(href, content, class string) *html.Node { a := &html.Node{ Type: html.ElementNode, Data: atom.A.String(), - Attr: []html.Attribute{ - {Key: "href", Val: href}, - {Key: "data-markdown-generated-content"}, - }, + Attr: []html.Attribute{{Key: "href", Val: href}}, + } + if !RenderBehaviorForTesting.DisableInternalAttributes { + a.Attr = append(a.Attr, html.Attribute{Key: "data-markdown-generated-content"}) } - if class != "" { a.Attr = append(a.Attr, html.Attribute{Key: "class", Val: class}) } diff --git a/modules/markup/html_codepreview_test.go b/modules/markup/html_codepreview_test.go index a90de278f57ff..5054627dde6c1 100644 --- a/modules/markup/html_codepreview_test.go +++ b/modules/markup/html_codepreview_test.go @@ -11,6 +11,7 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/markup" + "code.gitea.io/gitea/modules/markup/markdown" "github.com/stretchr/testify/assert" ) @@ -23,8 +24,8 @@ func TestRenderCodePreview(t *testing.T) { }) test := func(input, expected string) { buffer, err := markup.RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - Type: "markdown", + Ctx: git.DefaultContext, + MarkupType: markdown.MarkupName, }, input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) diff --git a/modules/markup/html_internal_test.go b/modules/markup/html_internal_test.go index 8f516751b08b8..2fb657f56b6ff 100644 --- a/modules/markup/html_internal_test.go +++ b/modules/markup/html_internal_test.go @@ -11,6 +11,7 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" + testModule "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" @@ -123,8 +124,9 @@ func TestRender_IssueIndexPattern2(t *testing.T) { } expectedNil := fmt.Sprintf(expectedFmt, links...) testRenderIssueIndexPattern(t, s, expectedNil, &RenderContext{ - Ctx: git.DefaultContext, - Metas: localMetas, + Ctx: git.DefaultContext, + Metas: localMetas, + ContentMode: RenderContentAsComment, }) class := "ref-issue" @@ -137,8 +139,9 @@ func TestRender_IssueIndexPattern2(t *testing.T) { } expectedNum := fmt.Sprintf(expectedFmt, links...) testRenderIssueIndexPattern(t, s, expectedNum, &RenderContext{ - Ctx: git.DefaultContext, - Metas: numericMetas, + Ctx: git.DefaultContext, + Metas: numericMetas, + ContentMode: RenderContentAsComment, }) } @@ -266,7 +269,6 @@ func TestRender_IssueIndexPattern_Document(t *testing.T) { "user": "someUser", "repo": "someRepo", "style": IssueNameStyleNumeric, - "mode": "document", } testRenderIssueIndexPattern(t, "#1", "#1", &RenderContext{ @@ -316,8 +318,8 @@ func TestRender_AutoLink(t *testing.T) { Links: Links{ Base: TestRepoURL, }, - Metas: localMetas, - IsWiki: true, + Metas: localMetas, + ContentMode: RenderContentAsWiki, }, strings.NewReader(input), &buffer) assert.Equal(t, err, nil) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer.String())) @@ -340,7 +342,7 @@ func TestRender_AutoLink(t *testing.T) { func TestRender_FullIssueURLs(t *testing.T) { setting.AppURL = TestAppURL - + defer testModule.MockVariableValue(&RenderBehaviorForTesting.DisableInternalAttributes, true)() test := func(input, expected string) { var result strings.Builder err := postProcess(&RenderContext{ @@ -351,9 +353,7 @@ func TestRender_FullIssueURLs(t *testing.T) { Metas: localMetas, }, []processor{fullIssuePatternProcessor}, strings.NewReader(input), &result) assert.NoError(t, err) - actual := result.String() - actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "") - assert.Equal(t, expected, actual) + assert.Equal(t, expected, result.String()) } test("Here is a link https://git.osgeo.org/gogs/postgis/postgis/pulls/6", "Here is a link https://git.osgeo.org/gogs/postgis/postgis/pulls/6") diff --git a/modules/markup/html_issue.go b/modules/markup/html_issue.go index b6d4ed6a8e2a3..fa630656cef41 100644 --- a/modules/markup/html_issue.go +++ b/modules/markup/html_issue.go @@ -67,9 +67,8 @@ func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) { return } - // FIXME: the use of "mode" is quite dirty and hacky, for example: what is a "document"? how should it be rendered? - // The "mode" approach should be refactored to some other more clear&reliable way. - crossLinkOnly := ctx.Metas["mode"] == "document" && !ctx.IsWiki + // crossLinkOnly if not comment and not wiki + crossLinkOnly := ctx.ContentMode != RenderContentAsTitle && ctx.ContentMode != RenderContentAsComment && ctx.ContentMode != RenderContentAsWiki var ( found bool diff --git a/modules/markup/html_link.go b/modules/markup/html_link.go index 9350634568317..30564da548a66 100644 --- a/modules/markup/html_link.go +++ b/modules/markup/html_link.go @@ -20,7 +20,7 @@ func ResolveLink(ctx *RenderContext, link, userContentAnchorPrefix string) (resu isAnchorFragment := link != "" && link[0] == '#' if !isAnchorFragment && !IsFullURLString(link) { linkBase := ctx.Links.Base - if ctx.IsWiki { + if ctx.ContentMode == RenderContentAsWiki { // no need to check if the link should be resolved as a wiki link or a wiki raw link // just use wiki link here and it will be redirected to a wiki raw link if necessary linkBase = ctx.Links.WikiLink() @@ -147,7 +147,7 @@ func shortLinkProcessor(ctx *RenderContext, node *html.Node) { } if image { if !absoluteLink { - link = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsWiki), link) + link = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.ContentMode == RenderContentAsWiki), link) } title := props["title"] if title == "" { diff --git a/modules/markup/html_node.go b/modules/markup/html_node.go index 6d784975b9849..c499854053fa2 100644 --- a/modules/markup/html_node.go +++ b/modules/markup/html_node.go @@ -17,7 +17,7 @@ func visitNodeImg(ctx *RenderContext, img *html.Node) (next *html.Node) { } if IsNonEmptyRelativePath(attr.Val) { - attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsWiki), attr.Val) + attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.ContentMode == RenderContentAsWiki), attr.Val) // By default, the "" tag should also be clickable, // because frontend use `` to paste the re-scaled image into the markdown, @@ -53,7 +53,7 @@ func visitNodeVideo(ctx *RenderContext, node *html.Node) (next *html.Node) { continue } if IsNonEmptyRelativePath(attr.Val) { - attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsWiki), attr.Val) + attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.ContentMode == RenderContentAsWiki), attr.Val) } attr.Val = camoHandleLink(attr.Val) node.Attr[i] = attr diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go index 82aded4407c2a..262d0fc4dd5d1 100644 --- a/modules/markup/html_test.go +++ b/modules/markup/html_test.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/markup/markdown" "code.gitea.io/gitea/modules/setting" + testModule "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" @@ -104,7 +105,7 @@ func TestRender_Commits(t *testing.T) { func TestRender_CrossReferences(t *testing.T) { setting.AppURL = markup.TestAppURL - + defer testModule.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)() test := func(input, expected string) { buffer, err := markup.RenderString(&markup.RenderContext{ Ctx: git.DefaultContext, @@ -116,9 +117,7 @@ func TestRender_CrossReferences(t *testing.T) { Metas: localMetas, }, input) assert.NoError(t, err) - actual := strings.TrimSpace(buffer) - actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "") - assert.Equal(t, strings.TrimSpace(expected), actual) + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) } test( @@ -148,7 +147,7 @@ func TestRender_CrossReferences(t *testing.T) { func TestRender_links(t *testing.T) { setting.AppURL = markup.TestAppURL - + defer testModule.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)() test := func(input, expected string) { buffer, err := markup.RenderString(&markup.RenderContext{ Ctx: git.DefaultContext, @@ -158,9 +157,7 @@ func TestRender_links(t *testing.T) { }, }, input) assert.NoError(t, err) - actual := strings.TrimSpace(buffer) - actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "") - assert.Equal(t, strings.TrimSpace(expected), actual) + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) } oldCustomURLSchemes := setting.Markdown.CustomURLSchemes @@ -261,7 +258,7 @@ func TestRender_links(t *testing.T) { func TestRender_email(t *testing.T) { setting.AppURL = markup.TestAppURL - + defer testModule.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)() test := func(input, expected string) { res, err := markup.RenderString(&markup.RenderContext{ Ctx: git.DefaultContext, @@ -271,9 +268,7 @@ func TestRender_email(t *testing.T) { }, }, input) assert.NoError(t, err) - actual := strings.TrimSpace(res) - actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "") - assert.Equal(t, strings.TrimSpace(expected), actual) + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(res)) } // Text that should be turned into email link @@ -302,10 +297,10 @@ func TestRender_email(t *testing.T) { j.doe@example.com; j.doe@example.com? j.doe@example.com!`, - `

j.doe@example.com,
-j.doe@example.com.
-j.doe@example.com;
-j.doe@example.com?
+ `

j.doe@example.com, +j.doe@example.com. +j.doe@example.com; +j.doe@example.com? j.doe@example.com!

`) // Test that should *not* be turned into email links @@ -418,8 +413,8 @@ func TestRender_ShortLinks(t *testing.T) { Links: markup.Links{ Base: markup.TestRepoURL, }, - Metas: localMetas, - IsWiki: true, + Metas: localMetas, + ContentMode: markup.RenderContentAsWiki, }, input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(string(buffer))) @@ -531,10 +526,10 @@ func TestRender_ShortLinks(t *testing.T) { func TestRender_RelativeMedias(t *testing.T) { render := func(input string, isWiki bool, links markup.Links) string { buffer, err := markdown.RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - Links: links, - Metas: localMetas, - IsWiki: isWiki, + Ctx: git.DefaultContext, + Links: links, + Metas: localMetas, + ContentMode: util.Iif(isWiki, markup.RenderContentAsWiki, markup.RenderContentAsComment), }, input) assert.NoError(t, err) return strings.TrimSpace(string(buffer)) @@ -604,12 +599,7 @@ func Test_ParseClusterFuzz(t *testing.T) { func TestPostProcess_RenderDocument(t *testing.T) { setting.AppURL = markup.TestAppURL setting.StaticURLPrefix = markup.TestAppURL // can't run standalone - - localMetas := map[string]string{ - "user": "go-gitea", - "repo": "gitea", - "mode": "document", - } + defer testModule.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)() test := func(input, expected string) { var res strings.Builder @@ -619,12 +609,10 @@ func TestPostProcess_RenderDocument(t *testing.T) { AbsolutePrefix: true, Base: "https://example.com", }, - Metas: localMetas, + Metas: map[string]string{"user": "go-gitea", "repo": "gitea"}, }, strings.NewReader(input), &res) assert.NoError(t, err) - actual := strings.TrimSpace(res.String()) - actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "") - assert.Equal(t, strings.TrimSpace(expected), actual) + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(res.String())) } // Issue index shouldn't be post processing in a document. diff --git a/modules/markup/markdown/goldmark.go b/modules/markup/markdown/goldmark.go index 0cd9dc5f30c61..c8488cfb50c37 100644 --- a/modules/markup/markdown/goldmark.go +++ b/modules/markup/markdown/goldmark.go @@ -72,7 +72,12 @@ func (g *ASTTransformer) Transform(node *ast.Document, reader text.Reader, pc pa g.transformList(ctx, v, rc) case *ast.Text: if v.SoftLineBreak() && !v.HardLineBreak() { - if ctx.Metas["mode"] != "document" { + // TODO: this was a quite unclear part, old code: `if metas["mode"] != "document" { use comment link break setting }` + // many places render non-comment contents with no mode=document, then these contents also use comment's hard line break setting + // especially in many tests. + if markup.RenderBehaviorForTesting.ForceHardLineBreak { + v.SetHardLineBreak(true) + } else if ctx.ContentMode == markup.RenderContentAsComment { v.SetHardLineBreak(setting.Markdown.EnableHardLineBreakInComments) } else { v.SetHardLineBreak(setting.Markdown.EnableHardLineBreakInDocuments) diff --git a/modules/markup/markdown/markdown.go b/modules/markup/markdown/markdown.go index db4e5706f6dda..6af0deb27bb2b 100644 --- a/modules/markup/markdown/markdown.go +++ b/modules/markup/markdown/markdown.go @@ -257,9 +257,7 @@ func (Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.Wri // Render renders Markdown to HTML with all specific handling stuff. func Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error { - if ctx.Type == "" { - ctx.Type = MarkupName - } + ctx.MarkupType = MarkupName return markup.Render(ctx, input, output) } diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go index ad38e7a088fbf..315eed2e62e7c 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/modules/markup/markdown/markdown_test.go @@ -16,6 +16,7 @@ import ( "code.gitea.io/gitea/modules/markup/markdown" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/svg" + "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" @@ -74,7 +75,7 @@ func TestRender_StandardLinks(t *testing.T) { Links: markup.Links{ Base: FullURL, }, - IsWiki: true, + ContentMode: markup.RenderContentAsWiki, }, input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(string(buffer))) @@ -296,23 +297,22 @@ This PR has been generated by [Renovate Bot](https://github.com/renovatebot/reno } func TestTotal_RenderWiki(t *testing.T) { + defer test.MockVariableValue(&markup.RenderBehaviorForTesting.ForceHardLineBreak, true)() + defer test.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)() setting.AppURL = AppURL - answers := testAnswers(util.URLJoin(FullURL, "wiki"), util.URLJoin(FullURL, "wiki", "raw")) - for i := 0; i < len(sameCases); i++ { line, err := markdown.RenderString(&markup.RenderContext{ Ctx: git.DefaultContext, Links: markup.Links{ Base: FullURL, }, - Repo: newMockRepo(testRepoOwnerName, testRepoName), - Metas: localMetas, - IsWiki: true, + Repo: newMockRepo(testRepoOwnerName, testRepoName), + Metas: localMetas, + ContentMode: markup.RenderContentAsWiki, }, sameCases[i]) assert.NoError(t, err) - actual := strings.ReplaceAll(string(line), ` data-markdown-generated-content=""`, "") - assert.Equal(t, answers[i], actual) + assert.Equal(t, answers[i], string(line)) } testCases := []string{ @@ -334,19 +334,18 @@ func TestTotal_RenderWiki(t *testing.T) { Links: markup.Links{ Base: FullURL, }, - IsWiki: true, + ContentMode: markup.RenderContentAsWiki, }, testCases[i]) assert.NoError(t, err) - actual := strings.ReplaceAll(string(line), ` data-markdown-generated-content=""`, "") - assert.EqualValues(t, testCases[i+1], actual) + assert.EqualValues(t, testCases[i+1], string(line)) } } func TestTotal_RenderString(t *testing.T) { + defer test.MockVariableValue(&markup.RenderBehaviorForTesting.ForceHardLineBreak, true)() + defer test.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)() setting.AppURL = AppURL - answers := testAnswers(util.URLJoin(FullURL, "src", "master"), util.URLJoin(FullURL, "media", "master")) - for i := 0; i < len(sameCases); i++ { line, err := markdown.RenderString(&markup.RenderContext{ Ctx: git.DefaultContext, @@ -358,8 +357,7 @@ func TestTotal_RenderString(t *testing.T) { Metas: localMetas, }, sameCases[i]) assert.NoError(t, err) - actual := strings.ReplaceAll(string(line), ` data-markdown-generated-content=""`, "") - assert.Equal(t, answers[i], actual) + assert.Equal(t, answers[i], string(line)) } testCases := []string{} @@ -428,6 +426,7 @@ func TestRenderSiblingImages_Issue12925(t *testing.T) { expected := `

image1
image2

` + defer test.MockVariableValue(&markup.RenderBehaviorForTesting.ForceHardLineBreak, true)() res, err := markdown.RenderRawString(&markup.RenderContext{Ctx: git.DefaultContext}, testcase) assert.NoError(t, err) assert.Equal(t, expected, res) @@ -996,11 +995,16 @@ space

}, } + defer test.MockVariableValue(&markup.RenderBehaviorForTesting.ForceHardLineBreak, true)() + defer test.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)() for i, c := range cases { - result, err := markdown.RenderString(&markup.RenderContext{Ctx: context.Background(), Links: c.Links, IsWiki: c.IsWiki}, input) + result, err := markdown.RenderString(&markup.RenderContext{ + Ctx: context.Background(), + Links: c.Links, + ContentMode: util.Iif(c.IsWiki, markup.RenderContentAsWiki, markup.RenderContentAsDefault), + }, input) assert.NoError(t, err, "Unexpected error in testcase: %v", i) - actual := strings.ReplaceAll(string(result), ` data-markdown-generated-content=""`, "") - assert.Equal(t, c.Expected, actual, "Unexpected result in testcase %v", i) + assert.Equal(t, c.Expected, string(result), "Unexpected result in testcase %v", i) } } diff --git a/modules/markup/markdown/transform_image.go b/modules/markup/markdown/transform_image.go index 812e24f0a2bb4..4ed4118854386 100644 --- a/modules/markup/markdown/transform_image.go +++ b/modules/markup/markdown/transform_image.go @@ -21,7 +21,7 @@ func (g *ASTTransformer) transformImage(ctx *markup.RenderContext, v *ast.Image) // Check if the destination is a real link if len(v.Destination) > 0 && !markup.IsFullURLBytes(v.Destination) { v.Destination = []byte(giteautil.URLJoin( - ctx.Links.ResolveMediaLink(ctx.IsWiki), + ctx.Links.ResolveMediaLink(ctx.ContentMode == markup.RenderContentAsWiki), strings.TrimLeft(string(v.Destination), "/"), )) } diff --git a/modules/markup/orgmode/orgmode.go b/modules/markup/orgmode/orgmode.go index 25f8d15ef4739..6b9c9631575be 100644 --- a/modules/markup/orgmode/orgmode.go +++ b/modules/markup/orgmode/orgmode.go @@ -144,14 +144,15 @@ func (r *Writer) resolveLink(kind, link string) string { } base := r.Ctx.Links.Base - if r.Ctx.IsWiki { + isWiki := r.Ctx.ContentMode == markup.RenderContentAsWiki + if isWiki { base = r.Ctx.Links.WikiLink() } else if r.Ctx.Links.HasBranchInfo() { base = r.Ctx.Links.SrcLink() } if kind == "image" || kind == "video" { - base = r.Ctx.Links.ResolveMediaLink(r.Ctx.IsWiki) + base = r.Ctx.Links.ResolveMediaLink(isWiki) } link = util.URLJoin(base, link) diff --git a/modules/markup/orgmode/orgmode_test.go b/modules/markup/orgmode/orgmode_test.go index 75b60ed81f004..b882678c7e20e 100644 --- a/modules/markup/orgmode/orgmode_test.go +++ b/modules/markup/orgmode/orgmode_test.go @@ -10,6 +10,7 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" ) @@ -26,7 +27,7 @@ func TestRender_StandardLinks(t *testing.T) { Base: "/relative-path", BranchPath: "branch/main", }, - IsWiki: isWiki, + ContentMode: util.Iif(isWiki, markup.RenderContentAsWiki, markup.RenderContentAsDefault), }, input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) diff --git a/modules/markup/render.go b/modules/markup/render.go index f2ce9229af64d..add50f438272a 100644 --- a/modules/markup/render.go +++ b/modules/markup/render.go @@ -5,11 +5,9 @@ package markup import ( "context" - "errors" "fmt" "io" "net/url" - "path/filepath" "strings" "sync" @@ -29,15 +27,44 @@ const ( RenderMetaAsTable RenderMetaMode = "table" ) +type RenderContentMode string + +const ( + RenderContentAsDefault RenderContentMode = "" // empty means "default", no special handling, maybe just a simple "document" + RenderContentAsComment RenderContentMode = "comment" + RenderContentAsTitle RenderContentMode = "title" + RenderContentAsWiki RenderContentMode = "wiki" +) + +var RenderBehaviorForTesting struct { + // Markdown line break rendering has 2 default behaviors: + // * Use hard: replace "\n" with "
" for comments, setting.Markdown.EnableHardLineBreakInComments=true + // * Keep soft: "\n" for non-comments (a.k.a. documents), setting.Markdown.EnableHardLineBreakInDocuments=false + // In history, there was a mess: + // * The behavior was controlled by `Metas["mode"] != "document", + // * However, many places render the content without setting "mode" in Metas, all these places used comment line break setting incorrectly + ForceHardLineBreak bool + + // Gitea will emit some internal attributes for various purposes, these attributes don't affect rendering. + // But there are too many hard-coded test cases, to avoid changing all of them again and again, we can disable emitting these internal attributes. + DisableInternalAttributes bool +} + // RenderContext represents a render context type RenderContext struct { - Ctx context.Context - RelativePath string // relative path from tree root of the branch - Type string - IsWiki bool - Links Links - Metas map[string]string // user, repo, mode(comment/document) - DefaultLink string + Ctx context.Context + RelativePath string // relative path from tree root of the branch + + // eg: "orgmode", "asciicast", "console" + // for file mode, it could be left as empty, and will be detected by file extension in RelativePath + MarkupType string + + // what the content will be used for: eg: for comment or for wiki? or just render a file? + ContentMode RenderContentMode + + Links Links // special link references for rendering, especially when there is a branch/tree path + Metas map[string]string // user&repo, format&style®exp (for external issue pattern), teams&org (for mention), BranchNameSubURL(for iframe&asciicast) + DefaultLink string // TODO: need to figure out GitRepo *git.Repository Repo gitrepo.Repository ShaExistCache map[string]bool @@ -77,12 +104,29 @@ func (ctx *RenderContext) AddCancel(fn func()) { // Render renders markup file to HTML with all specific handling stuff. func Render(ctx *RenderContext, input io.Reader, output io.Writer) error { - if ctx.Type != "" { - return renderByType(ctx, input, output) - } else if ctx.RelativePath != "" { - return renderFile(ctx, input, output) + if ctx.MarkupType == "" && ctx.RelativePath != "" { + ctx.MarkupType = DetectMarkupTypeByFileName(ctx.RelativePath) + if ctx.MarkupType == "" { + return util.NewInvalidArgumentErrorf("unsupported file to render: %q", ctx.RelativePath) + } + } + + renderer := renderers[ctx.MarkupType] + if renderer == nil { + return util.NewInvalidArgumentErrorf("unsupported markup type: %q", ctx.MarkupType) + } + + if ctx.RelativePath != "" { + if externalRender, ok := renderer.(ExternalRenderer); ok && externalRender.DisplayInIFrame() { + if !ctx.InStandalonePage { + // for an external "DisplayInIFrame" render, it could only output its content in a standalone page + // otherwise, a