Clean up ref name rules (go-gitea#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 go-gitea#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.
mrsdizzie · Mar 26, 2019 · ebe0d20 · ebe0d20
1 parent 0bf7ed5
commit ebe0d20
Show file tree

Hide file tree

Showing 4 changed files with 132 additions and 4 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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))
 }