From d056bf300ff5ebd89d8b0035722c94a3b08ac745 Mon Sep 17 00:00:00 2001 From: mrsdizzie Date: Tue, 26 Mar 2019 15:59:48 -0400 Subject: [PATCH] Clean up ref name rules (#6437) * Clean up ref name rules Clean up checks on reference names to better conform to the guideline here: https://git-scm.com/docs/git-check-ref-format This fixes half of #6321 * Update branch create integration test According to: https://git-scm.com/docs/git-check-ref-format And: git check-ref-format "master/feature=test1" This is a valid branch name and we should not be testing for it to fail. --- integrations/repo_branch_test.go | 2 +- modules/validation/binding.go | 7 +- modules/validation/refname_test.go | 124 +++++++++++++++++++++++++++++ routers/repo/branch.go | 3 +- 4 files changed, 132 insertions(+), 4 deletions(-) diff --git a/integrations/repo_branch_test.go b/integrations/repo_branch_test.go index fb33778cd..3101dc4c0 100644 --- a/integrations/repo_branch_test.go +++ b/integrations/repo_branch_test.go @@ -58,7 +58,7 @@ func TestCreateBranch(t *testing.T) { OldRefSubURL: "branch/master", NewBranch: "feature=test1", ExpectedStatus: http.StatusFound, - FlashMessage: i18n.Tr("en", "form.NewBranchName") + i18n.Tr("en", "form.git_ref_name_error"), + FlashMessage: i18n.Tr("en", "repo.branch.create_success", "feature=test1"), }, { OldRefSubURL: "branch/master", diff --git a/modules/validation/binding.go b/modules/validation/binding.go index bf3e6c4f9..03b6f6276 100644 --- a/modules/validation/binding.go +++ b/modules/validation/binding.go @@ -19,7 +19,9 @@ const ( var ( // GitRefNamePattern is regular expression with unallowed characters in git reference name - GitRefNamePattern = regexp.MustCompile("[^\\d\\w-_\\./]") + // They cannot have ASCII control characters (i.e. bytes whose values are lower than \040, or \177 DEL), space, tilde ~, caret ^, or colon : anywhere. + // They cannot have question-mark ?, asterisk *, or open bracket [ anywhere + GitRefNamePattern = regexp.MustCompile(`[\000-\037\177 \\~^:?*[]+`) ) // AddBindingRules adds additional binding rules @@ -44,7 +46,8 @@ func addGitRefNameBindingRule() { // 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.HasSuffix(str, ".") || strings.Contains(str, "..") || - strings.Contains(str, "//") { + strings.Contains(str, "//") || strings.Contains(str, "@{") || + str == "@" { errs.Add([]string{name}, ErrGitRefName, "GitRefName") return false, errs } diff --git a/modules/validation/refname_test.go b/modules/validation/refname_test.go index b101ffafe..57e3f1bf8 100644 --- a/modules/validation/refname_test.go +++ b/modules/validation/refname_test.go @@ -25,6 +25,13 @@ var gitRefNameValidationTestCases = []validationTestCase{ }, expectedErrors: binding.Errors{}, }, + { + description: "Reference name has allowed special characters", + data: TestForm{ + BranchName: "debian/1%1.6.0-2", + }, + expectedErrors: binding.Errors{}, + }, { description: "Reference name contains backslash", data: TestForm{ @@ -129,6 +136,123 @@ var gitRefNameValidationTestCases = []validationTestCase{ }, }, }, + { + description: "Reference name is single @", + data: TestForm{ + BranchName: "@", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, + }, + { + description: "Reference name has @{", + data: TestForm{ + BranchName: "branch@{", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, + }, + { + description: "Reference name has unallowed special character ~", + data: TestForm{ + BranchName: "~debian/1%1.6.0-2", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, + }, + { + description: "Reference name has unallowed special character *", + data: TestForm{ + BranchName: "*debian/1%1.6.0-2", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, + }, + { + description: "Reference name has unallowed special character ?", + data: TestForm{ + BranchName: "?debian/1%1.6.0-2", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, + }, + { + description: "Reference name has unallowed special character ^", + data: TestForm{ + BranchName: "^debian/1%1.6.0-2", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, + }, + { + description: "Reference name has unallowed special character :", + data: TestForm{ + BranchName: "debian:jessie", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, + }, + { + description: "Reference name has unallowed special character (whitespace)", + data: TestForm{ + BranchName: "debian jessie", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, + }, + { + description: "Reference name has unallowed special character [", + data: TestForm{ + BranchName: "debian[jessie", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, + }, } func Test_GitRefNameValidation(t *testing.T) { diff --git a/routers/repo/branch.go b/routers/repo/branch.go index 823b031a4..44fd4bcd1 100644 --- a/routers/repo/branch.go +++ b/routers/repo/branch.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/util" ) const ( @@ -250,5 +251,5 @@ func CreateBranch(ctx *context.Context, form auth.NewBranchForm) { } ctx.Flash.Success(ctx.Tr("repo.branch.create_success", form.NewBranchName)) - ctx.Redirect(ctx.Repo.RepoLink + "/src/branch/" + form.NewBranchName) + ctx.Redirect(ctx.Repo.RepoLink + "/src/branch/" + util.PathEscapeSegments(form.NewBranchName)) }