API: Ensure slugs are not empty before saving/creating labels #4761

Signed-off-by: Michael Mayer <michael@photoprism.app>

internal/api/abort.go

@@ -110,6 +110,10 @@ func AbortBusy(c *gin.Context) {
 	Abort(c, http.StatusTooManyRequests, i18n.ErrBusy)
 }
 
+func AbortInvalidName(c *gin.Context) {
+	Abort(c, http.StatusBadRequest, i18n.ErrInvalidName)
+}
+
 func AbortInvalidCredentials(c *gin.Context) {
 	if c != nil {
 		c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": authn.ErrInvalidCredentials.Error(), "code": i18n.ErrInvalidCredentials, "message": i18n.Msg(i18n.ErrInvalidCredentials)})

internal/api/labels.go

@@ -46,22 +46,25 @@ func UpdateLabel(router *gin.RouterGroup) {
 		}
 
 		// Create new label form.
-		f, formErr := form.NewLabel(m)
+		frm, frmErr := form.NewLabel(m)
 
-		if formErr != nil {
+		if frmErr != nil {
 			Abort(c, http.StatusBadRequest, i18n.ErrBadRequest)
 			return
 		}
 
 		// Set form values from request.
-		if formErr = c.BindJSON(f); formErr != nil {
+		if frmErr = c.BindJSON(frm); frmErr != nil {
 			AbortBadRequest(c)
 			return
+		} else if frmErr = frm.Validate(); frmErr != nil {
+			AbortInvalidName(c)
+			return
 		}
 
 		// Save label and return new model values if successful.
-		if err = m.SaveForm(f); err != nil {
+		if err = m.SaveForm(frm); err != nil {
-			log.Error(err)
+			log.Errorf("label: %s", clean.Error(err))
 			AbortSaveFailed(c)
 			return
 		}

internal/api/photo_label.go

@@ -49,12 +49,15 @@ func AddPhotoLabel(router *gin.RouterGroup) {
 		if err = c.BindJSON(frm); err != nil {
 			AbortBadRequest(c)
 			return
+		} else if err = frm.Validate(); err != nil {
+			AbortInvalidName(c)
+			return
 		}
 
 		labelEntity := entity.FirstOrCreateLabel(entity.NewLabel(frm.LabelName, frm.LabelPriority))
 
 		if labelEntity == nil {
-			c.AbortWithStatusJSON(http.StatusInternalServerError, gin.H{"error": "failed to create label"})
+			AbortInvalidName(c)
 			return
 		}
 

internal/entity/entity_errors.go

@@ -0,0 +1,7 @@
+package entity
+
+import "fmt"
+
+var (
+	ErrInvalidName = fmt.Errorf("invalid name")
+)

internal/entity/label.go

@@ -114,8 +114,8 @@ func (m *Label) Save() error {
 func (m *Label) SaveForm(f *form.Label) error {
 	if f == nil {
 		return fmt.Errorf("form is nil")
-	} else if f.LabelName == "" {
+	} else if f.LabelName == "" || txt.Slug(f.LabelName) == "" {
-		return fmt.Errorf("missing name")
+		return ErrInvalidName
 	}
 
 	labelMutex.Lock()
@@ -125,9 +125,11 @@ func (m *Label) SaveForm(f *form.Label) error {
 		return err
 	}
 
-	m.SetName(f.LabelName)
+	if m.SetName(f.LabelName) {
-
 		return Db().Save(m).Error
+	} else {
+		return ErrInvalidName
+	}
 }
 
 // Create inserts the label to the database.
@@ -202,10 +204,14 @@ func (m *Label) Update(attr string, value interface{}) error {
 
 // FirstOrCreateLabel returns the existing label, inserts a new label or nil in case of errors.
 func FirstOrCreateLabel(m *Label) *Label {
+	if m.LabelSlug == "" && m.CustomSlug == "" {
+		return nil
+	}
+
 	result := &Label{}
 
 	if err := UnscopedDb().
-		Where("label_slug = ? OR (custom_slug <> '' AND custom_slug = ? OR label_slug <> '' AND label_slug = ?)", m.LabelSlug, m.CustomSlug, m.LabelSlug).
+		Where("(custom_slug <> '' AND custom_slug = ? OR label_slug <> '' AND label_slug = ?)", m.CustomSlug, m.LabelSlug).
 		First(result).Error; err == nil {
 		return result
 	} else if createErr := m.Create(); createErr == nil {
@@ -219,7 +225,7 @@ func FirstOrCreateLabel(m *Label) *Label {
 
 		return m
 	} else if err = UnscopedDb().
-		Where("label_slug = ? OR (custom_slug <> '' AND custom_slug = ? OR label_slug <> '' AND label_slug = ?)", m.LabelSlug, m.CustomSlug, m.LabelSlug).
+		Where("(custom_slug <> '' AND custom_slug = ? OR label_slug <> '' AND label_slug = ?)", m.CustomSlug, m.LabelSlug).
 		First(result).Error; err == nil {
 		return result
 	} else {
@@ -230,15 +236,44 @@ func FirstOrCreateLabel(m *Label) *Label {
 }
 
 // SetName changes the label name.
-func (m *Label) SetName(name string) {
+func (m *Label) SetName(name string) bool {
-	name = clean.NameCapitalized(name)
+	labelName := txt.Clip(clean.NameCapitalized(name), txt.ClipName)
 
-	if name == "" {
+	if labelName == "" {
-		return
+		return false
 	}
 
-	m.LabelName = txt.Clip(name, txt.ClipName)
+	labelSlug := txt.Slug(labelName)
-	m.CustomSlug = txt.Slug(name)
+
+	if labelSlug == "" {
+		return false
+	}
+
+	m.LabelName = labelName
+	m.CustomSlug = labelSlug
+
+	if m.LabelSlug == "" {
+		m.LabelSlug = labelSlug
+	}
+
+	return true
+}
+
+// InvalidName checks if the label name is invalid.
+func (m *Label) InvalidName() bool {
+	labelName := txt.Clip(clean.NameCapitalized(m.LabelName), txt.ClipName)
+
+	if labelName == "" {
+		return true
+	}
+
+	labelSlug := txt.Slug(labelName)
+
+	if labelSlug == "" {
+		return true
+	}
+
+	return false
 }
 
 // GetSlug returns the label slug.
@@ -271,8 +306,11 @@ func (m *Label) UpdateClassify(label classify.Label) error {
 	}
 
 	if m.CustomSlug == m.LabelSlug && label.Title() != m.LabelName {
-		m.SetName(label.Title())
+		if m.SetName(label.Title()) {
 			save = true
+		} else {
+			return ErrInvalidName
+		}
 	}
 
 	// Save label.

internal/entity/label_test.go

@@ -44,7 +44,6 @@ func TestLabel_SetName(t *testing.T) {
 		assert.Equal(t, "landscape", entity.LabelSlug)
 		assert.Equal(t, "landschaft", entity.CustomSlug)
 	})
-
 	t.Run("new name empty", func(t *testing.T) {
 		entity := LabelFixtures["flower"]
 
@@ -52,7 +51,7 @@ func TestLabel_SetName(t *testing.T) {
 		assert.Equal(t, "flower", entity.LabelSlug)
 		assert.Equal(t, "flower", entity.CustomSlug)
 
-		entity.SetName("")
+		assert.False(t, entity.SetName(""))
 
 		assert.Equal(t, "Flower", entity.LabelName)
 		assert.Equal(t, "flower", entity.LabelSlug)

internal/entity/photo.go

@@ -692,7 +692,7 @@ func (m *Photo) AddLabels(labels classify.Labels) {
 		labelEntity := FirstOrCreateLabel(NewLabel(classifyLabel.Title(), classifyLabel.Priority))
 
 		if labelEntity == nil {
-			log.Errorf("index: label %s should not be nil - you may have found a bug (%s)", clean.Log(classifyLabel.Title()), m)
+			log.Errorf("index: label %s coud not be created (%s)", clean.Log(classifyLabel.Title()), m)
 			continue
 		}
 

internal/entity/photo_label.go

@@ -50,8 +50,10 @@ func (m *PhotoLabel) Save() error {
 		m.Photo = nil
 	}
 
-	if m.Label != nil {
+	if m.Label == nil {
-		m.Label.SetName(m.Label.LabelName)
+		// Do nothing.
+	} else if !m.Label.SetName(m.Label.LabelName) {
+		return ErrInvalidName
 	}
 
 	return Db().Save(m).Error

internal/form/label.go

@@ -1,6 +1,11 @@
 package form
 
-import "github.com/ulule/deepcopier"
+import (
+	"github.com/photoprism/photoprism/pkg/clean"
+	"github.com/photoprism/photoprism/pkg/i18n"
+	"github.com/photoprism/photoprism/pkg/txt"
+	"github.com/ulule/deepcopier"
+)
 
 // Label represents a label edit form.
 type Label struct {
@@ -20,3 +25,20 @@ func NewLabel(m interface{}) (*Label, error) {
 	err := deepcopier.Copy(m).To(frm)
 	return frm, err
 }
+
+// Validate returns an error if any form values are invalid.
+func (frm *Label) Validate() error {
+	labelName := txt.Clip(clean.NameCapitalized(frm.LabelName), txt.ClipName)
+
+	if labelName == "" {
+		return i18n.Error(i18n.ErrInvalidName)
+	}
+
+	labelSlug := txt.Slug(labelName)
+
+	if labelSlug == "" {
+		return i18n.Error(i18n.ErrInvalidName)
+	}
+
+	return nil
+}