From fedc9dc993f0f14866fa4dcbe57c6ba8d90a180e Mon Sep 17 00:00:00 2001 From: Nicolas Date: Tue, 28 Apr 2026 17:26:08 +0200 Subject: [PATCH 1/3] FIX: URL sanitization to handle schemeless credentials (#37440) Fixes #37435 --------- Co-authored-by: silverwind Co-authored-by: Claude (Opus 4.7) Co-authored-by: wxiaoguang Co-authored-by: Giteabot --- modules/git/gitcmd/command.go | 6 +- modules/git/gitcmd/command_test.go | 5 +- modules/util/sanitize.go | 122 +++++++++++++++++++++-------- modules/util/sanitize_test.go | 53 ++++++++++--- services/migrations/migrate.go | 2 +- 5 files changed, 139 insertions(+), 49 deletions(-) diff --git a/modules/git/gitcmd/command.go b/modules/git/gitcmd/command.go index e9b51802fe..ee447dfd03 100644 --- a/modules/git/gitcmd/command.go +++ b/modules/git/gitcmd/command.go @@ -57,14 +57,12 @@ type Command struct { } func logArgSanitize(arg string) string { - if strings.Contains(arg, "://") && strings.Contains(arg, "@") { - return util.SanitizeCredentialURLs(arg) - } else if filepath.IsAbs(arg) { + if filepath.IsAbs(arg) { base := filepath.Base(arg) dir := filepath.Dir(arg) return ".../" + filepath.Join(filepath.Base(dir), base) } - return arg + return util.SanitizeCredentialURLs(arg) } func (c *Command) LogString() string { diff --git a/modules/git/gitcmd/command_test.go b/modules/git/gitcmd/command_test.go index 19ec02b808..1a9bfe7d75 100644 --- a/modules/git/gitcmd/command_test.go +++ b/modules/git/gitcmd/command_test.go @@ -109,7 +109,10 @@ func TestCommandString(t *testing.T) { assert.Equal(t, cmd.prog+` a "-m msg" "it's a test" "say \"hello\""`, cmd.LogString()) cmd = NewCommand("url: https://a:b@c/", "/root/dir-a/dir-b") - assert.Equal(t, cmd.prog+` "url: https://sanitized-credential@c/" .../dir-a/dir-b`, cmd.LogString()) + assert.Equal(t, cmd.prog+` "url: https://(masked)@c/" .../dir-a/dir-b`, cmd.LogString()) + + cmd = NewCommand("url: a:b@c/", "/root/dir-a/dir-b") + assert.Equal(t, cmd.prog+` "url: (masked)@c/" .../dir-a/dir-b`, cmd.LogString()) } func TestRunStdError(t *testing.T) { diff --git a/modules/util/sanitize.go b/modules/util/sanitize.go index 0dd8b342a2..88ca34788f 100644 --- a/modules/util/sanitize.go +++ b/modules/util/sanitize.go @@ -5,7 +5,8 @@ package util import ( "bytes" - "unicode" + "net" + "strings" ) type sanitizedError struct { @@ -25,48 +26,103 @@ func SanitizeErrorCredentialURLs(err error) error { return sanitizedError{err: err} } -const userPlaceholder = "sanitized-credential" - var schemeSep = []byte("://") -// SanitizeCredentialURLs remove all credentials in URLs (starting with "scheme://") for the input string: "https://user:pass@domain.com" => "https://sanitized-credential@domain.com" +const userInfoPlaceholder = "(masked)" + +// SanitizeCredentialURLs remove all credentials in URLs for the input string: +// * "https://userinfo@domain.com" => "https://***@domain.com" +// * "user:pass@domain.com" => "***@domain.com" +// "***" is a magic string internally used, doesn't guarantee to be anything. func SanitizeCredentialURLs(s string) string { + sepColPos := strings.Index(s, ":") + if sepColPos == -1 { + return s // fast path: no colon, unlikely contain any URL credential + } + sepAtPos := strings.Index(s[sepColPos+1:], "@") + for sepAtPos == -1 { + return s // fast path: no "@" after colon, unlikely contain any URL credential + } + sepAtPos += sepColPos + 1 + + res := make([]byte, 0, len(s)+len(userInfoPlaceholder)) // a best guess to avoid too many re-allocations bs := UnsafeStringToBytes(s) - schemeSepPos := bytes.Index(bs, schemeSep) - if schemeSepPos == -1 || bytes.IndexByte(bs[schemeSepPos:], '@') == -1 { - return s // fast return if there is no URL scheme or no userinfo - } - out := make([]byte, 0, len(bs)+len(userPlaceholder)) - for schemeSepPos != -1 { - schemeSepPos += 3 // skip the "://" - sepAtPos := -1 // the possible '@' position: "https://foo@[^here]host" - sepEndPos := schemeSepPos // the possible end position: "The https://host[^here] in log for test" - sepLoop: - for ; sepEndPos < len(bs); sepEndPos++ { - c := bs[sepEndPos] - if ('A' <= c && c <= 'Z') || ('a' <= c && c <= 'z') || ('0' <= c && c <= '9') { - continue - } + for { + // left part (before "@") is likely to be the "userinfo" (single username, or "username:password") + leftPos := sepAtPos - 1 + leftLoop: + for leftPos >= 0 { + c := bs[leftPos] switch c { - case '@': - sepAtPos = sepEndPos case '-', '.', '_', '~', '!', '$', '&', '\'', '(', ')', '*', '+', ',', ';', '=', ':', '%': - continue // due to RFC 3986, userinfo can contain - . _ ~ ! $ & ' ( ) * + , ; = : and any percent-encoded chars + // RFC 3986, userinfo can contain - . _ ~ ! $ & ' ( ) * + , ; = : and any percent-encoded chars default: - break sepLoop // if it is an invalid char for URL (eg: space, '/', and others), stop the loop + valid := 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' || '0' <= c && c <= '9' + if !valid { + break leftLoop + } } + leftPos-- } - // if there is '@', and the string is like "s://u@h", then hide the "u" part - if sepAtPos != -1 && (schemeSepPos >= 4 && unicode.IsLetter(rune(bs[schemeSepPos-4]))) && sepAtPos-schemeSepPos > 0 && sepEndPos-sepAtPos > 0 { - out = append(out, bs[:schemeSepPos]...) - out = append(out, userPlaceholder...) - out = append(out, bs[sepAtPos:sepEndPos]...) + // left pos should point to the beginning of the left part, this pos is always valid in the buffer + leftPos++ + + // right part is likely to be the host (domain name, ip address) + rightPos := sepAtPos + 1 + rightLoop: + for rightPos < len(bs) { + c := bs[rightPos] + switch c { + case '.', '-': + // valid host char + case '[': + // ipv6 begin + if rightPos != sepAtPos+1 { + break rightLoop + } + case ']': + // ipv6 end + rightPos++ + break rightLoop + default: + valid := 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' || '0' <= c && c <= '9' + if bs[sepAtPos+1] == '[' { + // ipv6 host + valid = 'a' <= c && c <= 'f' || 'A' <= c && c <= 'F' || '0' <= c && c <= '9' || c == ':' + } + if !valid { + break rightLoop + } + } + rightPos++ + } + + leading, leftPart, rightPart := bs[:leftPos], bs[leftPos:sepAtPos], bs[sepAtPos+1:rightPos] + + // Either: + // * git log message: "user:pass@host" (it contains a colon in userinfo), ignore "git@host" pattern + // * http like URL: "https://userinfo@host.com" (it has "://" before the userinfo) + needSanitize := bytes.IndexByte(leftPart, ':') >= 0 || bytes.HasSuffix(leading, schemeSep) + needSanitize = needSanitize && len(leftPart) > 0 && len(rightPart) > 0 + // TODO: can also do more checks for right part + // for example: ipv6 quick check + if needSanitize && rightPart[0] == '[' { + needSanitize = rightPart[len(rightPart)-1] == ']' && net.ParseIP(UnsafeBytesToString(rightPart[1:len(rightPart)-1])) != nil + } + if needSanitize { + res = append(res, leading...) + res = append(res, userInfoPlaceholder...) + res = append(res, '@') + res = append(res, rightPart...) } else { - out = append(out, bs[:sepEndPos]...) + res = append(res, bs[:rightPos]...) + } + bs = bs[rightPos:] + sepAtPos = bytes.IndexByte(bs, '@') + if sepAtPos == -1 { + break } - bs = bs[sepEndPos:] - schemeSepPos = bytes.Index(bs, schemeSep) } - out = append(out, bs...) - return UnsafeBytesToString(out) + res = append(res, bs...) + return UnsafeBytesToString(res) } diff --git a/modules/util/sanitize_test.go b/modules/util/sanitize_test.go index 0bcfd45ca4..c1a80016f8 100644 --- a/modules/util/sanitize_test.go +++ b/modules/util/sanitize_test.go @@ -13,7 +13,7 @@ import ( func TestSanitizeErrorCredentialURLs(t *testing.T) { err := errors.New("error with https://a@b.com") se := SanitizeErrorCredentialURLs(err) - assert.Equal(t, "error with https://"+userPlaceholder+"@b.com", se.Error()) + assert.Equal(t, "error with https://"+userInfoPlaceholder+"@b.com", se.Error()) } func TestSanitizeCredentialURLs(t *testing.T) { @@ -27,15 +27,35 @@ func TestSanitizeCredentialURLs(t *testing.T) { }, { "https://mytoken@github.com/go-gitea/test_repo.git", - "https://" + userPlaceholder + "@github.com/go-gitea/test_repo.git", + "https://" + userInfoPlaceholder + "@github.com/go-gitea/test_repo.git", }, { "https://user:password@github.com/go-gitea/test_repo.git", - "https://" + userPlaceholder + "@github.com/go-gitea/test_repo.git", + "https://" + userInfoPlaceholder + "@github.com/go-gitea/test_repo.git", + }, + { + "https://user:password@[::]/go-gitea/test_repo.git", + "https://" + userInfoPlaceholder + "@[::]/go-gitea/test_repo.git", + }, + { + "https://user:password@[2001:db8::1]:8080/go-gitea/test_repo.git", + "https://" + userInfoPlaceholder + "@[2001:db8::1]:8080/go-gitea/test_repo.git", + }, + { + "see https://u:p@[::1]/x and https://u2:p2@h2", + "see https://" + userInfoPlaceholder + "@[::1]/x and https://" + userInfoPlaceholder + "@h2", + }, + { + "https://user:secret@[unclosed-ipv6", + "https://user:secret@[unclosed-ipv6", + }, + { + "https://user:secret@[invalid-ipv6]", + "https://user:secret@[invalid-ipv6]", }, { "ftp://x@", - "ftp://" + userPlaceholder + "@", + "ftp://x@", }, { "ftp://x/@", @@ -43,27 +63,40 @@ func TestSanitizeCredentialURLs(t *testing.T) { }, { "ftp://u@x/@", // test multiple @ chars - "ftp://" + userPlaceholder + "@x/@", + "ftp://" + userInfoPlaceholder + "@x/@", }, { "😊ftp://u@x😊", // test unicode - "😊ftp://" + userPlaceholder + "@x😊", + "😊ftp://" + userInfoPlaceholder + "@x😊", }, { "://@", "://@", }, { - "//u:p@h", // do not process URLs without explicit scheme, they are not treated as "valid" URLs because there is no scheme context in string "//u:p@h", + "//" + userInfoPlaceholder + "@h", }, { - "s://u@h", // the minimal pattern to be sanitized - "s://" + userPlaceholder + "@h", + "s://u@h", + "s://" + userInfoPlaceholder + "@h", }, { "URLs in log https://u:b@h and https://u:b@h:80/, with https://h.com and u@h.com", - "URLs in log https://" + userPlaceholder + "@h and https://" + userPlaceholder + "@h:80/, with https://h.com and u@h.com", + "URLs in log https://" + userInfoPlaceholder + "@h and https://" + userInfoPlaceholder + "@h:80/, with https://h.com and u@h.com", + }, + { + "fatal: unable to look up username:token@github.com (port 9418)", + "fatal: unable to look up " + userInfoPlaceholder + "@github.com (port 9418)", + }, + { + "git failed for user:token@github.com/go-gitea/test_repo.git", + "git failed for " + userInfoPlaceholder + "@github.com/go-gitea/test_repo.git", + }, + { + // SSH-form git URL ("git@host:path") must not let a later credential URL through + "failed remote git@github.com:foo, retried via https://user:tok@github.com/foo", + "failed remote git@github.com:foo, retried via https://" + userInfoPlaceholder + "@github.com/foo", }, } diff --git a/services/migrations/migrate.go b/services/migrations/migrate.go index 99f8dba92f..0cdd96496d 100644 --- a/services/migrations/migrate.go +++ b/services/migrations/migrate.go @@ -218,7 +218,7 @@ func migrateRepository(ctx context.Context, doer *user_model.User, downloader ba // We don't actually need to check the OriginalURL as it isn't used anywhere } - log.Trace("migrating git data from %s", repo.CloneURL) + log.Trace("migrating git data from %s", util.SanitizeCredentialURLs(repo.CloneURL)) messenger("repo.migrate.migrating_git") if err = uploader.CreateRepo(ctx, repo, opts); err != nil { return err From deec2b0929c5a7badd74df8bcb767a8cce51e33f Mon Sep 17 00:00:00 2001 From: Nicolas Date: Tue, 28 Apr 2026 23:03:50 +0200 Subject: [PATCH 2/3] Fix compare dropdown for branches without common history (#37470) --- modules/gitrepo/compare_test.go | 32 +++++ modules/gitrepo/merge.go | 6 +- options/locale/locale_en-US.json | 1 + routers/web/repo/compare.go | 197 ++++++++++-------------------- routers/web/repo/pull.go | 31 ++--- services/git/compare.go | 11 +- templates/repo/diff/compare.tmpl | 1 + tests/integration/compare_test.go | 35 ++++++ 8 files changed, 160 insertions(+), 154 deletions(-) diff --git a/modules/gitrepo/compare_test.go b/modules/gitrepo/compare_test.go index 2d2af0934d..91ee32bed5 100644 --- a/modules/gitrepo/compare_test.go +++ b/modules/gitrepo/compare_test.go @@ -4,9 +4,15 @@ package gitrepo import ( + "path/filepath" + "strings" "testing" + "code.gitea.io/gitea/modules/git/gitcmd" + "code.gitea.io/gitea/modules/util" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type mockRepository struct { @@ -17,6 +23,32 @@ func (r *mockRepository) RelativePath() string { return r.path } +func TestMergeBaseNoCommonHistory(t *testing.T) { + repoDir := filepath.Join(t.TempDir(), "repo.git") + require.NoError(t, gitcmd.NewCommand("init").AddDynamicArguments(repoDir).Run(t.Context())) + _, _, runErr := gitcmd.NewCommand("fast-import").WithDir(repoDir).WithStdinBytes([]byte(strings.TrimSpace(` +commit refs/heads/branch1 +committer User 1714310400 +0000 +data 12 +First commit +M 100644 inline file1.txt +data 12 +Hello from 1 + +commit refs/heads/branch2 +committer User 1714310400 +0000 +data 13 +Second commit +M 100644 inline file2.txt +data 12 +Hello from 2 +`))).RunStdString(t.Context()) + require.NoError(t, runErr) + mergeBase, err := MergeBase(t.Context(), &mockRepository{path: repoDir}, "branch1", "branch2") + assert.Empty(t, mergeBase) + assert.ErrorIs(t, err, util.ErrNotExist) +} + func TestRepoGetDivergingCommits(t *testing.T) { repo := &mockRepository{path: "repo1_bare"} do, err := GetDivergingCommits(t.Context(), repo, "master", "branch2") diff --git a/modules/gitrepo/merge.go b/modules/gitrepo/merge.go index 8d58e21c8d..5198392949 100644 --- a/modules/gitrepo/merge.go +++ b/modules/gitrepo/merge.go @@ -9,13 +9,17 @@ import ( "strings" "code.gitea.io/gitea/modules/git/gitcmd" + "code.gitea.io/gitea/modules/util" ) // MergeBase checks and returns merge base of two commits. func MergeBase(ctx context.Context, repo Repository, baseCommitID, headCommitID string) (string, error) { - mergeBase, _, err := RunCmdString(ctx, repo, gitcmd.NewCommand("merge-base"). + mergeBase, stderr, err := RunCmdString(ctx, repo, gitcmd.NewCommand("merge-base"). AddDashesAndList(baseCommitID, headCommitID)) if err != nil { + if gitcmd.IsErrorExitCode(err, 1) && strings.TrimSpace(stderr) == "" { + return "", util.NewNotExistErrorf("merge-base for %s and %s doesn't exist", baseCommitID, headCommitID) + } return "", fmt.Errorf("get merge-base of %s and %s failed: %w", baseCommitID, headCommitID, err) } return strings.TrimSpace(mergeBase), nil diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index 6281ff8f54..2ee983ae18 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -1784,6 +1784,7 @@ "repo.pulls.review_only_possible_for_full_diff": "Review is only possible when viewing the full diff", "repo.pulls.filter_changes_by_commit": "Filter by commit", "repo.pulls.nothing_to_compare": "These branches are equal. There is no need to create a pull request.", + "repo.pulls.no_common_history": "These branches do not share a common merge base. Select a different base or compare branch.", "repo.pulls.nothing_to_compare_have_tag": "The selected branches/tags are equal.", "repo.pulls.nothing_to_compare_and_allow_empty_pr": "These branches are equal. This PR will be empty.", "repo.pulls.has_pull_request": "A pull request between these branches already exists: %[2]s#%[3]d", diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 7598ce561c..f37c9ef2d1 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -189,8 +189,8 @@ func setCsvCompareContext(ctx *context.Context) { } } -// ParseCompareInfo parse compare info between two commit for preparing comparing references -func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo { +// parseCompareInfo parse compare info between two commit for preparing comparing references +func parseCompareInfo(ctx *context.Context) (*git_service.CompareInfo, error) { baseRepo := ctx.Repo.Repository fileOnly := ctx.FormBool("file-only") @@ -199,47 +199,29 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo { // remove the check when we support compare with carets if compareReq.BaseOriRefSuffix != "" { - ctx.HTTPError(http.StatusBadRequest, "Unsupported comparison syntax: ref with suffix") - return nil + return nil, util.NewInvalidArgumentErrorf("unsupported comparison syntax: ref with suffix") } // 2 get repository and owner for head headOwner, headRepo, err := common.GetHeadOwnerAndRepo(ctx, baseRepo, compareReq) - switch { - case errors.Is(err, util.ErrInvalidArgument): - ctx.HTTPError(http.StatusBadRequest, err.Error()) - return nil - case errors.Is(err, util.ErrNotExist): - ctx.NotFound(nil) - return nil - case err != nil: - ctx.ServerError("GetHeadOwnerAndRepo", err) - return nil + if err != nil { + return nil, err } - isSameRepo := baseRepo.ID == headRepo.ID - // 3 permission check // base repository's code unit read permission check has been done on web.go permBase := ctx.Repo.Permission // If we're not merging from the same repo: + isSameRepo := baseRepo.ID == headRepo.ID if !isSameRepo { // Assert ctx.Doer has permission to read headRepo's codes permHead, err := access_model.GetDoerRepoPermission(ctx, headRepo, ctx.Doer) if err != nil { - ctx.ServerError("GetDoerRepoPermission", err) - return nil + return nil, err } if !permHead.CanRead(unit.TypeCode) { - if log.IsTrace() { - log.Trace("Permission Denied: User: %-v cannot read code in Repo: %-v\nUser in headRepo has Permissions: %-+v", - ctx.Doer, - headRepo, - permHead) - } - ctx.NotFound(nil) - return nil + return nil, util.NewNotExistErrorf("") // permission: no error message for end users } ctx.Data["CanWriteToHeadRepo"] = permHead.CanWrite(unit.TypeCode) } @@ -250,24 +232,17 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo { baseRef := ctx.Repo.GitRepo.UnstableGuessRefByShortName(baseRefName) if baseRef == "" { - ctx.NotFound(nil) - return nil + return nil, util.NewNotExistErrorf("no base ref: %s", baseRefName) } - var headGitRepo *git.Repository - if isSameRepo { - headGitRepo = ctx.Repo.GitRepo - } else { - headGitRepo, err = gitrepo.OpenRepository(ctx, headRepo) - if err != nil { - ctx.ServerError("OpenRepository", err) - return nil - } - defer headGitRepo.Close() + headGitRepo, err := gitrepo.RepositoryFromRequestContextOrOpen(ctx, headRepo) + if err != nil { + ctx.ServerError("OpenRepository", err) + return nil, err } + headRef := headGitRepo.UnstableGuessRefByShortName(headRefName) if headRef == "" { - ctx.NotFound(nil) - return nil + return nil, util.NewNotExistErrorf("no head ref: %s", headRefName) } ctx.Data["BaseName"] = baseRepo.OwnerName @@ -291,12 +266,9 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo { var rootRepo *repo_model.Repository if baseRepo.IsFork { err = baseRepo.GetBaseRepo(ctx) - if err != nil { - if !repo_model.IsErrRepoNotExist(err) { - ctx.ServerError("Unable to find root repo", err) - return nil - } - } else { + if err != nil && !repo_model.IsErrRepoNotExist(err) { + return nil, err + } else if err == nil { rootRepo = baseRepo.BaseRepo } } @@ -313,42 +285,10 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo { } } - has := headRepo != nil - // 3. If the base is a forked from "RootRepo" and the owner of - // the "RootRepo" is the :headUser - set headRepo to that - if !has && rootRepo != nil && rootRepo.OwnerID == headOwner.ID { - headRepo = rootRepo - has = true - } - - // 4. If the ctx.Doer has their own fork of the baseRepo and the headUser is the ctx.Doer - // set the headRepo to the ownFork - if !has && ownForkRepo != nil && ownForkRepo.OwnerID == headOwner.ID { - headRepo = ownForkRepo - has = true - } - - // 5. If the headOwner has a fork of the baseRepo - use that - if !has { - headRepo = repo_model.GetForkedRepo(ctx, headOwner.ID, baseRepo.ID) - has = headRepo != nil - } - - // 6. If the baseRepo is a fork and the headUser has a fork of that use that - if !has && baseRepo.IsFork { - headRepo = repo_model.GetForkedRepo(ctx, headOwner.ID, baseRepo.ForkID) - has = headRepo != nil - } - - // 7. Otherwise if we're not the same repo and haven't found a repo give up - if !isSameRepo && !has { - ctx.Data["PageIsComparePull"] = false - } - ctx.Data["HeadRepo"] = headRepo ctx.Data["BaseCompareRepo"] = ctx.Repo.Repository - // If we have a rootRepo and it's different from: + // If we have a rootRepo, and it's different from: // 1. the computed base // 2. the computed head // then get the branches of it @@ -361,17 +301,15 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo { if !fileOnly { branches, tags, err := getBranchesAndTagsForRepo(ctx, rootRepo) if err != nil { - ctx.ServerError("GetBranchesForRepo", err) - return nil + return nil, err } - ctx.Data["RootRepoBranches"] = branches ctx.Data["RootRepoTags"] = tags } } } - // If we have a ownForkRepo and it's different from: + // If we have a ownForkRepo, and it's different from: // 1. The computed base // 2. The computed head // 3. The rootRepo (if we have one) @@ -386,8 +324,7 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo { if !fileOnly { branches, tags, err := getBranchesAndTagsForRepo(ctx, ownForkRepo) if err != nil { - ctx.ServerError("GetBranchesForRepo", err) - return nil + return nil, err } ctx.Data["OwnForkRepoBranches"] = branches ctx.Data["OwnForkRepoTags"] = tags @@ -395,33 +332,21 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo { } } - // Treat as pull request if both references are branches - if ctx.Data["PageIsComparePull"] == nil { - ctx.Data["PageIsComparePull"] = baseRef.IsBranch() && headRef.IsBranch() && permBase.CanReadIssuesOrPulls(true) - } - - if ctx.Data["PageIsComparePull"] == true && !permBase.CanReadIssuesOrPulls(true) { - if log.IsTrace() { - log.Trace("Permission Denied: User: %-v cannot create/read pull requests in Repo: %-v\nUser in baseRepo has Permissions: %-+v", - ctx.Doer, - baseRepo, - permBase) - } - ctx.NotFound(nil) - return nil - } - compareInfo, err := git_service.GetCompareInfo(ctx, baseRepo, headRepo, headGitRepo, baseRef, headRef, compareReq.DirectComparison(), fileOnly) if err != nil { - ctx.ServerError("GetCompareInfo", err) - return nil + return nil, err } + + // Treat as pull request if both references are branches + allowCreatePullRequest := baseRef.IsBranch() && headRef.IsBranch() && permBase.CanReadIssuesOrPulls(true) + allowCreatePullRequest = allowCreatePullRequest && compareInfo.MergeBase != "" + ctx.Data["PageIsComparePull"] = allowCreatePullRequest if compareReq.DirectComparison() { ctx.Data["BeforeCommitID"] = compareInfo.BaseCommitID } else { ctx.Data["BeforeCommitID"] = compareInfo.MergeBase } - return &compareInfo + return &compareInfo, nil } func prepareNewPullRequestTitleContent(ci *git_service.CompareInfo, commits []*git_model.SignCommitWithStatuses) (title, content string) { @@ -454,12 +379,11 @@ func prepareNewPullRequestTitleContent(ci *git_service.CompareInfo, commits []*g return title, content } -// PrepareCompareDiff renders compare diff page -func PrepareCompareDiff( - ctx *context.Context, - ci *git_service.CompareInfo, - whitespaceBehavior gitcmd.TrustedCmdArgs, -) (nothingToCompare bool) { +// prepareCompareDiff renders compare diff page. TODO: need to refactor it and other "compare diff" related functions together +func prepareCompareDiff(ctx *context.Context, ci *git_service.CompareInfo, whitespaceBehavior gitcmd.TrustedCmdArgs) (nothingToCompare bool) { + if ci.MergeBase == "" { + return true + } repo := ctx.Repo.Repository headCommitID := ci.HeadCommitID @@ -568,9 +492,6 @@ func PrepareCompareDiff( ctx.Data["CommitCount"] = len(commits) ctx.Data["title"], ctx.Data["content"] = prepareNewPullRequestTitleContent(ci, commits) - ctx.Data["Username"] = ci.HeadRepo.OwnerName - ctx.Data["Reponame"] = ci.HeadRepo.Name - setCompareContext(ctx, beforeCommit, headCommit, ci.HeadRepo.OwnerName, repo.Name) return false @@ -594,16 +515,24 @@ func getBranchesAndTagsForRepo(ctx gocontext.Context, repo *repo_model.Repositor // CompareDiff show different from one commit to another commit func CompareDiff(ctx *context.Context) { - ci := ParseCompareInfo(ctx) + ci, err := parseCompareInfo(ctx) if ctx.Written() { return } + if errors.Is(err, util.ErrNotExist) || errors.Is(err, util.ErrInvalidArgument) { + ctx.NotFound(nil) + return + } else if err != nil { + ctx.ServerError("ParseCompareInfo", err) + return + } ctx.Data["PageIsViewCode"] = true ctx.Data["PullRequestWorkInProgressPrefixes"] = setting.Repository.PullRequest.WorkInProgressPrefixes ctx.Data["CompareInfo"] = ci - nothingToCompare := PrepareCompareDiff(ctx, ci, gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string))) + // TODO: need to refactor "prepare compare" related functions together + nothingToCompare := prepareCompareDiff(ctx, ci, gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string))) if ctx.Written() { return } @@ -621,16 +550,13 @@ func CompareDiff(ctx *context.Context) { return } - headBranches, err := git_model.FindBranchNames(ctx, git_model.FindBranchOptions{ - RepoID: ci.HeadRepo.ID, - ListOptions: db.ListOptionsAll, - IsDeletedBranch: optional.Some(false), - }) + headBranches, headTags, err := getBranchesAndTagsForRepo(ctx, ci.HeadRepo) if err != nil { - ctx.ServerError("GetBranches", err) + ctx.ServerError("GetBranchesAndTagsForRepo", err) return } ctx.Data["HeadBranches"] = headBranches + ctx.Data["HeadTags"] = headTags // For compare repo branches PrepareBranchList(ctx) @@ -638,13 +564,20 @@ func CompareDiff(ctx *context.Context) { return } - headTags, err := repo_model.GetTagNamesByRepoID(ctx, ci.HeadRepo.ID) - if err != nil { - ctx.ServerError("GetTagNamesByRepoID", err) - return + if ci.MergeBase != "" { + prepareCreatePullRequestPage(ctx, ci, nothingToCompare) + if ctx.Written() { + return + } + } else { + ctx.Flash.Error(ctx.Tr("repo.pulls.no_common_history"), true) + ctx.Data["PageIsComparePull"] = false + ctx.Data["CommitCount"] = 0 } - ctx.Data["HeadTags"] = headTags + ctx.HTML(http.StatusOK, tplCompare) +} +func prepareCreatePullRequestPage(ctx *context.Context, ci *git_service.CompareInfo, nothingToCompare bool) { if ctx.Data["PageIsComparePull"] == true { pr, err := issues_model.GetUnmergedPullRequest(ctx, ci.HeadRepo.ID, ctx.Repo.Repository.ID, ci.HeadRef.ShortName(), ci.BaseRef.ShortName(), issues_model.PullRequestFlowGithub) if err != nil { @@ -685,7 +618,7 @@ func CompareDiff(ctx *context.Context) { if content, ok := ctx.Data["content"].(string); ok && content != "" { // If a template content is set, prepend the "content". In this case that's only // applicable if you have one commit to compare and that commit has a message. - // In that case the commit message will be prepend to the template body. + // In that case the commit message will be prepended to the template body. if templateContent, ok := ctx.Data[pullRequestTemplateKey].(string); ok && templateContent != "" { // Re-use the same key as that's prioritized over the "content" key. // Add two new lines between the content to ensure there's always at least @@ -713,14 +646,8 @@ func CompareDiff(ctx *context.Context) { ctx.Data["HasIssuesOrPullsWritePermission"] = ctx.Repo.Permission.CanWrite(unit.TypePullRequests) - if unit, err := ctx.Repo.Repository.GetUnit(ctx, unit.TypePullRequests); err == nil { - config := unit.PullRequestsConfig() - ctx.Data["AllowMaintainerEdit"] = config.DefaultAllowMaintainerEdit - } else { - ctx.Data["AllowMaintainerEdit"] = false - } - - ctx.HTML(http.StatusOK, tplCompare) + prConfig := ctx.Repo.Repository.MustGetUnit(ctx, unit.TypePullRequests).PullRequestsConfig() + ctx.Data["AllowMaintainerEdit"] = prConfig.DefaultAllowMaintainerEdit } // attachCommentsToLines attaches comments to their corresponding diff lines diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 7b31a26d6f..805fa6f419 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1281,24 +1281,26 @@ func PullsNewRedirect(ctx *context.Context) { // CompareAndPullRequestPost response for creating pull request func CompareAndPullRequestPost(ctx *context.Context) { form := web.GetForm(ctx).(*forms.CreateIssueForm) - ctx.Data["Title"] = ctx.Tr("repo.pulls.compare_changes") - ctx.Data["PageIsComparePull"] = true - ctx.Data["IsDiffCompare"] = true - ctx.Data["PullRequestWorkInProgressPrefixes"] = setting.Repository.PullRequest.WorkInProgressPrefixes - ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled - upload.AddUploadContext(ctx, "comment") - ctx.Data["HasIssuesOrPullsWritePermission"] = ctx.Repo.Permission.CanWrite(unit.TypePullRequests) + repo := ctx.Repo.Repository - var ( - repo = ctx.Repo.Repository - attachments []string - ) - - ci := ParseCompareInfo(ctx) + ci, err := parseCompareInfo(ctx) if ctx.Written() { return } - + if errors.Is(err, util.ErrNotExist) { + ctx.JSONErrorNotFound() + return + } else if errors.Is(err, util.ErrInvalidArgument) { + ctx.JSONError(err.Error()) + return + } else if err != nil { + ctx.ServerError("ParseCompareInfo", err) + return + } + if ci.MergeBase == "" { + ctx.JSONError(ctx.Tr("repo.pulls.no_common_history")) + return + } validateRet := ValidateRepoMetasForNewIssue(ctx, *form, true) if ctx.Written() { return @@ -1306,6 +1308,7 @@ func CompareAndPullRequestPost(ctx *context.Context) { labelIDs, assigneeIDs, milestoneID, projectID := validateRet.LabelIDs, validateRet.AssigneeIDs, validateRet.MilestoneID, validateRet.ProjectID + var attachments []string if setting.Attachment.Enabled { attachments = form.Files } diff --git a/services/git/compare.go b/services/git/compare.go index a8c2980112..40a0eaac08 100644 --- a/services/git/compare.go +++ b/services/git/compare.go @@ -45,6 +45,7 @@ func (ci *CompareInfo) DirectComparison() bool { // GetCompareInfo generates and returns compare information between base and head branches of repositories. // It does its best to fill the fields as many as it can. +// MergeBase can be empty if the base and head are unrelated. func GetCompareInfo(ctx context.Context, baseRepo, headRepo *repo_model.Repository, headGitRepo *git.Repository, baseRef, headRef git.RefName, directComparison, fileOnly bool) (compareInfo CompareInfo, err error) { baseCommitID, err1 := gitrepo.GetFullCommitID(ctx, baseRepo, baseRef.String()) headCommitID, err2 := gitrepo.GetFullCommitID(ctx, headRepo, headRef.String()) @@ -75,13 +76,17 @@ func GetCompareInfo(ctx context.Context, baseRepo, headRepo *repo_model.Reposito if !directComparison { compareInfo.MergeBase, err = gitrepo.MergeBase(ctx, headRepo, compareInfo.BaseCommitID, compareInfo.HeadCommitID) - if err != nil { + if err != nil && !errors.Is(err, util.ErrNotExist) { return compareInfo, fmt.Errorf("MergeBase: %w", err) } } else { compareInfo.MergeBase = compareInfo.BaseCommitID } + if compareInfo.MergeBase == "" { + return compareInfo, nil + } + // We have a common base - therefore we know that ... should work if !fileOnly { // In git log/rev-list, the "..." syntax represents the symmetric difference between two references, @@ -92,12 +97,10 @@ func GetCompareInfo(ctx context.Context, baseRepo, headRepo *repo_model.Reposito if err != nil { return compareInfo, fmt.Errorf("ShowPrettyFormatLogToList: %w", err) } - } else { - compareInfo.Commits = []*git.Commit{} } // Count number of changed files. - // This probably should be removed as we need to use shortstat elsewhere + // TODO: This probably should be removed as we need to use shortstat elsewhere // Now there is git diff --shortstat but this appears to be slower than simply iterating with --nameonly compareInfo.NumFiles, err = headGitRepo.GetDiffNumChangedFiles(compareInfo.BaseCommitID, compareInfo.HeadCommitID, directComparison) return compareInfo, err diff --git a/templates/repo/diff/compare.tmpl b/templates/repo/diff/compare.tmpl index afd44f26a4..9ed4b73174 100644 --- a/templates/repo/diff/compare.tmpl +++ b/templates/repo/diff/compare.tmpl @@ -13,6 +13,7 @@ {{ctx.Locale.Tr "action.compare_commits_general"}} {{end}} + {{template "base/alert" .}} {{$BaseCompareName := $.Repository.FullName -}} {{$HeadCompareName := $.HeadRepo.FullName -}} {{$OwnForkCompareName := "" -}} diff --git a/tests/integration/compare_test.go b/tests/integration/compare_test.go index 3f0034c86b..5e50fcf043 100644 --- a/tests/integration/compare_test.go +++ b/tests/integration/compare_test.go @@ -10,13 +10,16 @@ import ( "strings" "testing" + repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git/gitcmd" "code.gitea.io/gitea/modules/test" repo_service "code.gitea.io/gitea/services/repository" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestCompareTag(t *testing.T) { @@ -124,6 +127,38 @@ func TestCompareBranches(t *testing.T) { inspectCompare(t, htmlDoc, diffCount, diffChanges) } +func TestCompareBranchesNoCommonMergeBase(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user2"}) + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: user2.ID, Name: "repo1"}) + + repoPath := repo_model.RepoPath(user2.Name, repo1.Name) + _, _, runErr := gitcmd.NewCommand("fast-import").WithDir(repoPath).WithStdinBytes([]byte(strings.TrimSpace(` +commit refs/heads/unrelated-history +committer User 1714310400 +0000 +data 13 +Second commit +M 100644 inline file2.txt +data 12 +Hello from 2 +`))).RunStdString(t.Context()) + require.NoError(t, runErr) + + session := loginUser(t, "user2") + req := NewRequest(t, "GET", "/user2/repo1/compare/master...unrelated-history") + resp := session.MakeRequest(t, req, http.StatusOK) + body := resp.Body.String() + htmlDoc := NewHTMLParser(t, resp.Body) + + selection := htmlDoc.doc.Find(".ui.dropdown.select-branch") + assert.Lenf(t, selection.Nodes, 2, "The template has changed") + assert.Contains(t, body, "These branches do not share a common merge base") + assert.Equal(t, 1, htmlDoc.doc.Find(`a.item[href="/user2/repo1/compare/master...unrelated-history"]`).Length()) + assert.Equal(t, 1, htmlDoc.doc.Find(`a.item[href="/user2/repo1/compare/master...master"]`).Length()) + assert.Equal(t, 0, htmlDoc.doc.Find(".pullrequest-form").Length()) +} + func TestCompareCodeExpand(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) From 0ba862cb9779a6c720f5031c4838427ddf90f86f Mon Sep 17 00:00:00 2001 From: 0xGREG <28388707+0xGREG@users.noreply.github.com> Date: Tue, 28 Apr 2026 23:33:20 +0200 Subject: [PATCH 3/3] Add DEFAULT_TITLE_SOURCE setting for pull request title default behavior (#37465) Adds a new `DEFAULT_TITLE_SOURCE` option under `[repository.pull-request]` with three values: - `first-commit` (default): uses the oldest commit summary, current behavior since v1.26 - `auto`: normalizes branch name as title for multi-commit PRs (just like GitHub), use commit summary for single-commit PRs Closes: #37463 Co-authored-by: silverwind Co-authored-by: Claude (Opus 4.7) Co-authored-by: wxiaoguang Co-authored-by: Giteabot Co-authored-by: Nicolas --- custom/conf/app.example.ini | 5 +++ modules/setting/repository.go | 9 +++++ routers/web/repo/compare.go | 45 ++++++++++++++++++++-- routers/web/repo/compare_test.go | 66 ++++++++++++++++++++++++-------- 4 files changed, 106 insertions(+), 19 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 97af5fa5fb..4245957191 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1169,6 +1169,11 @@ LEVEL = Info ;; 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 ;; +;; Default source for the pull request title when opening a new PR. +;; "first-commit" uses the oldest commit's summary. +;; "auto" uses commit's summary if the PR only has one commit, normalizes the branch name if multiple commits. +;DEFAULT_TITLE_SOURCE = first-commit +;; ;; Delay mergeable check until page view or API access, for pull requests that have not been updated in the specified days when their base branches get updated. ;; Use "-1" to always check all pull requests (old behavior). Use "0" to always delay the checks. ;DELAY_CHECK_FOR_INACTIVE_DAYS = 7 diff --git a/modules/setting/repository.go b/modules/setting/repository.go index 9195b7ee50..a8bc91c089 100644 --- a/modules/setting/repository.go +++ b/modules/setting/repository.go @@ -18,6 +18,12 @@ const ( RepoCreatingPublic = "public" ) +// enumerates the values for [repository.pull-request] DEFAULT_TITLE_SOURCE +const ( + RepoPRTitleSourceFirstCommit = "first-commit" + RepoPRTitleSourceAuto = "auto" +) + // ItemsPerPage maximum items per page in forks, watchers and stars of a repo const ItemsPerPage = 40 @@ -89,6 +95,7 @@ var ( RetargetChildrenOnMerge bool DelayCheckForInactiveDays int DefaultDeleteBranchAfterMerge bool + DefaultTitleSource string } `ini:"repository.pull-request"` // Issue Setting @@ -213,6 +220,7 @@ var ( RetargetChildrenOnMerge bool DelayCheckForInactiveDays int DefaultDeleteBranchAfterMerge bool + DefaultTitleSource string }{ WorkInProgressPrefixes: []string{"WIP:", "[WIP]"}, // Same as GitHub. See @@ -229,6 +237,7 @@ var ( AddCoCommitterTrailers: true, RetargetChildrenOnMerge: true, DelayCheckForInactiveDays: 7, + DefaultTitleSource: RepoPRTitleSourceFirstCommit, }, // Issue settings diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index f37c9ef2d1..174cb1be22 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -13,6 +13,7 @@ import ( "path/filepath" "sort" "strings" + "unicode" "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" @@ -349,13 +350,46 @@ func parseCompareInfo(ctx *context.Context) (*git_service.CompareInfo, error) { return &compareInfo, nil } -func prepareNewPullRequestTitleContent(ci *git_service.CompareInfo, commits []*git_model.SignCommitWithStatuses) (title, content string) { - title = ci.HeadRef.ShortName() +// autoTitleFromBranchName humanizes a branch name into a PR title. +func autoTitleFromBranchName(name string) string { + var buf strings.Builder + var prevIsSpace bool + runes := []rune(name) + for i, r := range runes { + isSpace := unicode.IsSpace(r) + if r == '-' || r == '_' || isSpace { + if !prevIsSpace { + buf.WriteRune(' ') + } + prevIsSpace = true + continue + } + if !prevIsSpace && unicode.IsUpper(r) { + needSpace := i > 0 && unicode.IsLower(runes[i-1]) || i < len(runes)-1 && unicode.IsLower(runes[i+1]) + if needSpace { + buf.WriteRune(' ') + } + } + buf.WriteRune(unicode.ToLower(r)) + prevIsSpace = isSpace + } + out := strings.TrimSpace(buf.String()) + if out == "" { + return out + } + outRunes := []rune(out) + outRunes[0] = unicode.ToUpper(outRunes[0]) + return string(outRunes) +} - if len(commits) > 0 { +func prepareNewPullRequestTitleContent(ci *git_service.CompareInfo, commits []*git_model.SignCommitWithStatuses, defaultTitleSource string) (title, content string) { + useFirstCommitAsTitle := len(commits) == 1 || (defaultTitleSource == setting.RepoPRTitleSourceFirstCommit && len(commits) > 0) + if useFirstCommitAsTitle { // the "commits" are from "ShowPrettyFormatLogToList", which is ordered from newest to oldest, here take the oldest one c := commits[len(commits)-1] title = strings.TrimSpace(c.UserCommit.Summary()) + } else { + title = autoTitleFromBranchName(ci.HeadRef.ShortName()) } if len(commits) == 1 { @@ -491,7 +525,10 @@ func prepareCompareDiff(ctx *context.Context, ci *git_service.CompareInfo, white ctx.Data["Commits"] = commits ctx.Data["CommitCount"] = len(commits) - ctx.Data["title"], ctx.Data["content"] = prepareNewPullRequestTitleContent(ci, commits) + ctx.Data["title"], ctx.Data["content"] = prepareNewPullRequestTitleContent(ci, commits, setting.Repository.PullRequest.DefaultTitleSource) + ctx.Data["Username"] = ci.HeadRepo.OwnerName + ctx.Data["Reponame"] = ci.HeadRepo.Name + setCompareContext(ctx, beforeCommit, headCommit, ci.HeadRepo.OwnerName, repo.Name) return false diff --git a/routers/web/repo/compare_test.go b/routers/web/repo/compare_test.go index 700aba8821..63b0f287e5 100644 --- a/routers/web/repo/compare_test.go +++ b/routers/web/repo/compare_test.go @@ -13,6 +13,7 @@ import ( issues_model "code.gitea.io/gitea/models/issues" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/setting" git_service "code.gitea.io/gitea/services/git" "code.gitea.io/gitea/services/gitdiff" @@ -61,31 +62,66 @@ func TestNewPullRequestTitleContent(t *testing.T) { } } - title, content := prepareNewPullRequestTitleContent(ci, nil) - assert.Equal(t, "head-branch", title) + // no commit + title, content := prepareNewPullRequestTitleContent(ci, nil, setting.RepoPRTitleSourceAuto) + assert.Equal(t, "Head branch", title) assert.Empty(t, content) - title, content = prepareNewPullRequestTitleContent(ci, []*git_model.SignCommitWithStatuses{mockCommit("title-only")}) - assert.Equal(t, "title-only", title) + title, content = prepareNewPullRequestTitleContent(ci, nil, setting.RepoPRTitleSourceFirstCommit) + assert.Equal(t, "Head branch", title) assert.Empty(t, content) - title, content = prepareNewPullRequestTitleContent(ci, []*git_model.SignCommitWithStatuses{mockCommit("title-" + strings.Repeat("a", 255))}) - assert.Equal(t, "title-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa…", title) - assert.Equal(t, "…aaaaaaaaa\n", content) - - title, content = prepareNewPullRequestTitleContent(ci, []*git_model.SignCommitWithStatuses{mockCommit("title\nbody")}) - assert.Equal(t, "title", title) + // single commit + title, content = prepareNewPullRequestTitleContent(ci, []*git_model.SignCommitWithStatuses{mockCommit("single-commit-title\nbody")}, setting.RepoPRTitleSourceAuto) + assert.Equal(t, "single-commit-title", title) assert.Equal(t, "body", content) - title, content = prepareNewPullRequestTitleContent(ci, []*git_model.SignCommitWithStatuses{mockCommit("a\xf0\xf0\xf0\nb\xf0\xf0\xf0")}) - assert.Equal(t, "a?", title) // FIXME: GIT-COMMIT-MESSAGE-ENCODING: "title" doesn't use the same charset converting logic as "content" - assert.Equal(t, "b"+string(utf8.RuneError)+string(utf8.RuneError), content) + title, content = prepareNewPullRequestTitleContent(ci, []*git_model.SignCommitWithStatuses{mockCommit("single-commit-title\nbody")}, setting.RepoPRTitleSourceFirstCommit) + assert.Equal(t, "single-commit-title", title) + assert.Equal(t, "body", content) - title, content = prepareNewPullRequestTitleContent(ci, []*git_model.SignCommitWithStatuses{ + // multiple commits + commits := []*git_model.SignCommitWithStatuses{ // ordered from newest to oldest mockCommit("title2\nbody2"), mockCommit("title1\nbody1"), - }) + } + title, content = prepareNewPullRequestTitleContent(ci, commits, setting.RepoPRTitleSourceAuto) + assert.Equal(t, "Head branch", title) + assert.Empty(t, content) + + title, content = prepareNewPullRequestTitleContent(ci, commits, setting.RepoPRTitleSourceFirstCommit) assert.Equal(t, "title1", title) assert.Empty(t, content) + + // title string handling + title, content = prepareNewPullRequestTitleContent(ci, []*git_model.SignCommitWithStatuses{mockCommit("title-" + strings.Repeat("a", 255))}, setting.RepoPRTitleSourceFirstCommit) + assert.Equal(t, "title-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa…", title) + assert.Equal(t, "…aaaaaaaaa\n", content) + + title, content = prepareNewPullRequestTitleContent(ci, []*git_model.SignCommitWithStatuses{mockCommit("a\xf0\xf0\xf0\nb\xf0\xf0\xf0")}, setting.RepoPRTitleSourceFirstCommit) + assert.Equal(t, "a?", title) // FIXME: GIT-COMMIT-MESSAGE-ENCODING: "title" doesn't use the same charset converting logic as "content" + assert.Equal(t, "b"+string(utf8.RuneError)+string(utf8.RuneError), content) +} + +func TestAutoTitleFromBranchName(t *testing.T) { + cases := []struct { + branch string + want string + }{ + {"fix/the-bug", "Fix/the bug"}, + {"Already-Capitalized", "Already capitalized"}, + {"ALL-CAPS-BRANCH", "All caps branch"}, + {"FixHTMLBug", "Fix html bug"}, + {"MixedCase-Name", "Mixed case name"}, + {"fooBar-baz", "Foo bar baz"}, + {"foo/BAR", "Foo/bar"}, + {"_leading-underscore", "Leading underscore"}, + {"CamelCase", "Camel case"}, + {"foo--double-dash", "Foo double dash"}, + {"123-fix", "123 fix"}, + } + for _, c := range cases { + assert.Equal(t, c.want, autoTitleFromBranchName(c.branch), "branch: %q", c.branch) + } }