From f0ccaa89bf786d63e04006d2f1e2663ef0efabfd Mon Sep 17 00:00:00 2001 From: Claudio Costa Date: Tue, 16 Mar 2021 14:30:52 +0100 Subject: [PATCH] [MM-33603] Fix nil dereference panic in (*App).CreatePost() (#17124) * Fix possible nil dereference * fixed nil dereference for unfollowed thread Co-authored-by: Eli Yukelzon Co-authored-by: Mattermod --- app/notification.go | 15 ++++++++++----- app/post_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/app/notification.go b/app/notification.go index 6fdae2c8c7..f85e9e1eda 100644 --- a/app/notification.go +++ b/app/notification.go @@ -437,11 +437,16 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod } if sendEvent { message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_THREAD_UPDATED, team.Id, "", uid, nil) - userThread, _ := a.Srv().Store.Thread().GetThreadForUser(uid, channel.TeamId, thread.PostId, true) - a.sanitizeProfiles(userThread.Participants, false) - userThread.Post.SanitizeProps() - message.Add("thread", userThread.ToJson()) - a.Publish(message) + userThread, err := a.Srv().Store.Thread().GetThreadForUser(uid, channel.TeamId, thread.PostId, true) + if err != nil { + return nil, errors.Wrapf(err, "cannot get thread %q for user %q", thread.PostId, uid) + } + if userThread != nil { + a.sanitizeProfiles(userThread.Participants, false) + userThread.Post.SanitizeProps() + message.Add("thread", userThread.ToJson()) + a.Publish(message) + } } } diff --git a/app/post_test.go b/app/post_test.go index a816e1bdf9..faee08d8cc 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -6,6 +6,7 @@ package app import ( "fmt" "net/http" + "os" "sync" "testing" "time" @@ -1925,6 +1926,7 @@ func TestThreadMembership(t *testing.T) { func TestCollapsedThreadFetch(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ThreadAutoFollow = true *cfg.ServiceSettings.CollapsedThreads = model.COLLAPSED_THREADS_DEFAULT_ON @@ -1969,6 +1971,45 @@ func TestCollapsedThreadFetch(t *testing.T) { require.Len(t, l.Order, 1) require.NotEmpty(t, l.Posts[postRoot.Id].Participants[0].Email) }) + + t.Run("Should not panic on unexpected db error", func(t *testing.T) { + os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") + defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.FeatureFlags.CollapsedThreads = true + }) + + channel := th.CreateChannel(th.BasicTeam) + th.AddUserToChannel(user2, channel) + defer th.App.DeleteChannel(channel, user1.Id) + + postRoot, err := th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "root post", + }, channel, false, true) + require.Nil(t, err) + + // we introduce a race to trigger an unexpected error from the db side. + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + th.Server.Store.Post().PermanentDeleteByUser(user1.Id) + }() + + require.NotPanics(t, func() { + _, err = th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + RootId: postRoot.Id, + Message: fmt.Sprintf("@%s", user2.Username), + }, channel, false, true) + require.Nil(t, err) + }) + + wg.Wait() + }) } func TestReplyToPostWithLag(t *testing.T) {