From de02ea74c9cb3f19f8968e9d82749277c3db6562 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauris=20Buk=C5=A1is-Haberkorns?= Date: Mon, 17 Apr 2017 02:27:06 +0300 Subject: [PATCH 1/9] Add correct git branch name validation --- cmd/web.go | 2 ++ modules/auth/auth.go | 3 ++ modules/auth/repo_form.go | 6 ++-- modules/validation/binding.go | 55 +++++++++++++++++++++++++++++++++ options/locale/locale_en-US.ini | 1 + 5 files changed, 64 insertions(+), 3 deletions(-) create mode 100644 modules/validation/binding.go diff --git a/cmd/web.go b/cmd/web.go index b2cc3959a29e6..411b50d9bf0e2 100644 --- a/cmd/web.go +++ b/cmd/web.go @@ -23,6 +23,7 @@ import ( "code.gitea.io/gitea/modules/public" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/templates" + "code.gitea.io/gitea/modules/validation" "code.gitea.io/gitea/routers" "code.gitea.io/gitea/routers/admin" apiv1 "code.gitea.io/gitea/routers/api/v1" @@ -177,6 +178,7 @@ func runWeb(ctx *cli.Context) error { reqSignOut := context.Toggle(&context.ToggleOptions{SignOutRequired: true}) bindIgnErr := binding.BindIgnErr + validation.AddBindingRules() m.Use(user.GetNotificationCount) diff --git a/modules/auth/auth.go b/modules/auth/auth.go index 33ba777966612..f41d40760b63f 100644 --- a/modules/auth/auth.go +++ b/modules/auth/auth.go @@ -19,6 +19,7 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/validation" ) // IsAPIPath if URL is an api path @@ -253,6 +254,8 @@ func validate(errs binding.Errors, data map[string]interface{}, f Form, l macaro data["ErrorMsg"] = trName + l.Tr("form.alpha_dash_error") case binding.ERR_ALPHA_DASH_DOT: data["ErrorMsg"] = trName + l.Tr("form.alpha_dash_dot_error") + case validation.ERR_GIT_REF_NAME: + data["ErrorMsg"] = trName + l.Tr("form.git_ref_name_error") case binding.ERR_SIZE: data["ErrorMsg"] = trName + l.Tr("form.size_error", GetSize(field)) case binding.ERR_MIN_SIZE: diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index dd3fbff7bf468..629999987e60e 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -323,7 +323,7 @@ type EditRepoFileForm struct { CommitSummary string `binding:"MaxSize(100)"` CommitMessage string CommitChoice string `binding:"Required;MaxSize(50)"` - NewBranchName string `binding:"AlphaDashDot;MaxSize(100)"` + NewBranchName string `binding:"GitRefName;MaxSize(100)"` LastCommit string } @@ -356,7 +356,7 @@ type UploadRepoFileForm struct { CommitSummary string `binding:"MaxSize(100)"` CommitMessage string CommitChoice string `binding:"Required;MaxSize(50)"` - NewBranchName string `binding:"AlphaDashDot;MaxSize(100)"` + NewBranchName string `binding:"GitRefName;MaxSize(100)"` Files []string } @@ -387,7 +387,7 @@ type DeleteRepoFileForm struct { CommitSummary string `binding:"MaxSize(100)"` CommitMessage string CommitChoice string `binding:"Required;MaxSize(50)"` - NewBranchName string `binding:"AlphaDashDot;MaxSize(100)"` + NewBranchName string `binding:"GitRefName;MaxSize(100)"` } // Validate validates the fields diff --git a/modules/validation/binding.go b/modules/validation/binding.go new file mode 100644 index 0000000000000..e6b5ef0ce6910 --- /dev/null +++ b/modules/validation/binding.go @@ -0,0 +1,55 @@ +// Copyright 2017 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package validation + +import ( + "fmt" + "regexp" + "strings" + + "github.com/go-macaron/binding" +) + +const ( + // ERR_GIT_REF_NAME is git reference name error + ERR_GIT_REF_NAME = "GitRefNameError" +) + +var ( + // GitRefNamePattern is regular expression wirh unallowed characters in git reference name + GitRefNamePattern = regexp.MustCompile("[^\\d\\w-_\\./]") +) + +// AddBindingRules adds additional binding rules +func AddBindingRules() { + addGitRefNameBindingRule() +} + +func addGitRefNameBindingRule() { + // Git refname validation rule + binding.AddRule(&binding.Rule{ + IsMatch: func(rule string) bool { + return strings.HasPrefix(rule, "GitRefName") + }, + IsValid: func(errs binding.Errors, name string, val interface{}) (bool, binding.Errors) { + str := fmt.Sprintf("%v", val) + + if GitRefNamePattern.MatchString(str) { + errs.Add([]string{name}, ERR_GIT_REF_NAME, "GitRefName") + return false, errs + } + // Additional rules as described at https://www.kernel.org/pub/software/scm/git/docs/git-check-ref-format.html + if strings.HasPrefix(str, "/") || strings.HasSuffix(str, "/") || + strings.HasPrefix(str, ".") || strings.HasSuffix(str, ".") || + strings.HasSuffix(str, ".lock") || + strings.Contains(str, "..") || strings.Contains(str, "//") { + errs.Add([]string{name}, ERR_GIT_REF_NAME, "GitRefName") + return false, errs + } + + return true, errs + }, + }) +} diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index cb39ec857129b..04b5f44d2c25a 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -233,6 +233,7 @@ Content = Content require_error = ` cannot be empty.` alpha_dash_error = ` must be valid alphanumeric or dash(-_) characters.` alpha_dash_dot_error = ` must be valid alphanumeric, dash(-_) or dot characters.` +git_ref_name_error = ` must be well formed git reference name.` size_error = ` must be size %s.` min_size_error = ` must contain at least %s characters.` max_size_error = ` must contain at most %s characters.` From 1a049bf715cabccdf30b64234fcae6115c6f4441 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauris=20Buk=C5=A1is-Haberkorns?= Date: Mon, 17 Apr 2017 02:38:18 +0300 Subject: [PATCH 2/9] Change git refname validation error constant name --- modules/auth/auth.go | 2 +- modules/validation/binding.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/modules/auth/auth.go b/modules/auth/auth.go index f41d40760b63f..89b3e385099e7 100644 --- a/modules/auth/auth.go +++ b/modules/auth/auth.go @@ -254,7 +254,7 @@ func validate(errs binding.Errors, data map[string]interface{}, f Form, l macaro data["ErrorMsg"] = trName + l.Tr("form.alpha_dash_error") case binding.ERR_ALPHA_DASH_DOT: data["ErrorMsg"] = trName + l.Tr("form.alpha_dash_dot_error") - case validation.ERR_GIT_REF_NAME: + case validation.ErrGitRefName: data["ErrorMsg"] = trName + l.Tr("form.git_ref_name_error") case binding.ERR_SIZE: data["ErrorMsg"] = trName + l.Tr("form.size_error", GetSize(field)) diff --git a/modules/validation/binding.go b/modules/validation/binding.go index e6b5ef0ce6910..c1880797b0555 100644 --- a/modules/validation/binding.go +++ b/modules/validation/binding.go @@ -13,8 +13,8 @@ import ( ) const ( - // ERR_GIT_REF_NAME is git reference name error - ERR_GIT_REF_NAME = "GitRefNameError" + // ErrGitRefName is git reference name error + ErrGitRefName = "GitRefNameError" ) var ( @@ -37,7 +37,7 @@ func addGitRefNameBindingRule() { str := fmt.Sprintf("%v", val) if GitRefNamePattern.MatchString(str) { - errs.Add([]string{name}, ERR_GIT_REF_NAME, "GitRefName") + errs.Add([]string{name}, ErrGitRefName, "GitRefName") return false, errs } // Additional rules as described at https://www.kernel.org/pub/software/scm/git/docs/git-check-ref-format.html @@ -45,7 +45,7 @@ func addGitRefNameBindingRule() { strings.HasPrefix(str, ".") || strings.HasSuffix(str, ".") || strings.HasSuffix(str, ".lock") || strings.Contains(str, "..") || strings.Contains(str, "//") { - errs.Add([]string{name}, ERR_GIT_REF_NAME, "GitRefName") + errs.Add([]string{name}, ErrGitRefName, "GitRefName") return false, errs } From 351c2add81d2446d6efd20074bb38cb820ec4287 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauris=20Buk=C5=A1is-Haberkorns?= Date: Mon, 17 Apr 2017 23:46:51 +0300 Subject: [PATCH 3/9] Implement URL validation based on GoLang url.Parse method --- modules/auth/admin.go | 2 +- modules/auth/org.go | 2 +- modules/auth/repo_form.go | 6 +++--- modules/auth/user_form.go | 2 +- modules/validation/binding.go | 28 ++++++++++++++++++++++++++++ 5 files changed, 34 insertions(+), 6 deletions(-) diff --git a/modules/auth/admin.go b/modules/auth/admin.go index 68bc4f394133c..0bb7d355c45fe 100644 --- a/modules/auth/admin.go +++ b/modules/auth/admin.go @@ -32,7 +32,7 @@ type AdminEditUserForm struct { FullName string `binding:"MaxSize(100)"` Email string `binding:"Required;Email;MaxSize(254)"` Password string `binding:"MaxSize(255)"` - Website string `binding:"Url;MaxSize(255)"` + Website string `binding:"ValidUrl;MaxSize(255)"` Location string `binding:"MaxSize(50)"` MaxRepoCreation int Active bool diff --git a/modules/auth/org.go b/modules/auth/org.go index 8ad2ca6c6a10c..b9b3f981e1719 100644 --- a/modules/auth/org.go +++ b/modules/auth/org.go @@ -31,7 +31,7 @@ type UpdateOrgSettingForm struct { Name string `binding:"Required;AlphaDashDot;MaxSize(35)" locale:"org.org_name_holder"` FullName string `binding:"MaxSize(100)"` Description string `binding:"MaxSize(255)"` - Website string `binding:"Url;MaxSize(255)"` + Website string `binding:"ValidUrl;MaxSize(255)"` Location string `binding:"MaxSize(50)"` MaxRepoCreation int } diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index 629999987e60e..c56e76c871301 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -87,7 +87,7 @@ func (f MigrateRepoForm) ParseRemoteAddr(user *models.User) (string, error) { type RepoSettingForm struct { RepoName string `binding:"Required;AlphaDashDot;MaxSize(100)"` Description string `binding:"MaxSize(255)"` - Website string `binding:"Url;MaxSize(255)"` + Website string `binding:"ValidUrl;MaxSize(255)"` Interval string MirrorAddress string Private bool @@ -143,7 +143,7 @@ func (f WebhookForm) ChooseEvents() bool { // NewWebhookForm form for creating web hook type NewWebhookForm struct { - PayloadURL string `binding:"Required;Url"` + PayloadURL string `binding:"Required;ValidUrl"` ContentType int `binding:"Required"` Secret string WebhookForm @@ -156,7 +156,7 @@ func (f *NewWebhookForm) Validate(ctx *macaron.Context, errs binding.Errors) bin // NewSlackHookForm form for creating slack hook type NewSlackHookForm struct { - PayloadURL string `binding:"Required;Url"` + PayloadURL string `binding:"Required;ValidUrl"` Channel string `binding:"Required"` Username string IconURL string diff --git a/modules/auth/user_form.go b/modules/auth/user_form.go index bac9622fcc1e3..2f767d4c8cf58 100644 --- a/modules/auth/user_form.go +++ b/modules/auth/user_form.go @@ -103,7 +103,7 @@ type UpdateProfileForm struct { FullName string `binding:"MaxSize(100)"` Email string `binding:"Required;Email;MaxSize(254)"` KeepEmailPrivate bool - Website string `binding:"Url;MaxSize(255)"` + Website string `binding:"ValidUrl;MaxSize(255)"` Location string `binding:"MaxSize(50)"` } diff --git a/modules/validation/binding.go b/modules/validation/binding.go index c1880797b0555..f89609e28b1e3 100644 --- a/modules/validation/binding.go +++ b/modules/validation/binding.go @@ -6,6 +6,7 @@ package validation import ( "fmt" + "net/url" "regexp" "strings" @@ -25,6 +26,7 @@ var ( // AddBindingRules adds additional binding rules func AddBindingRules() { addGitRefNameBindingRule() + addValidURLBindingRule() } func addGitRefNameBindingRule() { @@ -53,3 +55,29 @@ func addGitRefNameBindingRule() { }, }) } + +func addValidURLBindingRule() { + // URL validation rule + binding.AddRule(&binding.Rule{ + IsMatch: func(rule string) bool { + return strings.HasPrefix(rule, "ValidUrl") + }, + IsValid: func(errs binding.Errors, name string, val interface{}) (bool, binding.Errors) { + if u, err := url.Parse(fmt.Sprintf("%v", val)); err != nil || (u.Scheme != "http" && u.Scheme != "https") || !validPort(u.Port()) { + errs.Add([]string{name}, binding.ERR_URL, "Url") + return false, errs + } + + return true, errs + }, + }) +} + +func validPort(p string) bool { + for _, r := range []byte(p) { + if r < '0' || r > '9' { + return false + } + } + return true +} From f6bba6acf87a011330db846acd6d155876830860 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauris=20Buk=C5=A1is-Haberkorns?= Date: Tue, 18 Apr 2017 00:21:22 +0300 Subject: [PATCH 4/9] Backward compatibility with older Go compiler --- modules/validation/binding.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/modules/validation/binding.go b/modules/validation/binding.go index f89609e28b1e3..5f21f08c696bf 100644 --- a/modules/validation/binding.go +++ b/modules/validation/binding.go @@ -63,7 +63,9 @@ func addValidURLBindingRule() { return strings.HasPrefix(rule, "ValidUrl") }, IsValid: func(errs binding.Errors, name string, val interface{}) (bool, binding.Errors) { - if u, err := url.Parse(fmt.Sprintf("%v", val)); err != nil || (u.Scheme != "http" && u.Scheme != "https") || !validPort(u.Port()) { + if u, err := url.Parse(fmt.Sprintf("%v", val)); err != nil || + (u.Scheme != "http" && u.Scheme != "https") || + !validPort(portOnly(u.Host)) { errs.Add([]string{name}, binding.ERR_URL, "Url") return false, errs } @@ -73,6 +75,20 @@ func addValidURLBindingRule() { }) } +func portOnly(hostport string) string { + colon := strings.IndexByte(hostport, ':') + if colon == -1 { + return "" + } + if i := strings.Index(hostport, "]:"); i != -1 { + return hostport[i+len("]:"):] + } + if strings.Contains(hostport, "]") { + return "" + } + return hostport[colon+len(":"):] +} + func validPort(p string) bool { for _, r := range []byte(p) { if r < '0' || r > '9' { From d23c65793902408434cf37c131afb75f4ffce8d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauris=20Buk=C5=A1is-Haberkorns?= Date: Tue, 18 Apr 2017 02:22:52 +0300 Subject: [PATCH 5/9] Add git reference name validation unit tests --- modules/validation/binding_test.go | 62 +++++++++++++ modules/validation/refname_test.go | 142 +++++++++++++++++++++++++++++ 2 files changed, 204 insertions(+) create mode 100644 modules/validation/binding_test.go create mode 100644 modules/validation/refname_test.go diff --git a/modules/validation/binding_test.go b/modules/validation/binding_test.go new file mode 100644 index 0000000000000..e726a145b92ca --- /dev/null +++ b/modules/validation/binding_test.go @@ -0,0 +1,62 @@ +// Copyright 2017 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package validation + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/go-macaron/binding" + "github.com/stretchr/testify/assert" + "gopkg.in/macaron.v1" +) + +const ( + testRoute = "/test" + formContentType = "application/x-www-form-urlencoded" +) + +type ( + validationTestCase struct { + description string + data interface{} + expectedErrors binding.Errors + } + + handlerFunc func(interface{}, ...interface{}) macaron.Handler + + modeler interface { + Model() string + } + + TestForm struct { + BranchName string `form:"BranchName" binding:"GitRefName"` + } +) + +func performValidationTest(t *testing.T, testCase validationTestCase) { + httpRecorder := httptest.NewRecorder() + m := macaron.Classic() + + m.Post(testRoute, binding.Validate(testCase.data), func(actual binding.Errors) { + assert.Equal(t, fmt.Sprintf("%+v", testCase.expectedErrors), fmt.Sprintf("%+v", actual)) + }) + + req, err := http.NewRequest("POST", testRoute, nil) + if err != nil { + panic(err) + } + + m.ServeHTTP(httpRecorder, req) + + switch httpRecorder.Code { + case http.StatusNotFound: + panic("Routing is messed up in test fixture (got 404): check methods and paths") + case http.StatusInternalServerError: + panic("Something bad happened on '" + testCase.description + "'") + } +} diff --git a/modules/validation/refname_test.go b/modules/validation/refname_test.go new file mode 100644 index 0000000000000..b101ffafef082 --- /dev/null +++ b/modules/validation/refname_test.go @@ -0,0 +1,142 @@ +// Copyright 2017 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package validation + +import ( + "testing" + + "github.com/go-macaron/binding" +) + +var gitRefNameValidationTestCases = []validationTestCase{ + { + description: "Referece contains only characters", + data: TestForm{ + BranchName: "test", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "Reference name contains single slash", + data: TestForm{ + BranchName: "feature/test", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "Reference name contains backslash", + data: TestForm{ + BranchName: "feature\\test", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, + }, + { + description: "Reference name starts with dot", + data: TestForm{ + BranchName: ".test", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, + }, + { + description: "Reference name ends with dot", + data: TestForm{ + BranchName: "test.", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, + }, + { + description: "Reference name starts with slash", + data: TestForm{ + BranchName: "/test", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, + }, + { + description: "Reference name ends with slash", + data: TestForm{ + BranchName: "test/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, + }, + { + description: "Reference name ends with .lock", + data: TestForm{ + BranchName: "test.lock", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, + }, + { + description: "Reference name contains multiple consecutive dots", + data: TestForm{ + BranchName: "te..st", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, + }, + { + description: "Reference name contains multiple consecutive slashes", + data: TestForm{ + BranchName: "te//st", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, + }, +} + +func Test_GitRefNameValidation(t *testing.T) { + AddBindingRules() + + for _, testCase := range gitRefNameValidationTestCases { + t.Run(testCase.description, func(t *testing.T) { + performValidationTest(t, testCase) + }) + } +} From 5fa98bb016d427b05281dccf92418bc39b4ac912 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauris=20Buk=C5=A1is-Haberkorns?= Date: Tue, 18 Apr 2017 02:24:24 +0300 Subject: [PATCH 6/9] Remove unused variable in unit test --- modules/validation/binding_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/validation/binding_test.go b/modules/validation/binding_test.go index e726a145b92ca..e2bf924134e5c 100644 --- a/modules/validation/binding_test.go +++ b/modules/validation/binding_test.go @@ -16,8 +16,7 @@ import ( ) const ( - testRoute = "/test" - formContentType = "application/x-www-form-urlencoded" + testRoute = "/test" ) type ( From e3ecbeb415d11542c39ec1db0615b3837dd88a34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauris=20Buk=C5=A1is-Haberkorns?= Date: Mon, 17 Apr 2017 23:46:51 +0300 Subject: [PATCH 7/9] Implement URL validation based on GoLang url.Parse method --- modules/auth/admin.go | 2 +- modules/auth/org.go | 2 +- modules/auth/repo_form.go | 6 +++--- modules/auth/user_form.go | 2 +- modules/validation/binding.go | 28 ++++++++++++++++++++++++++++ 5 files changed, 34 insertions(+), 6 deletions(-) diff --git a/modules/auth/admin.go b/modules/auth/admin.go index 68bc4f394133c..0bb7d355c45fe 100644 --- a/modules/auth/admin.go +++ b/modules/auth/admin.go @@ -32,7 +32,7 @@ type AdminEditUserForm struct { FullName string `binding:"MaxSize(100)"` Email string `binding:"Required;Email;MaxSize(254)"` Password string `binding:"MaxSize(255)"` - Website string `binding:"Url;MaxSize(255)"` + Website string `binding:"ValidUrl;MaxSize(255)"` Location string `binding:"MaxSize(50)"` MaxRepoCreation int Active bool diff --git a/modules/auth/org.go b/modules/auth/org.go index 8ad2ca6c6a10c..b9b3f981e1719 100644 --- a/modules/auth/org.go +++ b/modules/auth/org.go @@ -31,7 +31,7 @@ type UpdateOrgSettingForm struct { Name string `binding:"Required;AlphaDashDot;MaxSize(35)" locale:"org.org_name_holder"` FullName string `binding:"MaxSize(100)"` Description string `binding:"MaxSize(255)"` - Website string `binding:"Url;MaxSize(255)"` + Website string `binding:"ValidUrl;MaxSize(255)"` Location string `binding:"MaxSize(50)"` MaxRepoCreation int } diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index 629999987e60e..c56e76c871301 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -87,7 +87,7 @@ func (f MigrateRepoForm) ParseRemoteAddr(user *models.User) (string, error) { type RepoSettingForm struct { RepoName string `binding:"Required;AlphaDashDot;MaxSize(100)"` Description string `binding:"MaxSize(255)"` - Website string `binding:"Url;MaxSize(255)"` + Website string `binding:"ValidUrl;MaxSize(255)"` Interval string MirrorAddress string Private bool @@ -143,7 +143,7 @@ func (f WebhookForm) ChooseEvents() bool { // NewWebhookForm form for creating web hook type NewWebhookForm struct { - PayloadURL string `binding:"Required;Url"` + PayloadURL string `binding:"Required;ValidUrl"` ContentType int `binding:"Required"` Secret string WebhookForm @@ -156,7 +156,7 @@ func (f *NewWebhookForm) Validate(ctx *macaron.Context, errs binding.Errors) bin // NewSlackHookForm form for creating slack hook type NewSlackHookForm struct { - PayloadURL string `binding:"Required;Url"` + PayloadURL string `binding:"Required;ValidUrl"` Channel string `binding:"Required"` Username string IconURL string diff --git a/modules/auth/user_form.go b/modules/auth/user_form.go index bac9622fcc1e3..2f767d4c8cf58 100644 --- a/modules/auth/user_form.go +++ b/modules/auth/user_form.go @@ -103,7 +103,7 @@ type UpdateProfileForm struct { FullName string `binding:"MaxSize(100)"` Email string `binding:"Required;Email;MaxSize(254)"` KeepEmailPrivate bool - Website string `binding:"Url;MaxSize(255)"` + Website string `binding:"ValidUrl;MaxSize(255)"` Location string `binding:"MaxSize(50)"` } diff --git a/modules/validation/binding.go b/modules/validation/binding.go index c1880797b0555..f89609e28b1e3 100644 --- a/modules/validation/binding.go +++ b/modules/validation/binding.go @@ -6,6 +6,7 @@ package validation import ( "fmt" + "net/url" "regexp" "strings" @@ -25,6 +26,7 @@ var ( // AddBindingRules adds additional binding rules func AddBindingRules() { addGitRefNameBindingRule() + addValidURLBindingRule() } func addGitRefNameBindingRule() { @@ -53,3 +55,29 @@ func addGitRefNameBindingRule() { }, }) } + +func addValidURLBindingRule() { + // URL validation rule + binding.AddRule(&binding.Rule{ + IsMatch: func(rule string) bool { + return strings.HasPrefix(rule, "ValidUrl") + }, + IsValid: func(errs binding.Errors, name string, val interface{}) (bool, binding.Errors) { + if u, err := url.Parse(fmt.Sprintf("%v", val)); err != nil || (u.Scheme != "http" && u.Scheme != "https") || !validPort(u.Port()) { + errs.Add([]string{name}, binding.ERR_URL, "Url") + return false, errs + } + + return true, errs + }, + }) +} + +func validPort(p string) bool { + for _, r := range []byte(p) { + if r < '0' || r > '9' { + return false + } + } + return true +} From 0925765b4d8abf9f7e050500ebbcd4c27899a3ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauris=20Buk=C5=A1is-Haberkorns?= Date: Tue, 18 Apr 2017 00:21:22 +0300 Subject: [PATCH 8/9] Backward compatibility with older Go compiler --- modules/validation/binding.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/modules/validation/binding.go b/modules/validation/binding.go index f89609e28b1e3..5f21f08c696bf 100644 --- a/modules/validation/binding.go +++ b/modules/validation/binding.go @@ -63,7 +63,9 @@ func addValidURLBindingRule() { return strings.HasPrefix(rule, "ValidUrl") }, IsValid: func(errs binding.Errors, name string, val interface{}) (bool, binding.Errors) { - if u, err := url.Parse(fmt.Sprintf("%v", val)); err != nil || (u.Scheme != "http" && u.Scheme != "https") || !validPort(u.Port()) { + if u, err := url.Parse(fmt.Sprintf("%v", val)); err != nil || + (u.Scheme != "http" && u.Scheme != "https") || + !validPort(portOnly(u.Host)) { errs.Add([]string{name}, binding.ERR_URL, "Url") return false, errs } @@ -73,6 +75,20 @@ func addValidURLBindingRule() { }) } +func portOnly(hostport string) string { + colon := strings.IndexByte(hostport, ':') + if colon == -1 { + return "" + } + if i := strings.Index(hostport, "]:"); i != -1 { + return hostport[i+len("]:"):] + } + if strings.Contains(hostport, "]") { + return "" + } + return hostport[colon+len(":"):] +} + func validPort(p string) bool { for _, r := range []byte(p) { if r < '0' || r > '9' { From a550657c8a986cb8f34e8aab142fc8a94a5cc5d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauris=20Buk=C5=A1is-Haberkorns?= Date: Tue, 18 Apr 2017 02:59:40 +0300 Subject: [PATCH 9/9] Add url validation unit tests --- modules/validation/binding.go | 13 ++-- modules/validation/binding_test.go | 1 + modules/validation/validurl_test.go | 111 ++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 5 deletions(-) create mode 100644 modules/validation/validurl_test.go diff --git a/modules/validation/binding.go b/modules/validation/binding.go index 5f21f08c696bf..783499e69a82f 100644 --- a/modules/validation/binding.go +++ b/modules/validation/binding.go @@ -63,11 +63,14 @@ func addValidURLBindingRule() { return strings.HasPrefix(rule, "ValidUrl") }, IsValid: func(errs binding.Errors, name string, val interface{}) (bool, binding.Errors) { - if u, err := url.Parse(fmt.Sprintf("%v", val)); err != nil || - (u.Scheme != "http" && u.Scheme != "https") || - !validPort(portOnly(u.Host)) { - errs.Add([]string{name}, binding.ERR_URL, "Url") - return false, errs + str := fmt.Sprintf("%v", val) + if len(str) != 0 { + if u, err := url.ParseRequestURI(str); err != nil || + (u.Scheme != "http" && u.Scheme != "https") || + !validPort(portOnly(u.Host)) { + errs.Add([]string{name}, binding.ERR_URL, "Url") + return false, errs + } } return true, errs diff --git a/modules/validation/binding_test.go b/modules/validation/binding_test.go index e2bf924134e5c..7bc41ac3959bc 100644 --- a/modules/validation/binding_test.go +++ b/modules/validation/binding_test.go @@ -34,6 +34,7 @@ type ( TestForm struct { BranchName string `form:"BranchName" binding:"GitRefName"` + URL string `form:"ValidUrl" binding:"ValidUrl"` } ) diff --git a/modules/validation/validurl_test.go b/modules/validation/validurl_test.go new file mode 100644 index 0000000000000..ba4d7d53d9249 --- /dev/null +++ b/modules/validation/validurl_test.go @@ -0,0 +1,111 @@ +// Copyright 2017 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package validation + +import ( + "testing" + + "github.com/go-macaron/binding" +) + +var urlValidationTestCases = []validationTestCase{ + { + description: "Empty URL", + data: TestForm{ + URL: "", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "URL without port", + data: TestForm{ + URL: "http://test.lan/", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "URL with port", + data: TestForm{ + URL: "http://test.lan:3000/", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "URL with IPv6 address without port", + data: TestForm{ + URL: "http://[::1]/", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "URL with IPv6 address with port", + data: TestForm{ + URL: "http://[::1]:3000/", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "Invalid URL", + data: TestForm{ + URL: "http//test.lan/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URL"}, + Classification: binding.ERR_URL, + Message: "Url", + }, + }, + }, + { + description: "Invalid schema", + data: TestForm{ + URL: "ftp://test.lan/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URL"}, + Classification: binding.ERR_URL, + Message: "Url", + }, + }, + }, + { + description: "Invalid port", + data: TestForm{ + URL: "http://test.lan:3x4/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URL"}, + Classification: binding.ERR_URL, + Message: "Url", + }, + }, + }, + { + description: "Invalid port with IPv6 address", + data: TestForm{ + URL: "http://[::1]:3x4/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URL"}, + Classification: binding.ERR_URL, + Message: "Url", + }, + }, + }, +} + +func Test_ValidURLValidation(t *testing.T) { + AddBindingRules() + + for _, testCase := range urlValidationTestCases { + t.Run(testCase.description, func(t *testing.T) { + performValidationTest(t, testCase) + }) + } +}