From 8c62cf6e3d8a736547d9de57aa3f6a8dbbe7ee9f Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Wed, 22 Sep 2021 22:50:00 +0530 Subject: [PATCH] MM-38657 - Sentry crash: Fix nil dereference when post not found (#18440) * Revert "Add debug log for nil referencedPost (#18437)" This reverts commit e575315275481f5f339455d1b9f40c29667c842b. * Ignores post not found errors at a higher level And adds test ```release-note NONE ``` --- app/post_metadata.go | 40 ++++++++++++++------------------------- app/post_metadata_test.go | 17 +++++++++++++++++ 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/app/post_metadata.go b/app/post_metadata.go index 2d54d68bed..9e88e34db5 100644 --- a/app/post_metadata.go +++ b/app/post_metadata.go @@ -5,7 +5,6 @@ package app import ( "bytes" - "errors" "fmt" "image" "io" @@ -130,7 +129,12 @@ func (a *App) PreparePostForClient(originalPost *model.Post, isNewPost bool, isE firstLink, images := a.getFirstLinkAndImages(post.Message) if embed, err := a.getEmbedForPost(post, firstLink, isNewPost); err != nil { - mlog.Debug("Failed to get embedded content for a post", mlog.String("post_id", post.Id), mlog.Err(err)) + appErr, ok := err.(*model.AppError) + isNotFound := ok && appErr.StatusCode == http.StatusNotFound + // Ignore NotFound errors. + if !isNotFound { + mlog.Debug("Failed to get embedded content for a post", mlog.String("post_id", post.Id), mlog.Err(err)) + } } else if embed == nil { post.Metadata.Embeds = []*model.PostEmbed{} } else { @@ -291,8 +295,13 @@ func (a *App) getImagesForPost(post *model.Post, imageURLs []string, isNewPost b for _, imageURL := range imageURLs { if _, image, _, err := a.getLinkMetadata(imageURL, post.CreateAt, isNewPost, post.GetPreviewedPostProp()); err != nil { - mlog.Debug("Failed to get dimensions of an image in a post", - mlog.String("post_id", post.Id), mlog.String("image_url", imageURL), mlog.Err(err)) + appErr, ok := err.(*model.AppError) + isNotFound := ok && appErr.StatusCode == http.StatusNotFound + // Ignore NotFound errors. + if !isNotFound { + mlog.Debug("Failed to get dimensions of an image in a post", + mlog.String("post_id", post.Id), mlog.String("image_url", imageURL), mlog.Err(err)) + } } else if image != nil { images[imageURL] = image } @@ -479,46 +488,25 @@ func (a *App) getLinkMetadata(requestURL string, timestamp int64, isNewPost bool } var err error - if looksLikeAPermalink(requestURL, a.GetSiteURL()) && *a.Config().ServiceSettings.EnablePermalinkPreviews && a.Config().FeatureFlags.PermalinkPreviews { referencedPostID := requestURL[len(requestURL)-26:] referencedPost, appErr := a.GetSinglePost(referencedPostID) - // Ignore 'not found' errors; post could have been deleted via retention policy so we don't want to permanently log a warning. - // // TODO: Look into saving a value in the LinkMetadat.Data field to prevent perpetually re-querying for the deleted post. - if appErr != nil && appErr.StatusCode != http.StatusNotFound { + if appErr != nil { return nil, nil, nil, appErr } - if referencedPost == nil { - msg := "Referenced post is nil" - mlog.Debug(msg, mlog.String("post_id", referencedPostID)) - return nil, nil, nil, errors.New(msg) - } - referencedChannel, appErr := a.GetChannel(referencedPost.ChannelId) if appErr != nil { return nil, nil, nil, appErr } - if referencedChannel == nil { - msg := "Referenced channel is nil" - mlog.Debug(msg, mlog.String("channel_id", referencedPost.ChannelId)) - return nil, nil, nil, errors.New(msg) - } - referencedTeam, appErr := a.GetTeam(referencedChannel.TeamId) if appErr != nil { return nil, nil, nil, appErr } - if referencedTeam == nil { - msg := "Referenced team is nil" - mlog.Debug(msg, mlog.String("team_id", referencedChannel.TeamId)) - return nil, nil, nil, errors.New(msg) - } - permalink = &model.Permalink{PreviewPost: model.NewPreviewPost(referencedPost, referencedTeam, referencedChannel)} } else { diff --git a/app/post_metadata_test.go b/app/post_metadata_test.go index 7656c566dc..e524799414 100644 --- a/app/post_metadata_test.go +++ b/app/post_metadata_test.go @@ -2229,6 +2229,23 @@ func TestGetLinkMetadata(t *testing.T) { assert.NotNil(t, img) assert.NoError(t, err) }) + + t.Run("should throw error if post doesn't exist", func(t *testing.T) { + th := setup(t) + defer th.TearDown() + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.EnablePermalinkPreviews = true + *cfg.ServiceSettings.SiteURL = server.URL + cfg.FeatureFlags.PermalinkPreviews = true + }) + + requestURL := server.URL + "/pl/5rpoy4o3nbgwjm7gs4cm71h6ho" + timestamp := int64(1547510400000) + + _, _, _, err := th.App.getLinkMetadata(requestURL, timestamp, true, "") + assert.Error(t, err) + }) } func TestResolveMetadataURL(t *testing.T) {