Mirrors/gitea
0
0
Fork 0
mirror of https://github.com/go-gitea/gitea.git synced 2026-03-26 07:18:54 +01:00
Code Issues Activity

Fix various trivial problems (#36953)

Browse Source 
1. remove `TEST_CONFLICTING_PATCHES_WITH_GIT_APPLY`
* it defaults to false and is unlikely to be useful for most users (see
#22130)
* with new git versions (>= 2.40), "merge-tree" is used,
"checkConflictsByTmpRepo" isn't called, the option does nothing.
2. fix fragile `db.Cell2Int64` (new: `CellToInt`)
3. allow more routes in maintenance mode (e.g.: captcha)
4. fix MockLocale html escaping to make it have the same behavior as
production locale
This commit is contained in:
wxiaoguang 2026-03-24 02:23:42 +08:00 committed by GitHub
parent 788200de9f
commit 4f9f0fc4b8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 97 additions and 314 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
custom/conf/app.example.ini
View File

@ -1153,9 +1153,6 @@ LEVEL = Info
;; Add co-authored-by and co-committed-by trailers if committer does not match author
;ADD_CO_COMMITTER_TRAILERS = true
;;
;; In addition to testing patches using the three-way merge method, re-test conflicting patches with git apply
;TEST_CONFLICTING_PATCHES_WITH_GIT_APPLY = false
;;
;; Retarget child pull requests to the parent pull request branch target on merge of parent pull request. It only works on merged PRs where the head and base branch target the same repo.
;RETARGET_CHILDREN_ON_MERGE = true
;;

6
models/auth/source.go
View File

@ -12,6 +12,7 @@ import (
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/optional"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/modules/util"
@ -139,7 +140,10 @@ func init() {
// BeforeSet is invoked from XORM before setting the value of a field of this object.
func (source *Source) BeforeSet(colName string, val xorm.Cell) {
if colName == "type" {
typ := Type(db.Cell2Int64(val))
typ, _, err := db.CellToInt(val, NoType)
if err != nil {
setting.PanicInDevOrTesting("Unable to convert login source (id=%d) type: %v", source.ID, err)
}
constructor, ok := registeredConfigs[typ]
if !ok {
return

40
models/auth/source_test.go
View File

@ -17,13 +17,9 @@ import (
)
type TestSource struct {
auth_model.ConfigBase
auth_model.ConfigBase `json:"-"`
Provider string
ClientID string
ClientSecret string
OpenIDConnectAutoDiscoveryURL string
IconURL string
TestField string
}
// FromDB fills up a LDAPConfig from serialized format.
@ -37,27 +33,23 @@ func (source *TestSource) ToDB() ([]byte, error) {
}
func TestDumpAuthSource(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
require.NoError(t, unittest.PrepareTestDatabase())
authSourceSchema, err := unittest.GetXORMEngine().TableInfo(new(auth_model.Source))
assert.NoError(t, err)
require.NoError(t, err)
auth_model.RegisterTypeConfig(auth_model.OAuth2, new(TestSource))
source := &auth_model.Source{
Type: auth_model.OAuth2,
Name: "TestSource",
Cfg: &TestSource{TestField: "TestValue"},
}
require.NoError(t, auth_model.CreateSource(t.Context(), source))
auth_model.CreateSource(t.Context(), &auth_model.Source{
Type: auth_model.OAuth2,
Name: "TestSource",
IsActive: false,
Cfg: &TestSource{
Provider: "ConvertibleSourceName",
ClientID: "42",
},
})
sb := new(strings.Builder)
// TODO: this test is quite hacky, it should use a low-level "select" (without model processors) but not a database dump
engine := unittest.GetXORMEngine()
require.NoError(t, engine.DumpTables([]*schemas.Table{authSourceSchema}, sb))
assert.Contains(t, sb.String(), `"Provider":"ConvertibleSourceName"`)
// intentionally test the "dump" to make sure the dumped JSON is correct: https://github.com/go-gitea/gitea/pull/16847
sb := &strings.Builder{}
require.NoError(t, unittest.GetXORMEngine().DumpTables([]*schemas.Table{authSourceSchema}, sb))
// the dumped SQL is something like:
// INSERT INTO `login_source` (`id`, `type`, `name`, `is_active`, `is_sync_enabled`, `two_factor_policy`, `cfg`, `created_unix`, `updated_unix`) VALUES (1,6,'TestSource',0,0,'','{"TestField":"TestValue"}',1774179784,1774179784);
assert.Contains(t, sb.String(), `'{"TestField":"TestValue"}'`)
}

22
models/db/convert.go
View File

@ -5,12 +5,11 @@ package db
import (
"fmt"
"strconv"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"xorm.io/xorm"
"xorm.io/xorm/convert"
"xorm.io/xorm/schemas"
)
@ -74,15 +73,14 @@ WHERE ST.name ='varchar'`)
return err
}
// Cell2Int64 converts a xorm.Cell type to int64,
// and handles possible irregular cases.
func Cell2Int64(val xorm.Cell) int64 {
switch (*val).(type) {
case []uint8:
log.Trace("Cell2Int64 ([]uint8): %v", *val)
v, _ := strconv.ParseInt(string((*val).([]uint8)), 10, 64)
return v
// CellToInt converts a xorm.Cell field value to an int value
func CellToInt[T ~int | int64](cell xorm.Cell, def T) (ret T, has bool, err error) {
if *cell == nil {
return def, false, nil
}
return (*val).(int64)
val, err := convert.AsInt64(*cell)
if err != nil {
return def, false, err
}
return T(val), true, err
}

6
models/repo/repo_unit.go
View File

@ -227,7 +227,11 @@ func (cfg *ProjectsConfig) IsProjectsAllowed(m ProjectsMode) bool {
func (r *RepoUnit) BeforeSet(colName string, val xorm.Cell) {
switch colName {
case "type":
r.Type = unit.Type(db.Cell2Int64(val))
var err error
r.Type, _, err = db.CellToInt(val, unit.TypeInvalid)
if err != nil {
setting.PanicInDevOrTesting("Unable to convert repo unit (id=%d) type: %v", r.ID, err)
}
switch r.Type {
case unit.TypeExternalWiki:
r.Config = new(ExternalWikiConfig)

2
modules/setting/repository.go
View File

@ -86,7 +86,6 @@ var (
DefaultMergeMessageOfficialApproversOnly bool
PopulateSquashCommentWithCommitMessages bool
AddCoCommitterTrailers bool
TestConflictingPatchesWithGitApply bool
RetargetChildrenOnMerge bool
DelayCheckForInactiveDays int
DefaultDeleteBranchAfterMerge bool
@ -211,7 +210,6 @@ var (
DefaultMergeMessageOfficialApproversOnly bool
PopulateSquashCommentWithCommitMessages bool
AddCoCommitterTrailers bool
TestConflictingPatchesWithGitApply bool
RetargetChildrenOnMerge bool
DelayCheckForInactiveDays int
DefaultDeleteBranchAfterMerge bool

41
modules/translation/mock.go
View File

@ -5,8 +5,8 @@ package translation
import (
"fmt"
"html"
"html/template"
"strings"
)
// MockLocale provides a mocked locale without any translations
@ -20,25 +20,40 @@ func (l MockLocale) Language() string {
return "en"
}
func (l MockLocale) TrString(s string, args ...any) string {
return sprintAny(s, args...)
func (l MockLocale) TrString(format string, args ...any) (ret string) {
ret = format + ":"
for _, arg := range args {
// usually there is no arg or at most 1-2 args, so a simple string concatenation is more efficient
switch v := arg.(type) {
case string:
ret += v + ","
default:
ret += fmt.Sprint(v) + ","
}
}
return ret[:len(ret)-1]
}
func (l MockLocale) Tr(s string, args ...any) template.HTML {
return template.HTML(sprintAny(s, args...))
func (l MockLocale) Tr(format string, args ...any) (ret template.HTML) {
ret = template.HTML(html.EscapeString(format)) + ":"
for _, arg := range args {
// usually there is no arg or at most 1-2 args, so a simple string concatenation is more efficient
switch v := arg.(type) {
case template.HTML:
ret += v + ","
case string:
ret += template.HTML(html.EscapeString(v)) + ","
default:
ret += template.HTML(html.EscapeString(fmt.Sprint(v))) + ","
}
}
return ret[:len(ret)-1]
}
func (l MockLocale) TrN(cnt any, key1, keyN string, args ...any) template.HTML {
return template.HTML(sprintAny(key1, args...))
return l.Tr(key1, args...)
}
func (l MockLocale) PrettyNumber(v any) string {
return fmt.Sprint(v)
}
func sprintAny(s string, args ...any) string {
if len(args) == 0 {
return s
}
return s + ":" + fmt.Sprintf(strings.Repeat(",%v", len(args))[1:], args...)
}

48
routers/common/maintenancemode.go
View File

@ -7,29 +7,39 @@ import (
"net/http"
"strings"
"code.gitea.io/gitea/modules/container"
"code.gitea.io/gitea/modules/setting"
)
func isMaintenanceModeAllowedRequest(req *http.Request) bool {
if strings.HasPrefix(req.URL.Path, "/-/") {
// URLs like "/-/admin", "/-/fetch-redirect" and "/-/markup" are still accessible in maintenance mode
return true
}
if strings.HasPrefix(req.URL.Path, "/api/internal/") {
// internal APIs should be allowed
return true
}
if strings.HasPrefix(req.URL.Path, "/user/") {
// URLs like "/user/signin" and "/user/signup" are still accessible in maintenance mode
return true
}
if strings.HasPrefix(req.URL.Path, "/assets/") {
return true
}
return false
}
func MaintenanceModeHandler() func(h http.Handler) http.Handler {
allowedPrefixes := []string{
"/.well-known/",
"/assets/",
"/avatars/",
// admin: "/-/admin"
// general-purpose URLs: "/-/fetch-redirect", "/-/markup", etc.
"/-/",
// internal APIs
"/api/internal/",
// user login (for admin to login): "/user/login", "/user/logout", "/catpcha/..."
"/user/",
"/captcha/",
}
allowedPaths := container.SetOf(
"/api/healthz",
)
isMaintenanceModeAllowedRequest := func(req *http.Request) bool {
for _, prefix := range allowedPrefixes {
if strings.HasPrefix(req.URL.Path, prefix) {
return true
}
}
return allowedPaths.Contains(req.URL.Path)
}
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
maintenanceMode := setting.Config().Instance.MaintenanceMode.Value(req.Context())

162
services/pull/patch.go
View File

@ -5,7 +5,6 @@
package pull
import (
"bufio"
"context"
"fmt"
"io"
@ -15,15 +14,12 @@ import (
git_model "code.gitea.io/gitea/models/git"
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/unit"
"code.gitea.io/gitea/modules/container"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/git/gitcmd"
"code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/modules/glob"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/process"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"
)
@ -57,15 +53,6 @@ func DownloadDiffOrPatch(ctx context.Context, pr *issues_model.PullRequest, w io
return nil
}
var patchErrorSuffices = []string{
": already exists in index",
": patch does not apply",
": already exists in working directory",
"unrecognized input",
": No such file or directory",
": does not exist in index",
}
func checkPullRequestBranchMergeable(ctx context.Context, pr *issues_model.PullRequest) error {
ctx, _, finished := process.GetManager().AddContext(ctx, fmt.Sprintf("checkPullRequestBranchMergeable: %s", pr))
defer finished()
@ -349,151 +336,10 @@ func checkConflictsByTmpRepo(ctx context.Context, pr *issues_model.PullRequest,
}
// 3. OK the three-way merge method has detected conflicts
// 3a. Are still testing with GitApply? If not set the conflict status and move on
if !setting.Repository.PullRequest.TestConflictingPatchesWithGitApply {
pr.Status = issues_model.PullRequestStatusConflict
pr.ConflictedFiles = conflictFiles
log.Trace("Found %d files conflicted: %v", len(pr.ConflictedFiles), pr.ConflictedFiles)
return true, nil
}
// 3b. Create a plain patch from head to base
tmpPatchFile, cleanup, err := setting.AppDataTempDir("git-repo-content").CreateTempFileRandom("patch")
if err != nil {
log.Error("Unable to create temporary patch file! Error: %v", err)
return false, fmt.Errorf("unable to create temporary patch file! Error: %w", err)
}
defer cleanup()
if err := gitRepo.GetDiffBinary(pr.MergeBase+"...tracking", tmpPatchFile); err != nil {
log.Error("Unable to get patch file from %s to %s in %s Error: %v", pr.MergeBase, pr.HeadBranch, pr.BaseRepo.FullName(), err)
return false, fmt.Errorf("unable to get patch file from %s to %s in %s Error: %w", pr.MergeBase, pr.HeadBranch, pr.BaseRepo.FullName(), err)
}
stat, err := tmpPatchFile.Stat()
if err != nil {
return false, fmt.Errorf("unable to stat patch file: %w", err)
}
patchPath := tmpPatchFile.Name()
tmpPatchFile.Close()
// 3c. if the size of that patch is 0 - there can be no conflicts!
if stat.Size() == 0 {
log.Debug("PullRequest[%d]: Patch is empty - ignoring", pr.ID)
pr.Status = issues_model.PullRequestStatusEmpty
return false, nil
}
log.Trace("PullRequest[%d].checkPullRequestMergeableByTmpRepo (patchPath): %s", pr.ID, patchPath)
// 4. Read the base branch in to the index of the temporary repository
_, _, err = gitcmd.NewCommand("read-tree", tmpRepoBaseBranch).WithDir(tmpBasePath).RunStdString(ctx)
if err != nil {
return false, fmt.Errorf("git read-tree %s: %w", pr.BaseBranch, err)
}
// 5. Now get the pull request configuration to check if we need to ignore whitespace
prUnit, err := pr.BaseRepo.GetUnit(ctx, unit.TypePullRequests)
if err != nil {
return false, err
}
prConfig := prUnit.PullRequestsConfig()
// 6. Prepare the arguments to apply the patch against the index
cmdApply := gitcmd.NewCommand("apply", "--check", "--cached")
if prConfig.IgnoreWhitespaceConflicts {
cmdApply.AddArguments("--ignore-whitespace")
}
is3way := false
if git.DefaultFeatures().CheckVersionAtLeast("2.32.0") {
cmdApply.AddArguments("--3way")
is3way = true
}
cmdApply.AddDynamicArguments(patchPath)
// 7. Prep the pipe:
// - Here we could do the equivalent of:
// `git apply --check --cached patch_file > conflicts`
// Then iterate through the conflicts. However, that means storing all the conflicts
// in memory - which is very wasteful.
// - alternatively we can do the equivalent of:
// `git apply --check ... | grep ...`
// meaning we don't store all the conflicts unnecessarily.
stderrReader, stderrReaderClose := cmdApply.MakeStderrPipe()
defer stderrReaderClose()
// 8. Run the check command
conflict = false
err = cmdApply.
WithDir(tmpBasePath).
WithPipelineFunc(func(ctx gitcmd.Context) error {
const prefix = "error: patch failed:"
const errorPrefix = "error: "
const threewayFailed = "Failed to perform three-way merge..."
const appliedPatchPrefix = "Applied patch to '"
const withConflicts = "' with conflicts."
conflicts := make(container.Set[string])
// Now scan the output from the command
scanner := bufio.NewScanner(stderrReader)
for scanner.Scan() {
line := scanner.Text()
log.Trace("PullRequest[%d].checkPullRequestMergeableByTmpRepo: stderr: %s", pr.ID, line)
if strings.HasPrefix(line, prefix) {
conflict = true
filepath := strings.TrimSpace(strings.Split(line[len(prefix):], ":")[0])
conflicts.Add(filepath)
} else if is3way && line == threewayFailed {
conflict = true
} else if strings.HasPrefix(line, errorPrefix) {
conflict = true
for _, suffix := range patchErrorSuffices {
if strings.HasSuffix(line, suffix) {
filepath := strings.TrimSpace(strings.TrimSuffix(line[len(errorPrefix):], suffix))
if filepath != "" {
conflicts.Add(filepath)
}
break
}
}
} else if is3way && strings.HasPrefix(line, appliedPatchPrefix) && strings.HasSuffix(line, withConflicts) {
conflict = true
filepath := strings.TrimPrefix(strings.TrimSuffix(line, withConflicts), appliedPatchPrefix)
if filepath != "" {
conflicts.Add(filepath)
}
}
// only list part of conflicted files
if len(conflicts) >= gitrepo.MaxConflictedDetectFiles {
break
}
}
if len(conflicts) > 0 {
pr.ConflictedFiles = make([]string, 0, len(conflicts))
for key := range conflicts {
pr.ConflictedFiles = append(pr.ConflictedFiles, key)
}
}
return nil
}).
Run(gitRepo.Ctx)
// 9. Check if the found conflicted files is non-zero, "err" could be non-nil, so we should ignore it if we found conflicts.
// Note: `"err" could be non-nil` is due that if enable 3-way merge, it doesn't return any error on found conflicts.
if len(pr.ConflictedFiles) > 0 {
if conflict {
pr.Status = issues_model.PullRequestStatusConflict
log.Trace("Found %d files conflicted: %v", len(pr.ConflictedFiles), pr.ConflictedFiles)
return true, nil
}
} else if err != nil {
return false, fmt.Errorf("git apply --check: %w", err)
}
return false, nil
pr.Status = issues_model.PullRequestStatusConflict
pr.ConflictedFiles = conflictFiles
log.Trace("Found %d files conflicted: %v", len(pr.ConflictedFiles), pr.ConflictedFiles)
return true, nil
}
// ErrFilePathProtected represents a "FilePathProtected" kind of error.

81
tests/integration/pull_merge_test.go
View File

@ -39,7 +39,6 @@ import (
pull_service "code.gitea.io/gitea/services/pull"
repo_service "code.gitea.io/gitea/services/repository"
commitstatus_service "code.gitea.io/gitea/services/repository/commitstatus"
files_service "code.gitea.io/gitea/services/repository/files"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@ -549,86 +548,6 @@ func TestCantFastForwardOnlyMergeDiverging(t *testing.T) {
})
}
func TestConflictChecking(t *testing.T) {
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
// Create new clean repo to test conflict checking.
baseRepo, err := repo_service.CreateRepository(t.Context(), user, user, repo_service.CreateRepoOptions{
Name: "conflict-checking",
Description: "Tempo repo",
AutoInit: true,
Readme: "Default",
DefaultBranch: "main",
})
assert.NoError(t, err)
assert.NotEmpty(t, baseRepo)
// create a commit on new branch.
_, err = files_service.ChangeRepoFiles(t.Context(), baseRepo, user, &files_service.ChangeRepoFilesOptions{
Files: []*files_service.ChangeRepoFile{
{
Operation: "create",
TreePath: "important_file",
ContentReader: strings.NewReader("Just a non-important file"),
},
},
Message: "Add a important file",
OldBranch: "main",
NewBranch: "important-secrets",
})
assert.NoError(t, err)
// create a commit on main branch.
_, err = files_service.ChangeRepoFiles(t.Context(), baseRepo, user, &files_service.ChangeRepoFilesOptions{
Files: []*files_service.ChangeRepoFile{
{
Operation: "create",
TreePath: "important_file",
ContentReader: strings.NewReader("Not the same content :P"),
},
},
Message: "Add a important file",
OldBranch: "main",
NewBranch: "main",
})
assert.NoError(t, err)
// create Pull to merge the important-secrets branch into main branch.
pullIssue := &issues_model.Issue{
RepoID: baseRepo.ID,
Title: "PR with conflict!",
PosterID: user.ID,
Poster: user,
IsPull: true,
}
pullRequest := &issues_model.PullRequest{
HeadRepoID: baseRepo.ID,
BaseRepoID: baseRepo.ID,
HeadBranch: "important-secrets",
BaseBranch: "main",
HeadRepo: baseRepo,
BaseRepo: baseRepo,
Type: issues_model.PullRequestGitea,
}
prOpts := &pull_service.NewPullRequestOptions{Repo: baseRepo, Issue: pullIssue, PullRequest: pullRequest}
err = pull_service.NewPullRequest(t.Context(), prOpts)
assert.NoError(t, err)
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "PR with conflict!"})
assert.NoError(t, issue.LoadPullRequest(t.Context()))
conflictingPR := issue.PullRequest
// Ensure conflictedFiles is populated.
assert.Len(t, conflictingPR.ConflictedFiles, 1)
// Check if status is correct.
assert.Equal(t, issues_model.PullRequestStatusConflict, conflictingPR.Status)
// Ensure that mergeable returns false
assert.False(t, conflictingPR.Mergeable(t.Context()))
})
}
func TestPullRetargetChildOnBranchDelete(t *testing.T) {
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
session := loginUser(t, "user1") // FIXME: don't use admin user for testing