Skip to content

Commit f915161

Browse files
authored
skip email validation on empty string (#13627)
- move validation into its own function - use a session for UpdateUserSetting
1 parent 1bb5c09 commit f915161

File tree

2 files changed

+35
-17
lines changed

2 files changed

+35
-17
lines changed

models/user.go

+16-11
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"errors"
1515
"fmt"
1616
_ "image/jpeg" // Needed for jpeg support
17-
"net/mail"
1817
"os"
1918
"path/filepath"
2019
"regexp"
@@ -809,9 +808,8 @@ func CreateUser(u *User) (err error) {
809808
return ErrEmailAlreadyUsed{u.Email}
810809
}
811810

812-
_, err = mail.ParseAddress(u.Email)
813-
if err != nil {
814-
return ErrEmailInvalid{u.Email}
811+
if err = ValidateEmail(u.Email); err != nil {
812+
return err
815813
}
816814

817815
isExist, err = isEmailUsed(sess, u.Email)
@@ -956,11 +954,10 @@ func checkDupEmail(e Engine, u *User) error {
956954
return nil
957955
}
958956

959-
func updateUser(e Engine, u *User) error {
957+
func updateUser(e Engine, u *User) (err error) {
960958
u.Email = strings.ToLower(u.Email)
961-
_, err := mail.ParseAddress(u.Email)
962-
if err != nil {
963-
return ErrEmailInvalid{u.Email}
959+
if err = ValidateEmail(u.Email); err != nil {
960+
return err
964961
}
965962
_, err = e.ID(u.ID).AllCols().Update(u)
966963
return err
@@ -982,13 +979,21 @@ func updateUserCols(e Engine, u *User, cols ...string) error {
982979
}
983980

984981
// UpdateUserSetting updates user's settings.
985-
func UpdateUserSetting(u *User) error {
982+
func UpdateUserSetting(u *User) (err error) {
983+
sess := x.NewSession()
984+
defer sess.Close()
985+
if err = sess.Begin(); err != nil {
986+
return err
987+
}
986988
if !u.IsOrganization() {
987-
if err := checkDupEmail(x, u); err != nil {
989+
if err = checkDupEmail(sess, u); err != nil {
988990
return err
989991
}
990992
}
991-
return updateUser(x, u)
993+
if err = updateUser(sess, u); err != nil {
994+
return err
995+
}
996+
return sess.Commit()
992997
}
993998

994999
// deleteBeans deletes all given beans, beans should contain delete conditions.

models/user_mail.go

+19-6
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,21 @@ type EmailAddress struct {
3333
IsPrimary bool `xorm:"-"`
3434
}
3535

36+
// ValidateEmail check if email is a allowed address
37+
func ValidateEmail(email string) error {
38+
if len(email) == 0 {
39+
return nil
40+
}
41+
42+
if _, err := mail.ParseAddress(email); err != nil {
43+
return ErrEmailInvalid{email}
44+
}
45+
46+
// TODO: add an email allow/block list
47+
48+
return nil
49+
}
50+
3651
// GetEmailAddresses returns all email addresses belongs to given user.
3752
func GetEmailAddresses(uid int64) ([]*EmailAddress, error) {
3853
emails := make([]*EmailAddress, 0, 5)
@@ -144,9 +159,8 @@ func addEmailAddress(e Engine, email *EmailAddress) error {
144159
return ErrEmailAlreadyUsed{email.Email}
145160
}
146161

147-
_, err = mail.ParseAddress(email.Email)
148-
if err != nil {
149-
return ErrEmailInvalid{email.Email}
162+
if err = ValidateEmail(email.Email); err != nil {
163+
return err
150164
}
151165

152166
_, err = e.Insert(email)
@@ -173,9 +187,8 @@ func AddEmailAddresses(emails []*EmailAddress) error {
173187
} else if used {
174188
return ErrEmailAlreadyUsed{emails[i].Email}
175189
}
176-
_, err = mail.ParseAddress(emails[i].Email)
177-
if err != nil {
178-
return ErrEmailInvalid{emails[i].Email}
190+
if err = ValidateEmail(emails[i].Email); err != nil {
191+
return err
179192
}
180193
}
181194

0 commit comments

Comments
 (0)