diff --git a/server/channels/app/post_metadata.go b/server/channels/app/post_metadata.go index 3a5decbccd..50fbd64ece 100644 --- a/server/channels/app/post_metadata.go +++ b/server/channels/app/post_metadata.go @@ -34,6 +34,7 @@ type linkMetadataCache struct { OpenGraph *opengraph.OpenGraph PostImage *model.PostImage Permalink *model.Permalink + URL string } const MaxMetadataImageSize = MaxOpenGraphResponseSize @@ -812,6 +813,11 @@ func getLinkMetadataFromCache(requestURL string, timestamp int64) (*opengraph.Op return nil, nil, nil, false } + // Verify that the cached entry matches the requested URL + if cached.URL != requestURL { + return nil, nil, nil, false + } + return cached.OpenGraph, cached.PostImage, cached.Permalink, true } @@ -860,6 +866,7 @@ func cacheLinkMetadata(rctx request.CTX, requestURL string, timestamp int64, og OpenGraph: og, PostImage: image, Permalink: permalink, + URL: requestURL, } if err := platform.LinkCache().SetWithExpiry(strconv.FormatInt(model.GenerateLinkMetadataHash(requestURL, timestamp), 16), metadata, platform.LinkCacheDuration); err != nil { diff --git a/server/channels/app/post_metadata_test.go b/server/channels/app/post_metadata_test.go index 0e335df363..c76d48a5a3 100644 --- a/server/channels/app/post_metadata_test.go +++ b/server/channels/app/post_metadata_test.go @@ -3283,3 +3283,106 @@ func TestSanitizePostMetadataForUser(t *testing.T) { require.Equal(t, model.PostEmbedLink, sanitizedPost.Metadata.Embeds[0].Type) }) } + +func TestGetLinkMetadataFromCache(t *testing.T) { + mainHelper.Parallel(t) + + testURL := "https://example.com/test" + testTimestamp := int64(1640995200000) // 2022-01-01 00:00:00 UTC + + setup := func(t *testing.T) { + err := platform.PurgeLinkCache() + require.NoError(t, err) + } + + assertCached := func(t *testing.T, url string, expectedOG *opengraph.OpenGraph, expectedImage *model.PostImage, expectedPermalink *model.Permalink) { + og, image, permalink, found := getLinkMetadataFromCache(url, testTimestamp) + assert.True(t, found) + assert.Equal(t, expectedOG, og) + assert.Equal(t, expectedImage, image) + assert.Equal(t, expectedPermalink, permalink) + } + + assertNotCached := func(t *testing.T, url string) { + og, image, permalink, found := getLinkMetadataFromCache(url, testTimestamp) + assert.False(t, found) + assert.Nil(t, og) + assert.Nil(t, image) + assert.Nil(t, permalink) + } + + t.Run("should return false when cache is empty", func(t *testing.T) { + setup(t) + assertNotCached(t, testURL) + }) + + t.Run("should return cached data when URL matches", func(t *testing.T) { + setup(t) + expectedOG := &opengraph.OpenGraph{ + Title: "Test Title", + URL: testURL, + } + expectedImage := &model.PostImage{ + Width: 100, + Height: 200, + } + expectedPermalink := &model.Permalink{ + PreviewPost: &model.PreviewPost{ + PostID: "test-post-id", + }, + } + + ctx := request.TestContext(t) + cacheLinkMetadata(ctx, testURL, testTimestamp, expectedOG, expectedImage, expectedPermalink) + + assertCached(t, testURL, expectedOG, expectedImage, expectedPermalink) + }) + + t.Run("should return false when different url not cached", func(t *testing.T) { + setup(t) + + cachedURL := "https://example.com/cached" + requestedURL := "https://example.com/different" + + expectedOG := &opengraph.OpenGraph{ + Title: "Cached Title", + URL: cachedURL, + } + ctx := request.TestContext(t) + cacheLinkMetadata(ctx, cachedURL, testTimestamp, expectedOG, nil, nil) + + assertNotCached(t, requestedURL) + }) + + t.Run("should return false when different url not cached, even if hash collides with a cached url", func(t *testing.T) { + setup(t) + + url1 := "http://test.com/w4xg6hpvomau9j5iz371" + url2 := "http://collision.comupio5zw28x1m36c" + + hash1 := model.GenerateLinkMetadataHash(url1, testTimestamp) + hash2 := model.GenerateLinkMetadataHash(url2, testTimestamp) + assert.Equal(t, hash1, hash2, "URLs should have colliding hashes") + + og1 := &opengraph.OpenGraph{ + Title: "First URL Title", + URL: url1, + } + ctx := request.TestContext(t) + cacheLinkMetadata(ctx, url1, testTimestamp, og1, nil, nil) + + assertCached(t, url1, og1, nil, nil) + assertNotCached(t, url2) + }) + + t.Run("should handle cached nil values correctly", func(t *testing.T) { + setup(t) + + nilURL := "https://example.com/nil-test" + + ctx := request.TestContext(t) + cacheLinkMetadata(ctx, nilURL, testTimestamp, nil, nil, nil) + + assertCached(t, nilURL, nil, nil, nil) + }) +} diff --git a/server/channels/store/storetest/link_metadata_store.go b/server/channels/store/storetest/link_metadata_store.go index f32c3a2027..c98ca1747b 100644 --- a/server/channels/store/storetest/link_metadata_store.go +++ b/server/channels/store/storetest/link_metadata_store.go @@ -30,6 +30,7 @@ func TestLinkMetadataStore(t *testing.T, rctx request.CTX, ss store.Store) { t.Run("Save", func(t *testing.T) { testLinkMetadataStoreSave(t, rctx, ss) }) t.Run("Get", func(t *testing.T) { testLinkMetadataStoreGet(t, rctx, ss) }) t.Run("Types", func(t *testing.T) { testLinkMetadataStoreTypes(t, rctx, ss) }) + t.Run("HashCollisionHandling", func(t *testing.T) { testLinkMetadataStoreHashCollisionHandling(t, rctx, ss) }) } func testLinkMetadataStoreSave(t *testing.T, rctx request.CTX, ss store.Store) { @@ -252,3 +253,46 @@ func testLinkMetadataStoreTypes(t *testing.T, rctx request.CTX, ss store.Store) require.Nil(t, received.Data) }) } + +func testLinkMetadataStoreHashCollisionHandling(t *testing.T, rctx request.CTX, ss store.Store) { + testTimestamp := int64(1640995200000) // 2022-01-01 00:00:00 UTC + url1 := "http://test.com/w4xg6hpvomau9j5iz371" + url2 := "http://collision.comupio5zw28x1m36c" + + hash1 := model.GenerateLinkMetadataHash(url1, testTimestamp) + hash2 := model.GenerateLinkMetadataHash(url2, testTimestamp) + assert.Equal(t, hash1, hash2, "URLs should have colliding hashes") + + metadata1 := &model.LinkMetadata{ + URL: url1, + Timestamp: testTimestamp, + Type: model.LinkMetadataTypeOpengraph, + Data: &opengraph.OpenGraph{Title: "First URL Title"}, + } + _, err := ss.LinkMetadata().Save(metadata1) + require.NoError(t, err) + + retrieved, err := ss.LinkMetadata().Get(url1, testTimestamp) + require.NoError(t, err) + assert.Equal(t, url1, retrieved.URL) + assert.Equal(t, "First URL Title", retrieved.Data.(*opengraph.OpenGraph).Title) + + metadata2 := &model.LinkMetadata{ + URL: url2, + Timestamp: testTimestamp, + Type: model.LinkMetadataTypeOpengraph, + Data: &opengraph.OpenGraph{Title: "Second URL Title"}, + } + _, err = ss.LinkMetadata().Save(metadata2) + require.NoError(t, err) + + retrieved, err = ss.LinkMetadata().Get(url2, testTimestamp) + require.NoError(t, err) + assert.Equal(t, url2, retrieved.URL) + assert.Equal(t, "Second URL Title", retrieved.Data.(*opengraph.OpenGraph).Title) + + _, err = ss.LinkMetadata().Get(url1, testTimestamp) + require.Error(t, err) + var nfErr *store.ErrNotFound + assert.True(t, errors.As(err, &nfErr)) +}