From 7057e0298b68bd5ddb59f8bcc40d4c0879213621 Mon Sep 17 00:00:00 2001
From: Travis Ralston <travpc@gmail.com>
Date: Fri, 16 Nov 2018 20:19:43 -0700
Subject: [PATCH] Introduce configurable timeouts for various HTTP requests

The default is no limit, so any limit is a good thing.

Fixes https://github.com/turt2live/matrix-media-repo/issues/66
Fixes https://github.com/turt2live/matrix-media-repo/issues/38
---
 config.sample.yaml                            | 14 ++++++++
 .../matrix-media-repo/common/config/config.go | 34 +++++++++++++------
 .../previewers/calculated_previewer.go        |  6 +++-
 .../previewers/opengraph_previewer.go         | 11 ++++--
 .../matrix-media-repo/matrix/client_server.go | 12 +++----
 .../matrix-media-repo/matrix/federation.go    |  8 +++--
 6 files changed, 63 insertions(+), 22 deletions(-)

diff --git a/config.sample.yaml b/config.sample.yaml
index 0d04119b..90be0208 100644
--- a/config.sample.yaml
+++ b/config.sample.yaml
@@ -230,3 +230,17 @@ quarantine:
   # only. Global administrators can quarantine any media (local or remote) regardless of this
   # flag.
   allowLocalAdmins: true
+
+# The various timeouts that the media repo will use.
+timeouts:
+  # The maximum amount of time the media repo should spend trying to fetch a resource that is
+  # being previewed.
+  urlPreviewTimeoutSeconds: 10
+
+  # The maximum amount of time the media repo will spend making remote requests to other repos
+  # or homeservers. This is primarily used to download media.
+  federationTimeoutSeconds: 120
+
+  # The maximum amount of time the media repo will spend talking to your configured homeservers.
+  # This is usually used to verify a user's identity.
+  clientServerTimeoutSeconds: 30
\ No newline at end of file
diff --git a/src/github.com/turt2live/matrix-media-repo/common/config/config.go b/src/github.com/turt2live/matrix-media-repo/common/config/config.go
index 587911be..8fa232cc 100644
--- a/src/github.com/turt2live/matrix-media-repo/common/config/config.go
+++ b/src/github.com/turt2live/matrix-media-repo/common/config/config.go
@@ -101,18 +101,25 @@ type QuarantineConfig struct {
 	AllowLocalAdmins  bool   `yaml:"allowLocalAdmins"`
 }
 
+type TimeoutsConfig struct {
+	UrlPreviews  int `yaml:"urlPreviewTimeoutSeconds"`
+	Federation   int `yaml:"federationTimeoutSeconds"`
+	ClientServer int `yaml:"clientServerTimeoutSeconds"`
+}
+
 type MediaRepoConfig struct {
-	General     *GeneralConfig      `yaml:"repo"`
-	Homeservers []*HomeserverConfig `yaml:"homeservers,flow"`
-	Admins      []string            `yaml:"admins,flow"`
-	Database    *DatabaseConfig     `yaml:"database"`
-	Uploads     *UploadsConfig      `yaml:"uploads"`
-	Downloads   *DownloadsConfig    `yaml:"downloads"`
-	Thumbnails  *ThumbnailsConfig   `yaml:"thumbnails"`
-	UrlPreviews *UrlPreviewsConfig  `yaml:"urlPreviews"`
-	RateLimit   *RateLimitConfig    `yaml:"rateLimit"`
-	Identicons  *IdenticonsConfig   `yaml:"identicons"`
-	Quarantine  *QuarantineConfig   `yaml:"quarantine"`
+	General        *GeneralConfig      `yaml:"repo"`
+	Homeservers    []*HomeserverConfig `yaml:"homeservers,flow"`
+	Admins         []string            `yaml:"admins,flow"`
+	Database       *DatabaseConfig     `yaml:"database"`
+	Uploads        *UploadsConfig      `yaml:"uploads"`
+	Downloads      *DownloadsConfig    `yaml:"downloads"`
+	Thumbnails     *ThumbnailsConfig   `yaml:"thumbnails"`
+	UrlPreviews    *UrlPreviewsConfig  `yaml:"urlPreviews"`
+	RateLimit      *RateLimitConfig    `yaml:"rateLimit"`
+	Identicons     *IdenticonsConfig   `yaml:"identicons"`
+	Quarantine     *QuarantineConfig   `yaml:"quarantine"`
+	TimeoutSeconds *TimeoutsConfig     `yaml:"timeouts"`
 }
 
 var instance *MediaRepoConfig
@@ -263,5 +270,10 @@ func NewDefaultConfig() *MediaRepoConfig {
 			ReplaceThumbnails: true,
 			ThumbnailPath:     "",
 		},
+		TimeoutSeconds: &TimeoutsConfig{
+			UrlPreviews:  10,
+			ClientServer: 30,
+			Federation:   120,
+		},
 	}
 }
diff --git a/src/github.com/turt2live/matrix-media-repo/controllers/preview_controller/previewers/calculated_previewer.go b/src/github.com/turt2live/matrix-media-repo/controllers/preview_controller/previewers/calculated_previewer.go
index 7c9a292f..9a5d00af 100644
--- a/src/github.com/turt2live/matrix-media-repo/controllers/preview_controller/previewers/calculated_previewer.go
+++ b/src/github.com/turt2live/matrix-media-repo/controllers/preview_controller/previewers/calculated_previewer.go
@@ -8,6 +8,7 @@ import (
 	"mime"
 	"net/http"
 	"strconv"
+	"time"
 
 	"github.com/ryanuber/go-glob"
 	"github.com/sirupsen/logrus"
@@ -60,7 +61,10 @@ func GenerateCalculatedPreview(urlStr string, log *logrus.Entry) (PreviewResult,
 
 func downloadFileContent(urlStr string, log *logrus.Entry) (*PreviewImage, error) {
 	log.Info("Fetching remote content...")
-	resp, err := http.Get(urlStr)
+	client := &http.Client{
+		Timeout: time.Duration(config.Get().TimeoutSeconds.UrlPreviews) * time.Second,
+	}
+	resp, err := client.Get(urlStr)
 	if err != nil {
 		return nil, err
 	}
diff --git a/src/github.com/turt2live/matrix-media-repo/controllers/preview_controller/previewers/opengraph_previewer.go b/src/github.com/turt2live/matrix-media-repo/controllers/preview_controller/previewers/opengraph_previewer.go
index dff79d26..7c7215d3 100644
--- a/src/github.com/turt2live/matrix-media-repo/controllers/preview_controller/previewers/opengraph_previewer.go
+++ b/src/github.com/turt2live/matrix-media-repo/controllers/preview_controller/previewers/opengraph_previewer.go
@@ -11,6 +11,7 @@ import (
 	"net/url"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/PuerkitoBio/goquery"
 	"github.com/dyatlov/go-opengraph/opengraph"
@@ -116,10 +117,16 @@ func doHttpGet(urlStr string, log *logrus.Entry) (*http.Response, error) {
 				return conn, nil
 			},
 		}
-		client := &http.Client{Transport: tr}
+		client := &http.Client{
+			Transport: tr,
+			Timeout:   time.Duration(config.Get().TimeoutSeconds.UrlPreviews) * time.Second,
+		}
 		resp, err = client.Get(urlStr)
 	} else {
-		resp, err = http.Get(urlStr)
+		client := &http.Client{
+			Timeout: time.Duration(config.Get().TimeoutSeconds.UrlPreviews) * time.Second,
+		}
+		resp, err = client.Get(urlStr)
 	}
 
 	return resp, err
diff --git a/src/github.com/turt2live/matrix-media-repo/matrix/client_server.go b/src/github.com/turt2live/matrix-media-repo/matrix/client_server.go
index 85d859ea..a186b973 100644
--- a/src/github.com/turt2live/matrix-media-repo/matrix/client_server.go
+++ b/src/github.com/turt2live/matrix-media-repo/matrix/client_server.go
@@ -8,12 +8,9 @@ import (
 	"time"
 
 	"github.com/pkg/errors"
+	"github.com/turt2live/matrix-media-repo/common/config"
 )
 
-var matrixHttpClient = &http.Client{
-	Timeout: 30 * time.Second,
-}
-
 // Based in part on https://github.com/matrix-org/gomatrix/blob/072b39f7fa6b40257b4eead8c958d71985c28bdd/client.go#L180-L243
 func doRequest(method string, urlStr string, body interface{}, result interface{}, accessToken string, ipAddr string) (error) {
 	var bodyBytes []byte
@@ -40,7 +37,10 @@ func doRequest(method string, urlStr string, body interface{}, result interface{
 		req.Header.Set("X-Real-IP", ipAddr)
 	}
 
-	res, err := matrixHttpClient.Do(req)
+	client := &http.Client{
+		Timeout: time.Duration(config.Get().TimeoutSeconds.ClientServer) * time.Second,
+	}
+	res, err := client.Do(req)
 	if res != nil {
 		defer res.Body.Close()
 	}
@@ -81,4 +81,4 @@ func makeUrl(parts ... string) string {
 		}
 	}
 	return res
-}
\ No newline at end of file
+}
diff --git a/src/github.com/turt2live/matrix-media-repo/matrix/federation.go b/src/github.com/turt2live/matrix-media-repo/matrix/federation.go
index b3f67918..ac4bb2bf 100644
--- a/src/github.com/turt2live/matrix-media-repo/matrix/federation.go
+++ b/src/github.com/turt2live/matrix-media-repo/matrix/federation.go
@@ -10,6 +10,7 @@ import (
 
 	"github.com/patrickmn/go-cache"
 	"github.com/sirupsen/logrus"
+	"github.com/turt2live/matrix-media-repo/common/config"
 )
 
 var apiUrlCacheInstance *cache.Cache
@@ -52,7 +53,7 @@ func GetServerApiUrl(hostname string) (string, error) {
 		// Trim off the trailing period if there is one (golang doesn't like this)
 		realAddr := addrs[0].Target
 		if realAddr[len(realAddr)-1:] == "." {
-			realAddr = realAddr[0:len(realAddr)-1]
+			realAddr = realAddr[0 : len(realAddr)-1]
 		}
 		url := fmt.Sprintf("https://%s:%d", realAddr, addrs[0].Port)
 		apiUrlCacheInstance.Set(hostname, url, cache.DefaultExpiration)
@@ -72,7 +73,10 @@ func FederatedGet(url string, realHost string) (*http.Response, error) {
 	transport := &http.Transport{
 		// Based on https://github.com/matrix-org/gomatrixserverlib/blob/51152a681e69a832efcd934b60080b92bc98b286/client.go#L74-L90
 		DialTLS: func(network, addr string) (net.Conn, error) {
-			rawconn, err := net.Dial(network, addr)
+			dialer := &net.Dialer{
+				Timeout: time.Duration(config.Get().TimeoutSeconds.Federation) * time.Second,
+			}
+			rawconn, err := dialer.Dial(network, addr)
 			if err != nil {
 				return nil, err
 			}
-- 
GitLab