Fix SQL syntax error when a non-existant channelId is attemted to be viewed. (#9975)

api4/channel.go

@@ -917,7 +917,7 @@ func viewChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 	}
 
 	view := model.ChannelViewFromJson(r.Body)
-	if view == nil {
+	if view == nil || !model.IsValidId(view.ChannelId) || (view.PrevChannelId != "" && !model.IsValidId(view.PrevChannelId)) {
 		c.SetInvalidParam("channel_view")
 		return
 	}

api4/channel_test.go

@@ -1416,7 +1416,17 @@ func TestViewChannel(t *testing.T) {
 
 	view.PrevChannelId = "junk"
 	_, resp = Client.ViewChannel(th.BasicUser.Id, view)
-	CheckNoError(t, resp)
+	CheckBadRequestStatus(t, resp)
+
+	view.PrevChannelId = ""
+	view.ChannelId = "junk"
+	_, resp = Client.ViewChannel(th.BasicUser.Id, view)
+	CheckBadRequestStatus(t, resp)
+
+	view.ChannelId = "correctlysizedjunkdddfdfdf"
+	_, resp = Client.ViewChannel(th.BasicUser.Id, view)
+	CheckBadRequestStatus(t, resp)
+	view.ChannelId = th.BasicChannel.Id
 
 	member, resp := Client.GetChannelMember(th.BasicChannel.Id, th.BasicUser.Id, "")
 	CheckNoError(t, resp)

app/channel.go

@@ -1600,30 +1600,28 @@ func (a *App) MarkChannelsAsViewed(channelIds []string, userId string, clearPush
 	channelsToClearPushNotifications := []string{}
 	if *a.Config().EmailSettings.SendPushNotifications && clearPushNotifications {
 		for _, channelId := range channelIds {
-			if model.IsValidId(channelId) {
+			result := <-a.Srv.Store.Channel().GetMember(channelId, userId)
-				result := <-a.Srv.Store.Channel().GetMember(channelId, userId)
+			if result.Err != nil {
-				if result.Err != nil {
+				mlog.Warn(fmt.Sprintf("Failed to get membership %v", result.Err))
-					mlog.Warn(fmt.Sprintf("Failed to get membership %v", result.Err))
+				continue
-					continue
+			}
-				}
+			member := result.Data.(*model.ChannelMember)
-				member := result.Data.(*model.ChannelMember)
 
-				notify := member.NotifyProps[model.PUSH_NOTIFY_PROP]
+			notify := member.NotifyProps[model.PUSH_NOTIFY_PROP]
-				if notify == model.CHANNEL_NOTIFY_DEFAULT {
+			if notify == model.CHANNEL_NOTIFY_DEFAULT {
-					user, _ := a.GetUser(userId)
+				user, _ := a.GetUser(userId)
-					notify = user.NotifyProps[model.PUSH_NOTIFY_PROP]
+				notify = user.NotifyProps[model.PUSH_NOTIFY_PROP]
-				}
+			}
-				if notify == model.USER_NOTIFY_ALL {
+			if notify == model.USER_NOTIFY_ALL {
-					if result := <-a.Srv.Store.User().GetAnyUnreadPostCountForChannel(userId, channelId); result.Err == nil {
+				if result := <-a.Srv.Store.User().GetAnyUnreadPostCountForChannel(userId, channelId); result.Err == nil {
-						if result.Data.(int64) > 0 {
+					if result.Data.(int64) > 0 {
-							channelsToClearPushNotifications = append(channelsToClearPushNotifications, channelId)
+						channelsToClearPushNotifications = append(channelsToClearPushNotifications, channelId)
-						}
 					}
-				} else if notify == model.USER_NOTIFY_MENTION {
+				}
-					if result := <-a.Srv.Store.User().GetUnreadCountForChannel(userId, channelId); result.Err == nil {
+			} else if notify == model.USER_NOTIFY_MENTION {
-						if result.Data.(int64) > 0 {
+				if result := <-a.Srv.Store.User().GetUnreadCountForChannel(userId, channelId); result.Err == nil {
-							channelsToClearPushNotifications = append(channelsToClearPushNotifications, channelId)
+					if result.Data.(int64) > 0 {
-						}
+						channelsToClearPushNotifications = append(channelsToClearPushNotifications, channelId)
 					}
 				}
 			}
@@ -1637,11 +1635,9 @@ func (a *App) MarkChannelsAsViewed(channelIds []string, userId string, clearPush
 	times := result.Data.(map[string]int64)
 	if *a.Config().ServiceSettings.EnableChannelViewedMessages {
 		for _, channelId := range channelIds {
-			if model.IsValidId(channelId) {
+			message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_CHANNEL_VIEWED, "", "", userId, nil)
-				message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_CHANNEL_VIEWED, "", "", userId, nil)
+			message.Add("channel_id", channelId)
-				message.Add("channel_id", channelId)
+			a.Publish(message)
-				a.Publish(message)
-			}
 		}
 	}
 	for _, channelId := range channelsToClearPushNotifications {

store/sqlstore/channel_store.go

@@ -1734,8 +1734,16 @@ func (s SqlChannelStore) UpdateLastViewedAt(channelIds []string, userId string)
 
 		selectQuery := "SELECT Id, LastPostAt, TotalMsgCount FROM Channels WHERE (" + selectIdQuery + ")"
 
-		if _, err := s.GetMaster().Select(&lastPostAtTimes, selectQuery, props); err != nil {
+		if _, err := s.GetMaster().Select(&lastPostAtTimes, selectQuery, props); err != nil || len(lastPostAtTimes) <= 0 {
-			result.Err = model.NewAppError("SqlChannelStore.UpdateLastViewedAt", "store.sql_channel.update_last_viewed_at.app_error", nil, "channel_ids="+strings.Join(channelIds, ",")+", user_id="+userId+", "+err.Error(), http.StatusInternalServerError)
+			var extra string
+			status := http.StatusInternalServerError
+			if err == nil {
+				status = http.StatusBadRequest
+				extra = "No channels found"
+			} else {
+				extra = err.Error()
+			}
+			result.Err = model.NewAppError("SqlChannelStore.UpdateLastViewedAt", "store.sql_channel.update_last_viewed_at.app_error", nil, "channel_ids="+strings.Join(channelIds, ",")+", user_id="+userId+", "+extra, status)
 			return
 		}