Validate hex colors when creating/editing labels

modules/label/label.go

@@ -21,6 +21,19 @@ type Label struct {
 	ExclusiveOrder int    `yaml:"exclusive_order,omitempty"`
 }
 
+type ErrInvalidLabelColor struct {
+	Color string
+}
+
+func (e *ErrInvalidLabelColor) Error() string {
+	return "invalid label color: " + e.Color
+}
+
+func IsErrInvalidLabelColor(err error) bool {
+	_, ok := err.(*ErrInvalidLabelColor)
+	return ok
+}
+
 // NormalizeColor normalizes a color string to a 6-character hex code
 func NormalizeColor(color string) (string, error) {
 	// normalize case
@@ -32,7 +45,9 @@ func NormalizeColor(color string) (string, error) {
 	}
 
 	if !colorPattern.MatchString(color) {
-		return "", fmt.Errorf("bad color code: %s", color)
+		return "", &ErrInvalidLabelColor{
+			Color: color,
+		}
 	}
 
 	// convert 3-character shorthand into 6-character version

routers/web/org/org_labels.go

@@ -52,8 +52,12 @@ func NewLabel(ctx *context.Context) {
 		ExclusiveOrder: form.ExclusiveOrder,
 	}
 	if err := issues_model.NewLabel(ctx, l); err != nil {
-		ctx.ServerError("NewLabel", err)
-		return
+		if label.IsErrInvalidLabelColor(err) {
+			ctx.Flash.Error("NewLabel: " + err.Error())
+		} else {
+			ctx.ServerError("NewLabel", err)
+			return
+		}
 	}
 	ctx.Redirect(ctx.Org.OrgLink + "/settings/labels")
 }
@@ -79,8 +83,12 @@ func UpdateLabel(ctx *context.Context) {
 	l.Color = form.Color
 	l.SetArchived(form.IsArchived)
 	if err := issues_model.UpdateLabel(ctx, l); err != nil {
-		ctx.ServerError("UpdateLabel", err)
-		return
+		if label.IsErrInvalidLabelColor(err) {
+			ctx.Flash.Error("UpdateLabel: " + err.Error())
+		} else {
+			ctx.ServerError("UpdateLabel", err)
+			return
+		}
 	}
 	ctx.Redirect(ctx.Org.OrgLink + "/settings/labels")
 }

routers/web/repo/issue_label.go

@@ -119,8 +119,12 @@ func NewLabel(ctx *context.Context) {
 		Color:          form.Color,
 	}
 	if err := issues_model.NewLabel(ctx, l); err != nil {
-		ctx.ServerError("NewLabel", err)
-		return
+		if label.IsErrInvalidLabelColor(err) {
+			ctx.Flash.Error("NewLabel: " + err.Error())
+		} else {
+			ctx.ServerError("NewLabel", err)
+			return
+		}
 	}
 	ctx.Redirect(ctx.Repo.RepoLink + "/labels")
 }
@@ -146,8 +150,12 @@ func UpdateLabel(ctx *context.Context) {
 
 	l.SetArchived(form.IsArchived)
 	if err := issues_model.UpdateLabel(ctx, l); err != nil {
-		ctx.ServerError("UpdateLabel", err)
-		return
+		if label.IsErrInvalidLabelColor(err) {
+			ctx.Flash.Error("UpdateLabel: " + err.Error())
+		} else {
+			ctx.ServerError("UpdateLabel", err)
+			return
+		}
 	}
 	ctx.Redirect(ctx.Repo.RepoLink + "/labels")
 }

routers/web/repo/issue_label_test.go

@@ -92,6 +92,24 @@ func TestNewLabel(t *testing.T) {
 	assert.Equal(t, "/user2/repo1/labels", test.RedirectURL(ctx.Resp))
 }
 
+func TestNewLabelGivenInvalidLabelCode(t *testing.T) {
+	unittest.PrepareTestEnv(t)
+	ctx, _ := contexttest.MockContext(t, "user2/repo1/labels/edit")
+	contexttest.LoadUser(t, ctx, 2)
+	contexttest.LoadRepo(t, ctx, 1)
+	web.SetForm(ctx, &forms.CreateLabelForm{
+		Title: "newlabel",
+		Color: "bad-label-code",
+	})
+	NewLabel(ctx)
+	assert.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus())
+	assert.Equal(t, "/user2/repo1/labels", test.RedirectURL(ctx.Resp))
+	assert.True(t, ctx.Flash.Has("error"))
+	unittest.AssertNotExistsBean(t, &issues_model.Label{
+		Name: "newlabel",
+	})
+}
+
 func TestUpdateLabel(t *testing.T) {
 	unittest.PrepareTestEnv(t)
 	ctx, _ := contexttest.MockContext(t, "user2/repo1/labels/edit")
@@ -113,6 +131,29 @@ func TestUpdateLabel(t *testing.T) {
 	assert.Equal(t, "/user2/repo1/labels", test.RedirectURL(ctx.Resp))
 }
 
+func TestUpdateLabelGivenInvalidLabelCode(t *testing.T) {
+	unittest.PrepareTestEnv(t)
+	ctx, _ := contexttest.MockContext(t, "user2/repo1/labels/edit")
+	contexttest.LoadUser(t, ctx, 2)
+	contexttest.LoadRepo(t, ctx, 1)
+	web.SetForm(ctx, &forms.CreateLabelForm{
+		ID:    1,
+		Title: "label1",
+		Color: "bad-label-code",
+	})
+
+	UpdateLabel(ctx)
+
+	assert.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus())
+	assert.Equal(t, "/user2/repo1/labels", test.RedirectURL(ctx.Resp))
+	assert.True(t, ctx.Flash.Has("error"))
+	unittest.AssertExistsAndLoadBean(t, &issues_model.Label{
+		ID:    1,
+		Name:  "label1",
+		Color: "#abcdef",
+	})
+}
+
 func TestDeleteLabel(t *testing.T) {
 	unittest.PrepareTestEnv(t)
 	ctx, _ := contexttest.MockContext(t, "user2/repo1/labels/delete")

templates/repo/issue/labels/label_edit_modal.tmpl

@@ -50,7 +50,7 @@
 			<div class="field">
 				<label for="color">{{ctx.Locale.Tr "repo.issues.label_color"}}</label>
 				<div class="column js-color-picker-input">
-					<input name="color" value="#70c24a" placeholder="#c320f6" required maxlength="7">
+					<input name="color" value="#70c24a" placeholder="#c320f6" required pattern="^#?(?:[A-Fa-f0-9]{3}|[A-Fa-f0-9]{6})$" maxlength="7">
 					{{template "repo/issue/label_precolors"}}
 				</div>
 			</div>