diff --git a/.editorconfig b/.editorconfig index a547e8a585..5476eb02fb 100644 --- a/.editorconfig +++ b/.editorconfig @@ -12,6 +12,9 @@ insert_final_newline = true [{*.{go,tmpl,html},Makefile,go.mod}] indent_style = tab +[go.*] +indent_style = tab + [templates/custom/*.tmpl] insert_final_newline = false diff --git a/Makefile b/Makefile index c28e4e92ca..0594ea8866 100644 --- a/Makefile +++ b/Makefile @@ -523,7 +523,7 @@ lint-yaml: .venv .PHONY: security-check security-check: - go run $(GOVULNCHECK_PACKAGE) ./... + go run $(GOVULNCHECK_PACKAGE) -show color ./... ### # Development and testing targets diff --git a/modules/indexer/code/bleve/bleve.go b/modules/indexer/code/bleve/bleve.go index f793b5db3f..6adee68a0d 100644 --- a/modules/indexer/code/bleve/bleve.go +++ b/modules/indexer/code/bleve/bleve.go @@ -179,7 +179,8 @@ func (b *Indexer) addUpdate(ctx context.Context, batchWriter git.WriteCloserErro return err } else if !typesniffer.DetectContentType(fileContents).IsText() { // FIXME: UTF-16 files will probably fail here - return nil + // Even if the file is not recognized as a "text file", we could still put its name into the indexers to make the filename become searchable, while leave the content to empty. + fileContents = nil } if _, err = batchReader.Discard(1); err != nil { diff --git a/modules/optional/option.go b/modules/optional/option.go index af9e5ac852..ccbad259c2 100644 --- a/modules/optional/option.go +++ b/modules/optional/option.go @@ -3,6 +3,8 @@ package optional +import "strconv" + type Option[T any] []T func None[T any]() Option[T] { @@ -43,3 +45,12 @@ func (o Option[T]) ValueOrDefault(v T) T { } return v } + +// ParseBool get the corresponding optional.Option[bool] of a string using strconv.ParseBool +func ParseBool(s string) Option[bool] { + v, e := strconv.ParseBool(s) + if e != nil { + return None[bool]() + } + return Some(v) +} diff --git a/modules/optional/option_test.go b/modules/optional/option_test.go index f6d22d2431..a674caf633 100644 --- a/modules/optional/option_test.go +++ b/modules/optional/option_test.go @@ -57,3 +57,16 @@ func TestOption(t *testing.T) { assert.True(t, opt3.Has()) assert.Equal(t, int(1), opt3.Value()) } + +func Test_ParseBool(t *testing.T) { + assert.Equal(t, optional.None[bool](), optional.ParseBool("")) + assert.Equal(t, optional.None[bool](), optional.ParseBool("x")) + + assert.Equal(t, optional.Some(false), optional.ParseBool("0")) + assert.Equal(t, optional.Some(false), optional.ParseBool("f")) + assert.Equal(t, optional.Some(false), optional.ParseBool("False")) + + assert.Equal(t, optional.Some(true), optional.ParseBool("1")) + assert.Equal(t, optional.Some(true), optional.ParseBool("t")) + assert.Equal(t, optional.Some(true), optional.ParseBool("True")) +} diff --git a/modules/setting/setting.go b/modules/setting/setting.go index eb7b9e9373..80db4dfa3c 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -12,8 +12,8 @@ import ( "time" "forgejo.org/modules/log" + "forgejo.org/modules/optional" "forgejo.org/modules/user" - "forgejo.org/modules/util" ) var ForgejoVersion = "1.0.0" @@ -162,7 +162,7 @@ func loadRunModeFrom(rootCfg ConfigProvider) { // The following is a purposefully undocumented option. Please do not run Forgejo as root. It will only cause future headaches. // Please don't use root as a bandaid to "fix" something that is broken, instead the broken thing should instead be fixed properly. unsafeAllowRunAsRoot := ConfigSectionKeyBool(rootSec, "I_AM_BEING_UNSAFE_RUNNING_AS_ROOT") - unsafeAllowRunAsRoot = unsafeAllowRunAsRoot || util.OptionalBoolParse(os.Getenv("GITEA_I_AM_BEING_UNSAFE_RUNNING_AS_ROOT")).Value() + unsafeAllowRunAsRoot = unsafeAllowRunAsRoot || optional.ParseBool(os.Getenv("GITEA_I_AM_BEING_UNSAFE_RUNNING_AS_ROOT")).Value() RunMode = os.Getenv("GITEA_RUN_MODE") if RunMode == "" { RunMode = rootSec.Key("RUN_MODE").MustString("prod") diff --git a/modules/util/util.go b/modules/util/util.go index da405c9c4b..fc3fcc35f0 100644 --- a/modules/util/util.go +++ b/modules/util/util.go @@ -14,22 +14,11 @@ import ( "strconv" "strings" - "forgejo.org/modules/optional" - "golang.org/x/crypto/ssh" "golang.org/x/text/cases" "golang.org/x/text/language" ) -// OptionalBoolParse get the corresponding optional.Option[bool] of a string using strconv.ParseBool -func OptionalBoolParse(s string) optional.Option[bool] { - v, e := strconv.ParseBool(s) - if e != nil { - return optional.None[bool]() - } - return optional.Some(v) -} - // IsEmptyString checks if the provided string is empty func IsEmptyString(s string) bool { return len(strings.TrimSpace(s)) == 0 diff --git a/modules/util/util_test.go b/modules/util/util_test.go index d2fef1a035..61f98de88a 100644 --- a/modules/util/util_test.go +++ b/modules/util/util_test.go @@ -11,7 +11,6 @@ import ( "strings" "testing" - "forgejo.org/modules/optional" "forgejo.org/modules/test" "forgejo.org/modules/util" @@ -181,19 +180,6 @@ func Test_RandomBytes(t *testing.T) { assert.NotEqual(t, bytes3, bytes4) } -func TestOptionalBoolParse(t *testing.T) { - assert.Equal(t, optional.None[bool](), util.OptionalBoolParse("")) - assert.Equal(t, optional.None[bool](), util.OptionalBoolParse("x")) - - assert.Equal(t, optional.Some(false), util.OptionalBoolParse("0")) - assert.Equal(t, optional.Some(false), util.OptionalBoolParse("f")) - assert.Equal(t, optional.Some(false), util.OptionalBoolParse("False")) - - assert.Equal(t, optional.Some(true), util.OptionalBoolParse("1")) - assert.Equal(t, optional.Some(true), util.OptionalBoolParse("t")) - assert.Equal(t, optional.Some(true), util.OptionalBoolParse("True")) -} - // Test case for any function which accepts and returns a single string. type StringTest struct { in, out string diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index f53a0197cb..cefdfadc53 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -22,7 +22,6 @@ import ( "forgejo.org/modules/log" "forgejo.org/modules/optional" "forgejo.org/modules/setting" - "forgejo.org/modules/util" "forgejo.org/modules/validation" "forgejo.org/modules/web" "forgejo.org/routers/web/explore" @@ -77,11 +76,11 @@ func Users(ctx *context.Context) { PageSize: setting.UI.Admin.UserPagingNum, }, SearchByEmail: true, - IsActive: util.OptionalBoolParse(statusFilterMap["is_active"]), - IsAdmin: util.OptionalBoolParse(statusFilterMap["is_admin"]), - IsRestricted: util.OptionalBoolParse(statusFilterMap["is_restricted"]), - IsTwoFactorEnabled: util.OptionalBoolParse(statusFilterMap["is_2fa_enabled"]), - IsProhibitLogin: util.OptionalBoolParse(statusFilterMap["is_prohibit_login"]), + IsActive: optional.ParseBool(statusFilterMap["is_active"]), + IsAdmin: optional.ParseBool(statusFilterMap["is_admin"]), + IsRestricted: optional.ParseBool(statusFilterMap["is_restricted"]), + IsTwoFactorEnabled: optional.ParseBool(statusFilterMap["is_2fa_enabled"]), + IsProhibitLogin: optional.ParseBool(statusFilterMap["is_prohibit_login"]), IncludeReserved: true, // administrator needs to list all accounts include reserved, bot, remote ones Load2FAStatus: true, ExtraParamStrings: extraParamStrings, diff --git a/routers/web/repo/code_frequency.go b/routers/web/repo/code_frequency.go index 04009b4afa..44c07e617e 100644 --- a/routers/web/repo/code_frequency.go +++ b/routers/web/repo/code_frequency.go @@ -34,7 +34,7 @@ func CodeFrequencyData(ctx *context.Context) { ctx.Status(http.StatusAccepted) return } - ctx.ServerError("GetCodeFrequencyData", err) + ctx.ServerError("GetContributorStats", err) } else { ctx.JSON(http.StatusOK, contributorStats["total"].Weeks) } diff --git a/routers/web/repo/recent_commits.go b/routers/web/repo/recent_commits.go index 6154de7377..211b1b2b12 100644 --- a/routers/web/repo/recent_commits.go +++ b/routers/web/repo/recent_commits.go @@ -4,12 +4,10 @@ package repo import ( - "errors" "net/http" "forgejo.org/modules/base" "forgejo.org/services/context" - contributors_service "forgejo.org/services/repository" ) const ( @@ -26,16 +24,3 @@ func RecentCommits(ctx *context.Context) { ctx.HTML(http.StatusOK, tplRecentCommits) } - -// RecentCommitsData returns JSON of recent commits data -func RecentCommitsData(ctx *context.Context) { - if contributorStats, err := contributors_service.GetContributorStats(ctx, ctx.Cache, ctx.Repo.Repository, ctx.Repo.CommitID); err != nil { - if errors.Is(err, contributors_service.ErrAwaitGeneration) { - ctx.Status(http.StatusAccepted) - return - } - ctx.ServerError("RecentCommitsData", err) - } else { - ctx.JSON(http.StatusOK, contributorStats["total"].Weeks) - } -} diff --git a/routers/web/web.go b/routers/web/web.go index 5f0dfb64d2..58aa5d8766 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1455,7 +1455,7 @@ func registerRoutes(m *web.Route) { }, repo.MustBeNotEmpty, context.RequireRepoReaderOr(unit.TypeCode)) m.Group("/recent-commits", func() { m.Get("", repo.RecentCommits) - m.Get("/data", repo.RecentCommitsData) + m.Get("/data", repo.CodeFrequencyData) }, repo.MustBeNotEmpty, context.RequireRepoReaderOr(unit.TypeCode)) }, context.RepoRef(), context.RequireRepoReaderOr(unit.TypeCode, unit.TypePullRequests, unit.TypeIssues, unit.TypeReleases)) diff --git a/services/context/repo.go b/services/context/repo.go index 7c966e339f..369a94c870 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -361,7 +361,9 @@ func RedirectToRepo(ctx *Base, redirectRepoID int64) { if ctx.Req.URL.RawQuery != "" { redirectPath += "?" + ctx.Req.URL.RawQuery } - ctx.Redirect(path.Join(setting.AppSubURL, redirectPath), http.StatusTemporaryRedirect) + // Git client needs a 301 redirect by default to follow the new location + // It's not documentated in git documentation, but it's the behavior of git client + ctx.Redirect(path.Join(setting.AppSubURL, redirectPath), http.StatusMovedPermanently) } func repoAssignment(ctx *Context, repo *repo_model.Repository) { diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index a63cbcf40c..c46323f283 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -244,6 +244,24 @@ func pruneBrokenReferences(ctx context.Context, return pruneErr } +// checkRecoverableSyncError takes an error message from a git fetch command and returns false if it should be a fatal/blocking error +func checkRecoverableSyncError(stderrMessage string) bool { + switch { + case strings.Contains(stderrMessage, "unable to resolve reference") && strings.Contains(stderrMessage, "reference broken"): + return true + case strings.Contains(stderrMessage, "remote error") && strings.Contains(stderrMessage, "not our ref"): + return true + case strings.Contains(stderrMessage, "cannot lock ref") && strings.Contains(stderrMessage, "but expected"): + return true + case strings.Contains(stderrMessage, "cannot lock ref") && strings.Contains(stderrMessage, "unable to resolve reference"): + return true + case strings.Contains(stderrMessage, "Unable to create") && strings.Contains(stderrMessage, ".lock"): + return true + default: + return false + } +} + // runSync returns true if sync finished without error. func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bool) { repoPath := m.Repo.RepoPath() @@ -286,7 +304,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo stdoutMessage := util.SanitizeCredentialURLs(stdout) // Now check if the error is a resolve reference due to broken reference - if strings.Contains(stderr, "unable to resolve reference") && strings.Contains(stderr, "reference broken") { + if checkRecoverableSyncError(stderr) { log.Warn("SyncMirrors [repo: %-v]: failed to update mirror repository due to broken references:\nStdout: %s\nStderr: %s\nErr: %v\nAttempting Prune", m.Repo, stdoutMessage, stderrMessage, err) err = nil @@ -337,6 +355,15 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo return nil, false } + if m.LFS && setting.LFS.StartServer { + log.Trace("SyncMirrors [repo: %-v]: syncing LFS objects...", m.Repo) + endpoint := lfs.DetermineEndpoint(remoteURL.String(), m.LFSEndpoint) + lfsClient := lfs.NewClient(endpoint, nil) + if err = repo_module.StoreMissingLfsObjectsInRepository(ctx, m.Repo, gitRepo, lfsClient); err != nil { + log.Error("SyncMirrors [repo: %-v]: failed to synchronize LFS objects for repository: %v", m.Repo, err) + } + } + log.Trace("SyncMirrors [repo: %-v]: syncing branches...", m.Repo) if _, err = repo_module.SyncRepoBranchesWithRepo(ctx, m.Repo, gitRepo, 0); err != nil { log.Error("SyncMirrors [repo: %-v]: failed to synchronize branches: %v", m.Repo, err) @@ -346,15 +373,6 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo if err = repo_module.SyncReleasesWithTags(ctx, m.Repo, gitRepo); err != nil { log.Error("SyncMirrors [repo: %-v]: failed to synchronize tags to releases: %v", m.Repo, err) } - - if m.LFS && setting.LFS.StartServer { - log.Trace("SyncMirrors [repo: %-v]: syncing LFS objects...", m.Repo) - endpoint := lfs.DetermineEndpoint(remoteURL.String(), m.LFSEndpoint) - lfsClient := lfs.NewClient(endpoint, nil) - if err = repo_module.StoreMissingLfsObjectsInRepository(ctx, m.Repo, gitRepo, lfsClient); err != nil { - log.Error("SyncMirrors [repo: %-v]: failed to synchronize LFS objects for repository: %v", m.Repo, err) - } - } gitRepo.Close() log.Trace("SyncMirrors [repo: %-v]: updating size of repository", m.Repo) @@ -382,7 +400,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo stdoutMessage := util.SanitizeCredentialURLs(stdout) // Now check if the error is a resolve reference due to broken reference - if strings.Contains(stderrMessage, "unable to resolve reference") && strings.Contains(stderrMessage, "reference broken") { + if checkRecoverableSyncError(stderrMessage) { log.Warn("SyncMirrors [repo: %-v Wiki]: failed to update mirror wiki repository due to broken references:\nStdout: %s\nStderr: %s\nErr: %v\nAttempting Prune", m.Repo, stdoutMessage, stderrMessage, err) err = nil diff --git a/services/mirror/mirror_test.go b/services/mirror/mirror_pull_test.go similarity index 56% rename from services/mirror/mirror_test.go rename to services/mirror/mirror_pull_test.go index 60565b8c5e..97859be5b0 100644 --- a/services/mirror/mirror_test.go +++ b/services/mirror/mirror_pull_test.go @@ -64,3 +64,31 @@ func Test_parseRemoteUpdateOutput(t *testing.T) { assert.Equal(t, "1c97ebc746", results[9].oldCommitID) assert.Equal(t, "976d27d52f", results[9].newCommitID) } + +func Test_checkRecoverableSyncError(t *testing.T) { + cases := []struct { + recoverable bool + message string + }{ + // A race condition in http git-fetch where certain refs were listed on the remote and are no longer there, would exit status 128 + {true, "fatal: remote error: upload-pack: not our ref 988881adc9fc3655077dc2d4d757d480b5ea0e11"}, + // A race condition where a local gc/prune removes a named ref during a git-fetch would exit status 1 + {true, "cannot lock ref 'refs/pull/123456/merge': unable to resolve reference 'refs/pull/134153/merge'"}, + // A race condition in http git-fetch where named refs were listed on the remote and are no longer there + {true, "error: cannot lock ref 'refs/remotes/origin/foo': unable to resolve reference 'refs/remotes/origin/foo': reference broken"}, + // A race condition in http git-fetch where named refs were force-pushed during the update, would exit status 128 + {true, "error: cannot lock ref 'refs/pull/123456/merge': is at 988881adc9fc3655077dc2d4d757d480b5ea0e11 but expected 7f894307ffc9553edbd0b671cab829786866f7b2"}, + // A race condition with other local git operations, such as git-maintenance, would exit status 128 (well, "Unable" the "U" is uppercase) + {true, "fatal: Unable to create '/data/gitea-repositories/foo-org/bar-repo.git/./objects/info/commit-graphs/commit-graph-chain.lock': File exists."}, + // Missing or unauthorized credentials, would exit status 128 + {false, "fatal: Authentication failed for 'https://example.com/foo-does-not-exist/bar.git/'"}, + // A non-existent remote repository, would exit status 128 + {false, "fatal: Could not read from remote repository."}, + // A non-functioning proxy, would exit status 128 + {false, "fatal: unable to access 'https://example.com/foo-does-not-exist/bar.git/': Failed to connect to configured-https-proxy port 1080 after 0 ms: Couldn't connect to server"}, + } + + for _, c := range cases { + assert.Equal(t, c.recoverable, checkRecoverableSyncError(c.message), "test case: %s", c.message) + } +} diff --git a/templates/repo/navbar.tmpl b/templates/repo/navbar.tmpl index b2471dc17e..7536b96c63 100644 --- a/templates/repo/navbar.tmpl +++ b/templates/repo/navbar.tmpl @@ -1,14 +1,18 @@ +{{$canReadCode := $.Permission.CanRead $.UnitTypeCode}} +
diff --git a/templates/shared/actions/runner_edit.tmpl b/templates/shared/actions/runner_edit.tmpl index 54250f830b..d452d69f7a 100644 --- a/templates/shared/actions/runner_edit.tmpl +++ b/templates/shared/actions/runner_edit.tmpl @@ -40,7 +40,7 @@