Add URL validation to LinkMetadata cache and store (#31814) (#33453)

Automatic Merge

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 {

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)
+	})
+}

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))
+}