[API] Fix inconsistent label color format (#10129)

* update and use labelColorPattern

* add TestCases

* fix lint

* # optional for templates

* fix typo

* some more

* fix lint of **master**

integrations/api_issue_label_test.go

@@ -7,6 +7,7 @@ package integrations
 import (
 	"fmt"
 	"net/http"
+	"strings"
 	"testing"
 
 	"code.gitea.io/gitea/models"
@@ -15,6 +16,76 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+func TestAPIModifyLabels(t *testing.T) {
+	assert.NoError(t, models.LoadFixtures())
+
+	repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 2}).(*models.Repository)
+	owner := models.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User)
+	session := loginUser(t, owner.Name)
+	token := getTokenForLoggedInUser(t, session)
+	urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/labels?token=%s", owner.Name, repo.Name, token)
+
+	// CreateLabel
+	req := NewRequestWithJSON(t, "POST", urlStr, &api.CreateLabelOption{
+		Name:        "TestL 1",
+		Color:       "abcdef",
+		Description: "test label",
+	})
+	resp := session.MakeRequest(t, req, http.StatusCreated)
+	apiLabel := new(api.Label)
+	DecodeJSON(t, resp, &apiLabel)
+	dbLabel := models.AssertExistsAndLoadBean(t, &models.Label{ID: apiLabel.ID, RepoID: repo.ID}).(*models.Label)
+	assert.EqualValues(t, dbLabel.Name, apiLabel.Name)
+	assert.EqualValues(t, strings.TrimLeft(dbLabel.Color, "#"), apiLabel.Color)
+
+	req = NewRequestWithJSON(t, "POST", urlStr, &api.CreateLabelOption{
+		Name:        "TestL 2",
+		Color:       "#123456",
+		Description: "jet another test label",
+	})
+	session.MakeRequest(t, req, http.StatusCreated)
+	req = NewRequestWithJSON(t, "POST", urlStr, &api.CreateLabelOption{
+		Name:  "WrongTestL",
+		Color: "#12345g",
+	})
+	session.MakeRequest(t, req, http.StatusUnprocessableEntity)
+
+	//ListLabels
+	req = NewRequest(t, "GET", urlStr)
+	resp = session.MakeRequest(t, req, http.StatusOK)
+	var apiLabels []*api.Label
+	DecodeJSON(t, resp, &apiLabels)
+	assert.Len(t, apiLabels, 2)
+
+	//GetLabel
+	singleURLStr := fmt.Sprintf("/api/v1/repos/%s/%s/labels/%d?token=%s", owner.Name, repo.Name, dbLabel.ID, token)
+	req = NewRequest(t, "GET", singleURLStr)
+	resp = session.MakeRequest(t, req, http.StatusOK)
+	DecodeJSON(t, resp, &apiLabel)
+	assert.EqualValues(t, strings.TrimLeft(dbLabel.Color, "#"), apiLabel.Color)
+
+	//EditLabel
+	newName := "LabelNewName"
+	newColor := "09876a"
+	newColorWrong := "09g76a"
+	req = NewRequestWithJSON(t, "PATCH", singleURLStr, &api.EditLabelOption{
+		Name:  &newName,
+		Color: &newColor,
+	})
+	resp = session.MakeRequest(t, req, http.StatusOK)
+	DecodeJSON(t, resp, &apiLabel)
+	assert.EqualValues(t, newColor, apiLabel.Color)
+	req = NewRequestWithJSON(t, "PATCH", singleURLStr, &api.EditLabelOption{
+		Color: &newColorWrong,
+	})
+	session.MakeRequest(t, req, http.StatusUnprocessableEntity)
+
+	//DeleteLabel
+	req = NewRequest(t, "DELETE", singleURLStr)
+	resp = session.MakeRequest(t, req, http.StatusNoContent)
+
+}
+
 func TestAPIAddIssueLabels(t *testing.T) {
 	assert.NoError(t, models.LoadFixtures())
 

models/issue_label.go

@@ -18,7 +18,34 @@ import (
 	"xorm.io/xorm"
 )
 
-var labelColorPattern = regexp.MustCompile("#([a-fA-F0-9]{6})")
+// LabelColorPattern is a regexp witch can validate LabelColor
+var LabelColorPattern = regexp.MustCompile("^#[0-9a-fA-F]{6}$")
+
+// Label represents a label of repository for issues.
+type Label struct {
+	ID              int64 `xorm:"pk autoincr"`
+	RepoID          int64 `xorm:"INDEX"`
+	Name            string
+	Description     string
+	Color           string `xorm:"VARCHAR(7)"`
+	NumIssues       int
+	NumClosedIssues int
+	NumOpenIssues   int    `xorm:"-"`
+	IsChecked       bool   `xorm:"-"`
+	QueryString     string `xorm:"-"`
+	IsSelected      bool   `xorm:"-"`
+	IsExcluded      bool   `xorm:"-"`
+}
+
+// APIFormat converts a Label to the api.Label format
+func (label *Label) APIFormat() *api.Label {
+	return &api.Label{
+		ID:          label.ID,
+		Name:        label.Name,
+		Color:       strings.TrimLeft(label.Color, "#"),
+		Description: label.Description,
+	}
+}
 
 // GetLabelTemplateFile loads the label template file by given name,
 // then parses and returns a list of name-color pairs and optionally description.
@@ -43,7 +70,11 @@ func GetLabelTemplateFile(name string) ([][3]string, error) {
 			return nil, fmt.Errorf("line is malformed: %s", line)
 		}
 
-		if !labelColorPattern.MatchString(fields[0]) {
+		color := strings.Trim(fields[0], " ")
+		if len(color) == 6 {
+			color = "#" + color
+		}
+		if !LabelColorPattern.MatchString(color) {
 			return nil, fmt.Errorf("bad HTML color code in line: %s", line)
 		}
 
@@ -54,38 +85,12 @@ func GetLabelTemplateFile(name string) ([][3]string, error) {
 		}
 
 		fields[1] = strings.TrimSpace(fields[1])
-		list = append(list, [3]string{fields[1], fields[0], description})
+		list = append(list, [3]string{fields[1], color, description})
 	}
 
 	return list, nil
 }
 
-// Label represents a label of repository for issues.
-type Label struct {
-	ID              int64 `xorm:"pk autoincr"`
-	RepoID          int64 `xorm:"INDEX"`
-	Name            string
-	Description     string
-	Color           string `xorm:"VARCHAR(7)"`
-	NumIssues       int
-	NumClosedIssues int
-	NumOpenIssues   int    `xorm:"-"`
-	IsChecked       bool   `xorm:"-"`
-	QueryString     string `xorm:"-"`
-	IsSelected      bool   `xorm:"-"`
-	IsExcluded      bool   `xorm:"-"`
-}
-
-// APIFormat converts a Label to the api.Label format
-func (label *Label) APIFormat() *api.Label {
-	return &api.Label{
-		ID:          label.ID,
-		Name:        label.Name,
-		Color:       strings.TrimLeft(label.Color, "#"),
-		Description: label.Description,
-	}
-}
-
 // CalOpenIssues calculates the open issues of label.
 func (label *Label) CalOpenIssues() {
 	label.NumOpenIssues = label.NumIssues - label.NumClosedIssues
@@ -152,7 +157,7 @@ func LoadLabelsFormatted(labelTemplate string) (string, error) {
 	return strings.Join(labels, ", "), err
 }
 
-func initalizeLabels(e Engine, repoID int64, labelTemplate string) error {
+func initializeLabels(e Engine, repoID int64, labelTemplate string) error {
 	list, err := GetLabelTemplateFile(labelTemplate)
 	if err != nil {
 		return ErrIssueLabelTemplateLoad{labelTemplate, err}
@@ -175,9 +180,9 @@ func initalizeLabels(e Engine, repoID int64, labelTemplate string) error {
 	return nil
 }
 
-// InitalizeLabels adds a label set to a repository using a template
+// InitializeLabels adds a label set to a repository using a template
-func InitalizeLabels(ctx DBContext, repoID int64, labelTemplate string) error {
+func InitializeLabels(ctx DBContext, repoID int64, labelTemplate string) error {
-	return initalizeLabels(ctx.e, repoID, labelTemplate)
+	return initializeLabels(ctx.e, repoID, labelTemplate)
 }
 
 func newLabel(e Engine, label *Label) error {
@@ -187,6 +192,9 @@ func newLabel(e Engine, label *Label) error {
 
 // NewLabel creates a new label for a repository
 func NewLabel(label *Label) error {
+	if !LabelColorPattern.MatchString(label.Color) {
+		return fmt.Errorf("bad color code: %s", label.Color)
+	}
 	return newLabel(x, label)
 }
 
@@ -198,6 +206,9 @@ func NewLabels(labels ...*Label) error {
 		return err
 	}
 	for _, label := range labels {
+		if !LabelColorPattern.MatchString(label.Color) {
+			return fmt.Errorf("bad color code: %s", label.Color)
+		}
 		if err := newLabel(sess, label); err != nil {
 			return err
 		}
@@ -359,6 +370,9 @@ func updateLabel(e Engine, l *Label) error {
 
 // UpdateLabel updates label information.
 func UpdateLabel(l *Label) error {
+	if !LabelColorPattern.MatchString(l.Color) {
+		return fmt.Errorf("bad color code: %s", l.Color)
+	}
 	return updateLabel(x, l)
 }
 

models/issue_label_test.go

@@ -45,8 +45,11 @@ func TestNewLabels(t *testing.T) {
 	assert.NoError(t, PrepareTestDatabase())
 	labels := []*Label{
 		{RepoID: 2, Name: "labelName2", Color: "#123456"},
-		{RepoID: 3, Name: "labelName3", Color: "#234567"},
+		{RepoID: 3, Name: "labelName3", Color: "#23456F"},
 	}
+	assert.Error(t, NewLabel(&Label{RepoID: 3, Name: "invalid Color", Color: ""}))
+	assert.Error(t, NewLabel(&Label{RepoID: 3, Name: "invalid Color", Color: "123456"}))
+	assert.Error(t, NewLabel(&Label{RepoID: 3, Name: "invalid Color", Color: "#12345G"}))
 	for _, label := range labels {
 		AssertNotExistsBean(t, label)
 	}

modules/repository/create.go

@@ -58,15 +58,15 @@ func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (_ *m
 
 			// Initialize Issue Labels if selected
 			if len(opts.IssueLabels) > 0 {
-				if err = models.InitalizeLabels(ctx, repo.ID, opts.IssueLabels); err != nil {
+				if err = models.InitializeLabels(ctx, repo.ID, opts.IssueLabels); err != nil {
-					return fmt.Errorf("initalizeLabels: %v", err)
+					return fmt.Errorf("InitializeLabels: %v", err)
 				}
 			}
 
 			if stdout, err := git.NewCommand("update-server-info").
 				SetDescription(fmt.Sprintf("CreateRepository(git update-server-info): %s", repoPath)).
 				RunInDir(repoPath); err != nil {
-				log.Error("CreateRepitory(git update-server-info) in %v: Stdout: %s\nError: %v", repo, stdout, err)
+				log.Error("CreateRepository(git update-server-info) in %v: Stdout: %s\nError: %v", repo, stdout, err)
 				return fmt.Errorf("CreateRepository(git update-server-info): %v", err)
 			}
 		}

modules/structs/issue_label.go

@@ -22,7 +22,7 @@ type CreateLabelOption struct {
 	Name string `json:"name" binding:"Required"`
 	// required:true
 	// example: #00aabb
-	Color       string `json:"color" binding:"Required;Size(7)"`
+	Color       string `json:"color" binding:"Required"`
 	Description string `json:"description"`
 }
 

routers/api/v1/repo/label.go

@@ -6,8 +6,10 @@
 package repo
 
 import (
+	"fmt"
 	"net/http"
 	"strconv"
+	"strings"
 
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/context"
@@ -135,6 +137,17 @@ func CreateLabel(ctx *context.APIContext, form api.CreateLabelOption) {
 	// responses:
 	//   "201":
 	//     "$ref": "#/responses/Label"
+	//   "422":
+	//     "$ref": "#/responses/validationError"
+
+	form.Color = strings.Trim(form.Color, " ")
+	if len(form.Color) == 6 {
+		form.Color = "#" + form.Color
+	}
+	if !models.LabelColorPattern.MatchString(form.Color) {
+		ctx.Error(http.StatusUnprocessableEntity, "ColorPattern", fmt.Errorf("bad color code: %s", form.Color))
+		return
+	}
 
 	label := &models.Label{
 		Name:        form.Name,
@@ -182,6 +195,8 @@ func EditLabel(ctx *context.APIContext, form api.EditLabelOption) {
 	// responses:
 	//   "200":
 	//     "$ref": "#/responses/Label"
+	//   "422":
+	//     "$ref": "#/responses/validationError"
 
 	label, err := models.GetLabelInRepoByID(ctx.Repo.Repository.ID, ctx.ParamsInt64(":id"))
 	if err != nil {
@@ -197,7 +212,14 @@ func EditLabel(ctx *context.APIContext, form api.EditLabelOption) {
 		label.Name = *form.Name
 	}
 	if form.Color != nil {
-		label.Color = *form.Color
+		label.Color = strings.Trim(*form.Color, " ")
+		if len(label.Color) == 6 {
+			label.Color = "#" + label.Color
+		}
+		if !models.LabelColorPattern.MatchString(label.Color) {
+			ctx.Error(http.StatusUnprocessableEntity, "ColorPattern", fmt.Errorf("bad color code: %s", label.Color))
+			return
+		}
 	}
 	if form.Description != nil {
 		label.Description = *form.Description

routers/repo/issue_label.go

@@ -35,14 +35,14 @@ func InitializeLabels(ctx *context.Context, form auth.InitializeLabelsForm) {
 		return
 	}
 
-	if err := models.InitalizeLabels(models.DefaultDBContext(), ctx.Repo.Repository.ID, form.TemplateName); err != nil {
+	if err := models.InitializeLabels(models.DefaultDBContext(), ctx.Repo.Repository.ID, form.TemplateName); err != nil {
 		if models.IsErrIssueLabelTemplateLoad(err) {
 			originalErr := err.(models.ErrIssueLabelTemplateLoad).OriginalError
 			ctx.Flash.Error(ctx.Tr("repo.issues.label_templates.fail_to_load_file", form.TemplateName, originalErr))
 			ctx.Redirect(ctx.Repo.RepoLink + "/labels")
 			return
 		}
-		ctx.ServerError("InitalizeLabels", err)
+		ctx.ServerError("InitializeLabels", err)
 		return
 	}
 	ctx.Redirect(ctx.Repo.RepoLink + "/labels")

templates/swagger/v1_json.tmpl

@@ -5317,6 +5317,9 @@
         "responses": {
           "201": {
             "$ref": "#/responses/Label"
+          },
+          "422": {
+            "$ref": "#/responses/validationError"
           }
         }
       }
@@ -5443,6 +5446,9 @@
         "responses": {
           "200": {
             "$ref": "#/responses/Label"
+          },
+          "422": {
+            "$ref": "#/responses/validationError"
           }
         }
       }