fix reaction's name validation with + sign in it (#6221)

5 files changed, 62 insertions, 18 deletions

diff --git a/api4/api.go b/api4/api.go
index e25494f93..e75d1bf45 100644
--- a/api4/api.go
+++ b/api4/api.go
@@ -90,7 +90,7 @@ type Routes struct {
 	Emojis *mux.Router // 'api/v4/emoji'
 	Emoji  *mux.Router // 'api/v4/emoji/{emoji_id:[A-Za-z0-9]+}'
 
-	ReactionByNameForPostForUser *mux.Router // 'api/v4/users/{user_id:[A-Za-z0-9]+}/posts/{post_id:[A-Za-z0-9]+}/reactions/{emoji_name:[A-Za-z0-9_-]+}'
+	ReactionByNameForPostForUser *mux.Router // 'api/v4/users/{user_id:[A-Za-z0-9]+}/posts/{post_id:[A-Za-z0-9]+}/reactions/{emoji_name:[A-Za-z0-9_-+]+}'
 
 	Webrtc *mux.Router // 'api/v4/webrtc'
 }
@@ -170,7 +170,7 @@ func InitApi(full bool) {
 	BaseRoutes.Emojis = BaseRoutes.ApiRoot.PathPrefix("/emoji").Subrouter()
 	BaseRoutes.Emoji = BaseRoutes.Emojis.PathPrefix("/{emoji_id:[A-Za-z0-9]+}").Subrouter()
 
-	BaseRoutes.ReactionByNameForPostForUser = BaseRoutes.PostForUser.PathPrefix("/reactions/{emoji_name:[A-Za-z0-9_-]+}").Subrouter()
+	BaseRoutes.ReactionByNameForPostForUser = BaseRoutes.PostForUser.PathPrefix("/reactions/{emoji_name:[A-Za-z0-9\\_\\-\\+]+}").Subrouter()
 
 	BaseRoutes.Webrtc = BaseRoutes.ApiRoot.PathPrefix("/webrtc").Subrouter()
 
diff --git a/api4/context.go b/api4/context.go
index c7fba4f5f..5522d1836 100644
--- a/api4/context.go
+++ b/api4/context.go
@@ -6,6 +6,7 @@ package api4
 import (
 	"fmt"
 	"net/http"
+	"regexp"
 	"strings"
 	"time"
 
@@ -504,7 +505,9 @@ func (c *Context) RequireEmojiName() *Context {
 		return c
 	}
 
-	if len(c.Params.EmojiName) == 0 || len(c.Params.EmojiName) > 64 || !model.IsValidAlphaNumHyphenUnderscore(c.Params.EmojiName, false) {
+	validName := regexp.MustCompile(`^[a-zA-Z0-9\-\+_]+$`)
+
+	if len(c.Params.EmojiName) == 0 || len(c.Params.EmojiName) > 64 || !validName.MatchString(c.Params.EmojiName) {
 		c.SetInvalidUrlParam("emoji_name")
 	}
 
diff --git a/api4/reaction_test.go b/api4/reaction_test.go
index b80c96118..c973c9a13 100644
--- a/api4/reaction_test.go
+++ b/api4/reaction_test.go
@@ -70,6 +70,20 @@ func TestSaveReaction(t *testing.T) {
 		t.Fatal("should have save multiple reactions")
 	}
 
+	// saving special case
+	reaction.EmojiName = "+1"
+
+	rr, resp = Client.SaveReaction(reaction)
+	CheckNoError(t, resp)
+
+	if rr.EmojiName != reaction.EmojiName {
+		t.Fatal("EmojiName did not match")
+	}
+
+	if reactions, err := app.GetReactionsForPost(postId); err != nil && len(reactions) != 3 {
+		t.Fatal("should have save multiple reactions")
+	}
+
 	reaction.PostId = GenerateTestId()
 
 	_, resp = Client.SaveReaction(reaction)
@@ -244,22 +258,41 @@ func TestDeleteReaction(t *testing.T) {
 		t.Fatal("should have deleted 1 reaction only")
 	}
 
-	// deleting a reaction made by another user
+	// deleting one reaction of name +1
 	r3 := &model.Reaction{
+		UserId:    userId,
+		PostId:    postId,
+		EmojiName: "+1",
+	}
+
+	app.SaveReactionForPost(r3)
+	if reactions, err := app.GetReactionsForPost(postId); err != nil || len(reactions) != 2 {
+		t.Fatal("didn't save reactions correctly")
+	}
+
+	_, resp = Client.DeleteReaction(r3)
+	CheckNoError(t, resp)
+
+	if reactions, err := app.GetReactionsForPost(postId); err != nil || len(reactions) != 1 || *reactions[0] != *r1 {
+		t.Fatal("should have deleted 1 reaction only")
+	}
+
+	// deleting a reaction made by another user
+	r4 := &model.Reaction{
 		UserId:    user2Id,
 		PostId:    postId,
 		EmojiName: "smile_",
 	}
 
 	th.LoginBasic2()
-	app.SaveReactionForPost(r3)
+	app.SaveReactionForPost(r4)
 	if reactions, err := app.GetReactionsForPost(postId); err != nil || len(reactions) != 2 {
 		t.Fatal("didn't save reaction correctly")
 	}
 
 	th.LoginBasic()
 
-	ok, resp = Client.DeleteReaction(r3)
+	ok, resp = Client.DeleteReaction(r4)
 	CheckForbiddenStatus(t, resp)
 
 	if ok {
@@ -310,7 +343,7 @@ func TestDeleteReaction(t *testing.T) {
 	_, resp = th.SystemAdminClient.DeleteReaction(r1)
 	CheckNoError(t, resp)
 
-	_, resp = th.SystemAdminClient.DeleteReaction(r3)
+	_, resp = th.SystemAdminClient.DeleteReaction(r4)
 	CheckNoError(t, resp)
 
 	if reactions, err := app.GetReactionsForPost(postId); err != nil || len(reactions) != 0 {
diff --git a/model/reaction.go b/model/reaction.go
index 5dbf07152..3d334c214 100644
--- a/model/reaction.go
+++ b/model/reaction.go
@@ -6,6 +6,7 @@ package model
 import (
 	"encoding/json"
 	"io"
+	"regexp"
 )
 
 type Reaction struct {
@@ -60,7 +61,9 @@ func (o *Reaction) IsValid() *AppError {
 		return NewLocAppError("Reaction.IsValid", "model.reaction.is_valid.post_id.app_error", nil, "post_id="+o.PostId)
 	}
 
-	if len(o.EmojiName) == 0 || len(o.EmojiName) > 64 || !IsValidAlphaNumHyphenUnderscore(o.EmojiName, false) {
+	validName := regexp.MustCompile(`^[a-zA-Z0-9\-\+_]+$`)
+
+	if len(o.EmojiName) == 0 || len(o.EmojiName) > 64 || !validName.MatchString(o.EmojiName) {
 		return NewLocAppError("Reaction.IsValid", "model.reaction.is_valid.emoji_name.app_error", nil, "emoji_name="+o.EmojiName)
 	}
 
diff --git a/model/reaction_test.go b/model/reaction_test.go
index e447e4329..a35750477 100644
--- a/model/reaction_test.go
+++ b/model/reaction_test.go
@@ -42,16 +42,6 @@ func TestReactionIsValid(t *testing.T) {
 	}
 
 	reaction.PostId = NewId()
-	reaction.EmojiName = ""
-	if err := reaction.IsValid(); err == nil {
-		t.Fatal("emoji name should be invalid")
-	}
-
-	reaction.EmojiName = strings.Repeat("a", 65)
-	if err := reaction.IsValid(); err == nil {
-		t.Fatal("emoji name should be invalid")
-	}
-
 	reaction.EmojiName = strings.Repeat("a", 64)
 	if err := reaction.IsValid(); err != nil {
 		t.Fatal(err)
@@ -67,11 +57,26 @@ func TestReactionIsValid(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	reaction.EmojiName = "+1"
+	if err := reaction.IsValid(); err != nil {
+		t.Fatal(err)
+	}
+
 	reaction.EmojiName = "emoji:"
 	if err := reaction.IsValid(); err == nil {
 		t.Fatal(err)
 	}
 
+	reaction.EmojiName = ""
+	if err := reaction.IsValid(); err == nil {
+		t.Fatal("emoji name should be invalid")
+	}
+
+	reaction.EmojiName = strings.Repeat("a", 65)
+	if err := reaction.IsValid(); err == nil {
+		t.Fatal("emoji name should be invalid")
+	}
+
 	reaction.CreateAt = 0
 	if err := reaction.IsValid(); err == nil {
 		t.Fatal("create at should be invalid")