Skip to content

Commit

Permalink
Refactor context flash msg and global variables (#33375)
Browse files Browse the repository at this point in the history
1. add `GetSiteCookieFlashMessage` to help to parse flash message
2. clarify `handleRepoHomeFeed` logic
3. remove unnecessary global variables, use `sync.OnceValue` instead
4. add some tests for `IsUsableUsername` and `IsUsableRepoName`
  • Loading branch information
wxiaoguang authored Jan 25, 2025
1 parent 6a516a0 commit 2c1ff87
Show file tree
Hide file tree
Showing 30 changed files with 667 additions and 606 deletions.
4 changes: 1 addition & 3 deletions models/db/name.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ import (
"code.gitea.io/gitea/modules/util"
)

var ErrNameEmpty = util.SilentWrap{Message: "name is empty", Err: util.ErrInvalidArgument}

// ErrNameReserved represents a "reserved name" error.
type ErrNameReserved struct {
Name string
Expand Down Expand Up @@ -79,7 +77,7 @@ func (err ErrNameCharsNotAllowed) Unwrap() error {
func IsUsableName(reservedNames, reservedPatterns []string, name string) error {
name = strings.TrimSpace(strings.ToLower(name))
if utf8.RuneCountInString(name) == 0 {
return ErrNameEmpty
return util.SilentWrap{Message: "name is empty", Err: util.ErrInvalidArgument}
}

for i := range reservedNames {
Expand Down
27 changes: 19 additions & 8 deletions models/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"regexp"
"strconv"
"strings"
"sync"

"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/unit"
Expand Down Expand Up @@ -61,20 +62,30 @@ func (err ErrRepoIsArchived) Error() string {
return fmt.Sprintf("%s is archived", err.Repo.LogString())
}

var (
validRepoNamePattern = regexp.MustCompile(`[-.\w]+`)
invalidRepoNamePattern = regexp.MustCompile(`[.]{2,}`)
reservedRepoNames = []string{".", "..", "-"}
reservedRepoPatterns = []string{"*.git", "*.wiki", "*.rss", "*.atom"}
)
type globalVarsStruct struct {
validRepoNamePattern *regexp.Regexp
invalidRepoNamePattern *regexp.Regexp
reservedRepoNames []string
reservedRepoPatterns []string
}

var globalVars = sync.OnceValue(func() *globalVarsStruct {
return &globalVarsStruct{
validRepoNamePattern: regexp.MustCompile(`[-.\w]+`),
invalidRepoNamePattern: regexp.MustCompile(`[.]{2,}`),
reservedRepoNames: []string{".", "..", "-"},
reservedRepoPatterns: []string{"*.git", "*.wiki", "*.rss", "*.atom"},
}
})

// IsUsableRepoName returns true when name is usable
func IsUsableRepoName(name string) error {
if !validRepoNamePattern.MatchString(name) || invalidRepoNamePattern.MatchString(name) {
vars := globalVars()
if !vars.validRepoNamePattern.MatchString(name) || vars.invalidRepoNamePattern.MatchString(name) {
// Note: usually this error is normally caught up earlier in the UI
return db.ErrNameCharsNotAllowed{Name: name}
}
return db.IsUsableName(reservedRepoNames, reservedRepoPatterns, name)
return db.IsUsableName(vars.reservedRepoNames, vars.reservedRepoPatterns, name)
}

// TrustModelType defines the types of trust model for this repository
Expand Down
1 change: 1 addition & 0 deletions models/repo/repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,4 +219,5 @@ func TestIsUsableRepoName(t *testing.T) {
assert.Error(t, IsUsableRepoName("the..repo"))
assert.Error(t, IsUsableRepoName("foo.wiki"))
assert.Error(t, IsUsableRepoName("foo.git"))
assert.Error(t, IsUsableRepoName("foo.RSS"))
}
8 changes: 4 additions & 4 deletions models/user/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -502,10 +502,10 @@ func (u *User) IsMailable() bool {
return u.IsActive
}

// IsUserExist checks if given user name exist,
// the user name should be noncased unique.
// IsUserExist checks if given username exist,
// the username should be non-cased unique.
// If uid is presented, then check will rule out that one,
// it is used when update a user name in settings page.
// it is used when update a username in settings page.
func IsUserExist(ctx context.Context, uid int64, name string) (bool, error) {
if len(name) == 0 {
return false, nil
Expand All @@ -515,7 +515,7 @@ func IsUserExist(ctx context.Context, uid int64, name string) (bool, error) {
Get(&User{LowerName: strings.ToLower(name)})
}

// Note: As of the beginning of 2022, it is recommended to use at least
// SaltByteLength as of the beginning of 2022, it is recommended to use at least
// 64 bits of salt, but NIST is already recommending to use to 128 bits.
// (16 bytes = 16 * 8 = 128 bits)
const SaltByteLength = 16
Expand Down
15 changes: 15 additions & 0 deletions models/user/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,21 @@ import (
"github.com/stretchr/testify/assert"
)

func TestIsUsableUsername(t *testing.T) {
assert.NoError(t, user_model.IsUsableUsername("a"))
assert.NoError(t, user_model.IsUsableUsername("foo.wiki"))
assert.NoError(t, user_model.IsUsableUsername("foo.git"))

assert.Error(t, user_model.IsUsableUsername("a--b"))
assert.Error(t, user_model.IsUsableUsername("-1_."))
assert.Error(t, user_model.IsUsableUsername(".profile"))
assert.Error(t, user_model.IsUsableUsername("-"))
assert.Error(t, user_model.IsUsableUsername("🌞"))
assert.Error(t, user_model.IsUsableUsername("the..repo"))
assert.Error(t, user_model.IsUsableUsername("foo.RSS"))
assert.Error(t, user_model.IsUsableUsername("foo.PnG"))
}

func TestOAuth2Application_LoadUser(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
app := unittest.AssertExistsAndLoadBean(t, &auth.OAuth2Application{ID: 1})
Expand Down
56 changes: 28 additions & 28 deletions modules/validation/glob_pattern_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,39 +19,39 @@ func getGlobPatternErrorString(pattern string) string {
return ""
}

var globValidationTestCases = []validationTestCase{
{
description: "Empty glob pattern",
data: TestForm{
GlobPattern: "",
},
expectedErrors: binding.Errors{},
},
{
description: "Valid glob",
data: TestForm{
GlobPattern: "{master,release*}",
},
expectedErrors: binding.Errors{},
},
func Test_GlobPatternValidation(t *testing.T) {
AddBindingRules()

{
description: "Invalid glob",
data: TestForm{
GlobPattern: "[a-",
globValidationTestCases := []validationTestCase{
{
description: "Empty glob pattern",
data: TestForm{
GlobPattern: "",
},
expectedErrors: binding.Errors{},
},
expectedErrors: binding.Errors{
binding.Error{
FieldNames: []string{"GlobPattern"},
Classification: ErrGlobPattern,
Message: getGlobPatternErrorString("[a-"),
{
description: "Valid glob",
data: TestForm{
GlobPattern: "{master,release*}",
},
expectedErrors: binding.Errors{},
},
},
}

func Test_GlobPatternValidation(t *testing.T) {
AddBindingRules()
{
description: "Invalid glob",
data: TestForm{
GlobPattern: "[a-",
},
expectedErrors: binding.Errors{
binding.Error{
FieldNames: []string{"GlobPattern"},
Classification: ErrGlobPattern,
Message: getGlobPatternErrorString("[a-"),
},
},
},
}

for _, testCase := range globValidationTestCases {
t.Run(testCase.description, func(t *testing.T) {
Expand Down
27 changes: 18 additions & 9 deletions modules/validation/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,26 @@ import (
"net/url"
"regexp"
"strings"
"sync"

"code.gitea.io/gitea/modules/setting"

"github.com/gobwas/glob"
)

var externalTrackerRegex = regexp.MustCompile(`({?)(?:user|repo|index)+?(}?)`)
type globalVarsStruct struct {
externalTrackerRegex *regexp.Regexp
validUsernamePattern *regexp.Regexp
invalidUsernamePattern *regexp.Regexp
}

var globalVars = sync.OnceValue(func() *globalVarsStruct {
return &globalVarsStruct{
externalTrackerRegex: regexp.MustCompile(`({?)(?:user|repo|index)+?(}?)`),
validUsernamePattern: regexp.MustCompile(`^[\da-zA-Z][-.\w]*$`),
invalidUsernamePattern: regexp.MustCompile(`[-._]{2,}|[-._]$`), // No consecutive or trailing non-alphanumeric chars
}
})

func isLoopbackIP(ip string) bool {
return net.ParseIP(ip).IsLoopback()
Expand Down Expand Up @@ -105,9 +118,9 @@ func IsValidExternalTrackerURLFormat(uri string) bool {
if !IsValidExternalURL(uri) {
return false
}

vars := globalVars()
// check for typoed variables like /{index/ or /[repo}
for _, match := range externalTrackerRegex.FindAllStringSubmatch(uri, -1) {
for _, match := range vars.externalTrackerRegex.FindAllStringSubmatch(uri, -1) {
if (match[1] == "{" || match[2] == "}") && (match[1] != "{" || match[2] != "}") {
return false
}
Expand All @@ -116,14 +129,10 @@ func IsValidExternalTrackerURLFormat(uri string) bool {
return true
}

var (
validUsernamePattern = regexp.MustCompile(`^[\da-zA-Z][-.\w]*$`)
invalidUsernamePattern = regexp.MustCompile(`[-._]{2,}|[-._]$`) // No consecutive or trailing non-alphanumeric chars
)

// IsValidUsername checks if username is valid
func IsValidUsername(name string) bool {
// It is difficult to find a single pattern that is both readable and effective,
// but it's easier to use positive and negative checks.
return validUsernamePattern.MatchString(name) && !invalidUsernamePattern.MatchString(name)
vars := globalVars()
return vars.validUsernamePattern.MatchString(name) && !vars.invalidUsernamePattern.MatchString(name)
}
Loading

0 comments on commit 2c1ff87

Please sign in to comment.