#1070 Clearer error message for illegal characters

models/error.go

@@ -8,6 +8,32 @@ import (
 	"fmt"
 )
 
+type ErrNameReserved struct {
+	Name string
+}
+
+func IsErrNameReserved(err error) bool {
+	_, ok := err.(ErrNameReserved)
+	return ok
+}
+
+func (err ErrNameReserved) Error() string {
+	return fmt.Sprintf("name is reserved: [name: %s]", err.Name)
+}
+
+type ErrNamePatternNotAllowed struct {
+	Pattern string
+}
+
+func IsErrNamePatternNotAllowed(err error) bool {
+	_, ok := err.(ErrNamePatternNotAllowed)
+	return ok
+}
+
+func (err ErrNamePatternNotAllowed) Error() string {
+	return fmt.Sprintf("name pattern is not allowed: [pattern: %s]", err.Pattern)
+}
+
 //  ____ ___
 // |    |   \______ ___________
 // |    |   /  ___// __ \_  __ \
@@ -15,6 +41,32 @@ import (
 // |______//____  >\___  >__|
 //              \/     \/
 
+type ErrUserAlreadyExist struct {
+	Name string
+}
+
+func IsErrUserAlreadyExist(err error) bool {
+	_, ok := err.(ErrUserAlreadyExist)
+	return ok
+}
+
+func (err ErrUserAlreadyExist) Error() string {
+	return fmt.Sprintf("user already exists: [name: %s]", err.Name)
+}
+
+type ErrEmailAlreadyUsed struct {
+	Email string
+}
+
+func IsErrEmailAlreadyUsed(err error) bool {
+	_, ok := err.(ErrEmailAlreadyUsed)
+	return ok
+}
+
+func (err ErrEmailAlreadyUsed) Error() string {
+	return fmt.Sprintf("e-mail has been used: [email: %s]", err.Email)
+}
+
 type ErrUserOwnRepos struct {
 	UID int64
 }

models/org.go

@@ -105,23 +105,23 @@ func IsOrgEmailUsed(email string) (bool, error) {
 }
 
 // CreateOrganization creates record of a new organization.
-func CreateOrganization(org, owner *User) (*User, error) {
-	if !IsLegalName(org.Name) {
-		return nil, ErrUserNameIllegal
+func CreateOrganization(org, owner *User) (err error) {
+	if err = IsUsableName(org.Name); err != nil {
+		return err
 	}
 
 	isExist, err := IsUserExist(0, org.Name)
 	if err != nil {
-		return nil, err
+		return err
 	} else if isExist {
-		return nil, ErrUserAlreadyExist
+		return ErrUserAlreadyExist{org.Name}
 	}
 
 	isExist, err = IsOrgEmailUsed(org.Email)
 	if err != nil {
-		return nil, err
+		return err
 	} else if isExist {
-		return nil, ErrEmailAlreadyUsed
+		return ErrEmailAlreadyUsed{org.Email}
 	}
 
 	org.LowerName = strings.ToLower(org.Name)
@@ -135,11 +135,11 @@ func CreateOrganization(org, owner *User) (*User, error) {
 	sess := x.NewSession()
 	defer sessionRelease(sess)
 	if err = sess.Begin(); err != nil {
-		return nil, err
+		return err
 	}
 
 	if _, err = sess.Insert(org); err != nil {
-		return nil, err
+		return fmt.Errorf("insert organization: %v", err)
 	}
 
 	// Create default owner team.
@@ -151,7 +151,7 @@ func CreateOrganization(org, owner *User) (*User, error) {
 		NumMembers: 1,
 	}
 	if _, err = sess.Insert(t); err != nil {
-		return nil, err
+		return fmt.Errorf("insert owner team: %v", err)
 	}
 
 	// Add initial creator to organization and owner team.
@@ -162,7 +162,7 @@ func CreateOrganization(org, owner *User) (*User, error) {
 		NumTeams: 1,
 	}
 	if _, err = sess.Insert(ou); err != nil {
-		return nil, err
+		return fmt.Errorf("insert org-user relation: %v", err)
 	}
 
 	tu := &TeamUser{
@@ -171,14 +171,14 @@ func CreateOrganization(org, owner *User) (*User, error) {
 		TeamID: t.ID,
 	}
 	if _, err = sess.Insert(tu); err != nil {
-		return nil, err
+		return fmt.Errorf("insert team-user relation: %v", err)
 	}
 
 	if err = os.MkdirAll(UserPath(org.Name), os.ModePerm); err != nil {
-		return nil, err
+		return fmt.Errorf("create directory: %v", err)
 	}
 
-	return org, sess.Commit()
+	return sess.Commit()
 }
 
 // GetOrgByName returns organization by given name.
@@ -594,9 +594,9 @@ func (t *Team) RemoveRepository(repoID int64) error {
 
 // NewTeam creates a record of new team.
 // It's caller's responsibility to assign organization ID.
-func NewTeam(t *Team) error {
-	if !IsLegalName(t.Name) {
-		return ErrTeamNameIllegal
+func NewTeam(t *Team) (err error) {
+	if err = IsUsableName(t.Name); err != nil {
+		return err
 	}
 
 	has, err := x.Id(t.OrgID).Get(new(User))
@@ -670,8 +670,8 @@ func GetTeamById(teamId int64) (*Team, error) {
 
 // UpdateTeam updates information of team.
 func UpdateTeam(t *Team, authChanged bool) (err error) {
-	if !IsLegalName(t.Name) {
-		return ErrTeamNameIllegal
+	if err = IsUsableName(t.Name); err != nil {
+		return err
 	}
 
 	if len(t.Description) > 255 {

models/repo.go

@@ -37,7 +37,6 @@ const (
 var (
 	ErrRepoAlreadyExist  = errors.New("Repository already exist")
 	ErrRepoFileNotExist  = errors.New("Repository file does not exist")
-	ErrRepoNameIllegal   = errors.New("Repository name contains illegal characters")
 	ErrRepoFileNotLoaded = errors.New("Repository file not loaded")
 	ErrMirrorNotExist    = errors.New("Mirror does not exist")
 	ErrInvalidReference  = errors.New("Invalid reference specified")
@@ -223,12 +222,12 @@ func (repo *Repository) DescriptionHtml() template.HTML {
 }
 
 // IsRepositoryExist returns true if the repository with given name under user has already existed.
-func IsRepositoryExist(u *User, repoName string) bool {
-	has, _ := x.Get(&Repository{
+func IsRepositoryExist(u *User, repoName string) (bool, error) {
+	has, err := x.Get(&Repository{
 		OwnerId:   u.Id,
 		LowerName: strings.ToLower(repoName),
 	})
-	return has && com.IsDir(RepoPath(u.Name, repoName))
+	return has && com.IsDir(RepoPath(u.Name, repoName)), err
 }
 
 // CloneLink represents different types of clone URLs of repository.
@@ -253,24 +252,27 @@ func (repo *Repository) CloneLink() (cl CloneLink, err error) {
 }
 
 var (
-	illegalEquals  = []string{"debug", "raw", "install", "api", "avatar", "user", "org", "help", "stars", "issues", "pulls", "commits", "repo", "template", "admin", "new"}
-	illegalSuffixs = []string{".git", ".keys"}
+	reservedNames    = []string{"debug", "raw", "install", "api", "avatar", "user", "org", "help", "stars", "issues", "pulls", "commits", "repo", "template", "admin", "new"}
+	reservedPatterns = []string{"*.git", "*.keys"}
 )
 
-// IsLegalName returns false if name contains illegal characters.
-func IsLegalName(repoName string) bool {
-	repoName = strings.ToLower(repoName)
-	for _, char := range illegalEquals {
-		if repoName == char {
-			return false
+// IsUsableName checks if name is reserved or pattern of name is not allowed.
+func IsUsableName(name string) error {
+	name = strings.ToLower(name)
+	for i := range reservedNames {
+		if name == reservedNames[i] {
+			return ErrNameReserved{name}
 		}
 	}
-	for _, char := range illegalSuffixs {
-		if strings.HasSuffix(repoName, char) {
-			return false
+
+	for _, pat := range reservedPatterns {
+		if pat[0] == '*' && strings.HasSuffix(name, pat[1:]) ||
+			(pat[len(pat)-1] == '*' && strings.HasPrefix(name, pat[:len(pat)-1])) {
+			return ErrNamePatternNotAllowed{pat}
 		}
 	}
-	return true
+
+	return nil
 }
 
 // Mirror represents a mirror information of repository.
@@ -504,11 +506,14 @@ func initRepository(e Engine, repoPath string, u *User, repo *Repository, initRe
 
 // CreateRepository creates a repository for given user or organization.
 func CreateRepository(u *User, name, desc, lang, license string, isPrivate, isMirror, initReadme bool) (_ *Repository, err error) {
-	if !IsLegalName(name) {
-		return nil, ErrRepoNameIllegal
+	if err = IsUsableName(name); err != nil {
+		return nil, err
 	}
 
-	if IsRepositoryExist(u, name) {
+	has, err := IsRepositoryExist(u, name)
+	if err != nil {
+		return nil, fmt.Errorf("IsRepositoryExist: %v", err)
+	} else if has {
 		return nil, ErrRepoAlreadyExist
 	}
 
@@ -619,7 +624,10 @@ func TransferOwnership(u *User, newOwnerName string, repo *Repository) error {
 	}
 
 	// Check if new owner has repository with same name.
-	if IsRepositoryExist(newOwner, repo.Name) {
+	has, err := IsRepositoryExist(newOwner, repo.Name)
+	if err != nil {
+		return fmt.Errorf("IsRepositoryExist: %v", err)
+	} else if has {
 		return ErrRepoAlreadyExist
 	}
 
@@ -727,16 +735,22 @@ func TransferOwnership(u *User, newOwnerName string, repo *Repository) error {
 }
 
 // ChangeRepositoryName changes all corresponding setting from old repository name to new one.
-func ChangeRepositoryName(userName, oldRepoName, newRepoName string) (err error) {
-	userName = strings.ToLower(userName)
+func ChangeRepositoryName(u *User, oldRepoName, newRepoName string) (err error) {
 	oldRepoName = strings.ToLower(oldRepoName)
 	newRepoName = strings.ToLower(newRepoName)
-	if !IsLegalName(newRepoName) {
-		return ErrRepoNameIllegal
+	if err = IsUsableName(newRepoName); err != nil {
+		return err
+	}
+
+	has, err := IsRepositoryExist(u, newRepoName)
+	if err != nil {
+		return fmt.Errorf("IsRepositoryExist: %v", err)
+	} else if has {
+		return ErrRepoAlreadyExist
 	}
 
 	// Change repository directory name.
-	return os.Rename(RepoPath(userName, oldRepoName), RepoPath(userName, newRepoName))
+	return os.Rename(RepoPath(u.LowerName, oldRepoName), RepoPath(u.LowerName, newRepoName))
 }
 
 func updateRepository(e Engine, repo *Repository, visibilityChanged bool) (err error) {
@@ -1340,7 +1354,10 @@ func IsStaring(uid, repoId int64) bool {
 //      \/                   \/
 
 func ForkRepository(u *User, oldRepo *Repository, name, desc string) (_ *Repository, err error) {
-	if IsRepositoryExist(u, name) {
+	has, err := IsRepositoryExist(u, name)
+	if err != nil {
+		return nil, fmt.Errorf("IsRepositoryExist: %v", err)
+	} else if has {
 		return nil, ErrRepoAlreadyExist
 	}
 

models/user.go

@@ -36,10 +36,8 @@ const (
 )
 
 var (
-	ErrUserAlreadyExist      = errors.New("User already exist")
 	ErrUserNotExist          = errors.New("User does not exist")
 	ErrUserNotKeyOwner       = errors.New("User does not the owner of public key")
-	ErrEmailAlreadyUsed      = errors.New("E-mail already used")
 	ErrEmailNotExist         = errors.New("E-mail does not exist")
 	ErrEmailNotActivated     = errors.New("E-mail address has not been activated")
 	ErrUserNameIllegal       = errors.New("User name contains illegal characters")
@@ -273,23 +271,23 @@ func GetUserSalt() string {
 }
 
 // CreateUser creates record of a new user.
-func CreateUser(u *User) error {
-	if !IsLegalName(u.Name) {
-		return ErrUserNameIllegal
+func CreateUser(u *User) (err error) {
+	if err = IsUsableName(u.Name); err != nil {
+		return err
 	}
 
 	isExist, err := IsUserExist(0, u.Name)
 	if err != nil {
 		return err
 	} else if isExist {
-		return ErrUserAlreadyExist
+		return ErrUserAlreadyExist{u.Name}
 	}
 
 	isExist, err = IsEmailUsed(u.Email)
 	if err != nil {
 		return err
 	} else if isExist {
-		return ErrEmailAlreadyUsed
+		return ErrEmailAlreadyUsed{u.Email}
 	}
 
 	u.LowerName = strings.ToLower(u.Name)
@@ -392,8 +390,15 @@ func VerifyActiveEmailCode(code, email string) *EmailAddress {
 
 // ChangeUserName changes all corresponding setting from old user name to new one.
 func ChangeUserName(u *User, newUserName string) (err error) {
-	if !IsLegalName(newUserName) {
-		return ErrUserNameIllegal
+	if err = IsUsableName(newUserName); err != nil {
+		return err
+	}
+
+	isExist, err := IsUserExist(0, newUserName)
+	if err != nil {
+		return err
+	} else if isExist {
+		return ErrUserAlreadyExist{newUserName}
 	}
 
 	return os.Rename(UserPath(u.LowerName), UserPath(newUserName))
@@ -405,7 +410,7 @@ func UpdateUser(u *User) error {
 	if err != nil {
 		return err
 	} else if has {
-		return ErrEmailAlreadyUsed
+		return ErrEmailAlreadyUsed{u.Email}
 	}
 
 	u.LowerName = strings.ToLower(u.Name)
@@ -641,7 +646,7 @@ func AddEmailAddress(email *EmailAddress) error {
 	if err != nil {
 		return err
 	} else if used {
-		return ErrEmailAlreadyUsed
+		return ErrEmailAlreadyUsed{email.Email}
 	}
 
 	_, err = x.Insert(email)