From 041c8e695ec9246d6bb5436aa6e334c4ecda9ede Mon Sep 17 00:00:00 2001
From: tobi <31960611+tsmethurst@users.noreply.github.com>
Date: Mon, 13 Feb 2023 12:58:22 +0100
Subject: [PATCH] [chore] Do cache-control in a less silly way to avoid writing
 header twice (#1481)

* do cache-control in a less silly way to avoid writing header twice

* add comment back in
---
 internal/api/fileserver.go           | 31 +++++++++++++++++++---------
 internal/api/fileserver/servefile.go |  7 +------
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/internal/api/fileserver.go b/internal/api/fileserver.go
index fe4a781c3..042936551 100644
--- a/internal/api/fileserver.go
+++ b/internal/api/fileserver.go
@@ -21,6 +21,7 @@ package api
 import (
 	"github.com/gin-gonic/gin"
 	"github.com/superseriousbusiness/gotosocial/internal/api/fileserver"
+	"github.com/superseriousbusiness/gotosocial/internal/config"
 	"github.com/superseriousbusiness/gotosocial/internal/middleware"
 	"github.com/superseriousbusiness/gotosocial/internal/processing"
 	"github.com/superseriousbusiness/gotosocial/internal/router"
@@ -30,21 +31,31 @@ type Fileserver struct {
 	fileserver *fileserver.Module
 }
 
+// maxAge returns an appropriate max-age value for the
+// storage method that's being used.
+//
+// The default max-age is very long to reflect that we
+// never host different files at the same URL (since
+// ULIDs are generated per piece of media), so we can
+// easily prevent clients having to fetch files repeatedly.
+//
+// If we're using non-proxying s3, however, the max age is
+// significantly shorter, to ensure that clients don't
+// cache redirect responses to expired pre-signed URLs.
+func maxAge() string {
+	if config.GetStorageBackend() == "s3" && !config.GetStorageS3Proxy() {
+		return "max-age=86400" // 24h
+	}
+
+	return "max-age=604800" // 7d
+}
+
 func (f *Fileserver) Route(r router.Router, m ...gin.HandlerFunc) {
 	fileserverGroup := r.AttachGroup("fileserver")
 
 	// attach middlewares appropriate for this group
 	fileserverGroup.Use(m...)
-	fileserverGroup.Use(
-		// Since we'll never host different files at the same
-		// URL (bc the ULIDs are generated per piece of media),
-		// it's sensible and safe to use a long cache here, so
-		// that clients don't keep fetching files over + over again.
-		//
-		// Nevertheless, we should use 'private' to indicate
-		// that there might be media in there which are gated by ACLs.
-		middleware.CacheControl("private", "max-age=604800"),
-	)
+	fileserverGroup.Use(middleware.CacheControl("private", maxAge()))
 
 	f.fileserver.Route(fileserverGroup.Handle)
 }
diff --git a/internal/api/fileserver/servefile.go b/internal/api/fileserver/servefile.go
index 6b125debc..df3c83fe7 100644
--- a/internal/api/fileserver/servefile.go
+++ b/internal/api/fileserver/servefile.go
@@ -88,12 +88,7 @@ func (m *Module) ServeFile(c *gin.Context) {
 	}
 
 	if content.URL != nil {
-		// This is a non-local S3 file we're redirecting to.
-		// Rewrite the cache control header to reflect the
-		// TTL of the generated signed link, instead of the
-		// default very long cache.
-		const cacheControl = "private,max-age=86400" // 24h
-		c.Header("Cache-Control", cacheControl)
+		// This is a non-local, non-proxied S3 file we're redirecting to.
 		c.Redirect(http.StatusFound, content.URL.String())
 		return
 	}