From b6f2c8205e5b9c8b667c3c5135d38696528f05ee Mon Sep 17 00:00:00 2001
From: Christopher Poile <cpoile@gmail.com>
Date: Tue, 5 Sep 2023 14:04:14 -0400
Subject: [PATCH] MM-54219 - Fix: Improve limits on redirectLocationDataCache
 (#24429)

* ignore redirect locations over certain length

* maybe the link isn't necessary

* remove unrelated debugging line

* PR comments
---
 server/channels/api4/system.go      | 22 +++++++++++++++++-----
 server/channels/api4/system_test.go | 27 +++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/server/channels/api4/system.go b/server/channels/api4/system.go
index 9c8ad6fc65..343016ead2 100644
--- a/server/channels/api4/system.go
+++ b/server/channels/api4/system.go
@@ -26,9 +26,11 @@ import (
 )
 
 const (
-	RedirectLocationCacheSize = 10000
-	DefaultServerBusySeconds  = 3600
-	MaxServerBusySeconds      = 86400
+	RedirectLocationCacheSize     = 10000
+	RedirectLocationMaximumLength = 2100
+	RedirectLocationCacheExpiry   = 1 * time.Hour
+	DefaultServerBusySeconds      = 3600
+	MaxServerBusySeconds          = 86400
 )
 
 var redirectLocationDataCache = cache.NewLRU(cache.LRUOptions{
@@ -585,7 +587,7 @@ func getRedirectLocation(c *Context, w http.ResponseWriter, r *http.Request) {
 	res, err := client.Head(url)
 	if err != nil {
 		// Cache failures to prevent retries.
-		redirectLocationDataCache.SetWithExpiry(url, "", 1*time.Hour)
+		redirectLocationDataCache.SetWithExpiry(url, "", RedirectLocationCacheExpiry)
 		// Always return a success status and a JSON string to limit information returned to client.
 		w.Write([]byte(model.MapToJSON(m)))
 		return
@@ -596,7 +598,17 @@ func getRedirectLocation(c *Context, w http.ResponseWriter, r *http.Request) {
 	}()
 
 	location = res.Header.Get("Location")
-	redirectLocationDataCache.SetWithExpiry(url, location, 1*time.Hour)
+
+	// If the location length is > 2100, we can probably ignore. Fixes https://mattermost.atlassian.net/browse/MM-54219
+	if len(location) > RedirectLocationMaximumLength {
+		// Treating as a "failure". Cache failures to prevent retries.
+		redirectLocationDataCache.SetWithExpiry(url, "", RedirectLocationCacheExpiry)
+		// Always return a success status and a JSON string to limit information returned to client.
+		w.Write([]byte(model.MapToJSON(m)))
+		return
+	}
+
+	redirectLocationDataCache.SetWithExpiry(url, location, RedirectLocationCacheExpiry)
 	m["location"] = location
 
 	w.Write([]byte(model.MapToJSON(m)))
diff --git a/server/channels/api4/system_test.go b/server/channels/api4/system_test.go
index cff0e28c71..c571c6f59f 100644
--- a/server/channels/api4/system_test.go
+++ b/server/channels/api4/system_test.go
@@ -673,6 +673,33 @@ func TestRedirectLocation(t *testing.T) {
 	_, resp, err = client.GetRedirectLocation(context.Background(), "", "")
 	require.Error(t, err)
 	CheckUnauthorizedStatus(t, resp)
+
+	// Check that too-long redirect locations are ignored
+	*th.App.Config().ServiceSettings.EnableLinkPreviews = true
+	urlPrefix := "https://example.co"
+	almostTooLongUrl := urlPrefix + strings.Repeat("a", 2100-len(urlPrefix))
+	testServer2 := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) {
+		res.Header().Set("Location", almostTooLongUrl)
+		res.WriteHeader(http.StatusFound)
+		res.Write([]byte("body"))
+	}))
+	defer func() { testServer2.Close() }()
+
+	actual, _, err = th.SystemAdminClient.GetRedirectLocation(context.Background(), testServer2.URL, "")
+	require.NoError(t, err)
+	assert.Equal(t, almostTooLongUrl, actual)
+
+	tooLongUrl := urlPrefix + strings.Repeat("a", 2101-len(urlPrefix))
+	testServer3 := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) {
+		res.Header().Set("Location", tooLongUrl)
+		res.WriteHeader(http.StatusFound)
+		res.Write([]byte("body"))
+	}))
+	defer func() { testServer3.Close() }()
+
+	actual, _, err = th.SystemAdminClient.GetRedirectLocation(context.Background(), testServer3.URL, "")
+	require.NoError(t, err)
+	assert.Equal(t, "", actual)
 }
 
 func TestSetServerBusy(t *testing.T) {