Switching redirect_location route to use httpservice client and adding caching. (#10118)

api4/system.go

@@ -14,8 +14,13 @@ import (
 	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/services/filesstore"
+	"github.com/mattermost/mattermost-server/utils"
 )
 
+const REDIRECT_LOCATION_CACHE_SIZE = 10000
+
+var redirectLocationDataCache = utils.NewLru(REDIRECT_LOCATION_CACHE_SIZE)
+
 func (api *API) InitSystem() {
 	api.BaseRoutes.System.Handle("/ping", api.ApiHandler(getSystemPing)).Methods("GET")
 
@@ -471,32 +476,42 @@ func testS3(c *Context, w http.ResponseWriter, r *http.Request) {
 func getRedirectLocation(c *Context, w http.ResponseWriter, r *http.Request) {
 	m := make(map[string]string)
 	m["location"] = ""
+
 	cfg := c.App.GetConfig()
 	if !*cfg.ServiceSettings.EnableLinkPreviews {
 		w.Write([]byte(model.MapToJson(m)))
 		return
 	}
+
 	url := r.URL.Query().Get("url")
 	if len(url) == 0 {
 		c.SetInvalidParam("url")
 		return
 	}
 
-	client := &http.Client{
+	if location, ok := openGraphDataCache.Get(url); ok {
-		CheckRedirect: func(req *http.Request, via []*http.Request) error {
+		m["location"] = location.(string)
+		w.Write([]byte(model.MapToJson(m)))
+		return
+	}
+
+	client := c.App.HTTPService.MakeClient(false)
+	client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
 		return http.ErrUseLastResponse
-		},
 	}
 
 	res, err := client.Head(url)
 	if err != nil {
-		// Always return a success status and a JSON string to limit the amount of information returned to a
+		// Cache failures to prevent retries.
-		// hacker attempting to use Mattermost to probe a private network.
+		redirectLocationDataCache.AddWithExpiresInSecs(url, "", 3600) // Expires after 1 hour
+		// Always return a success status and a JSON string to limit information returned to client.
 		w.Write([]byte(model.MapToJson(m)))
 		return
 	}
 
-	m["location"] = res.Header.Get("Location")
+	location := res.Header.Get("Location")
+	redirectLocationDataCache.AddWithExpiresInSecs(url, location, 3600) // Expires after 1 hour
+	m["location"] = location
 
 	w.Write([]byte(model.MapToJson(m)))
 	return

api4/system_test.go

@@ -751,6 +751,7 @@ func TestRedirectLocation(t *testing.T) {
 	}()
 
 	*th.App.Config().ServiceSettings.EnableLinkPreviews = true
+	*th.App.Config().ServiceSettings.AllowedUntrustedInternalConnections = "127.0.0.1"
 
 	_, resp := th.SystemAdminClient.GetRedirectLocation("https://mattermost.com/", "")
 	CheckNoError(t, resp)
@@ -760,9 +761,7 @@ func TestRedirectLocation(t *testing.T) {
 
 	actual, resp := th.SystemAdminClient.GetRedirectLocation(mockBitlyLink, "")
 	CheckNoError(t, resp)
-	if actual != expected {
+	assert.Equal(t, expected, actual)
-		t.Errorf("Expected %v but got %v.", expected, actual)
-	}
 
 	*th.App.Config().ServiceSettings.EnableLinkPreviews = false
 	actual, resp = th.SystemAdminClient.GetRedirectLocation("https://mattermost.com/", "")