[bugfix] Set Vary header correctly on cache-control (#1988)

* [bugfix] Set Vary header correctly on cache-control

* Prefer activitypub types on AP endpoints

* use immutable on file server, vary by range

* vary auth on Accept

internal/api/activitypub.go

@@ -41,11 +41,13 @@ func (a *ActivityPub) Route(r router.Router, m ...gin.HandlerFunc) {
 	usersGroup := r.AttachGroup("users")
 
 	// attach shared, non-global middlewares to both of these groups
-	cacheControlMiddleware := middleware.CacheControl("no-store")
+	ccMiddleware := middleware.CacheControl(middleware.CacheControlConfig{
+		Directives: []string{"no-store"},
+	})
 	emojiGroup.Use(m...)
 	usersGroup.Use(m...)
-	emojiGroup.Use(a.signatureCheckMiddleware, cacheControlMiddleware)
-	usersGroup.Use(a.signatureCheckMiddleware, cacheControlMiddleware)
+	emojiGroup.Use(a.signatureCheckMiddleware, ccMiddleware)
+	usersGroup.Use(a.signatureCheckMiddleware, ccMiddleware)
 
 	a.emoji.Route(emojiGroup.Handle)
 	a.users.Route(usersGroup.Handle)
@@ -53,8 +55,17 @@ func (a *ActivityPub) Route(r router.Router, m ...gin.HandlerFunc) {
 
 // Public key endpoint requires different middleware + cache policies from other AP endpoints.
 func (a *ActivityPub) RoutePublicKey(r router.Router, m ...gin.HandlerFunc) {
+	// Create grouping for the 'users/[username]/main-key' prefix.
 	publicKeyGroup := r.AttachGroup(publickey.PublicKeyPath)
-	publicKeyGroup.Use(a.signatureCheckMiddleware, middleware.CacheControl("public,max-age=604800"))
+
+	// Attach middleware allowing public cacheing of main-key.
+	ccMiddleware := middleware.CacheControl(middleware.CacheControlConfig{
+		Directives: []string{"public", "max-age=604800"},
+		Vary:       []string{"Accept", "Accept-Encoding"},
+	})
+	publicKeyGroup.Use(m...)
+	publicKeyGroup.Use(a.signatureCheckMiddleware, ccMiddleware)
+
 	a.publicKey.Route(publicKeyGroup.Handle)
 }
 

internal/api/activitypub/emoji/emojiget.go

@@ -36,7 +36,7 @@ func (m *Module) EmojiGetHandler(c *gin.Context) {
 		return
 	}
 
-	format, err := apiutil.NegotiateAccept(c, apiutil.ActivityPubAcceptHeaders...)
+	format, err := apiutil.NegotiateAccept(c, apiutil.ActivityPubHeaders...)
 	if err != nil {
 		apiutil.ErrorHandler(c, gtserror.NewErrorNotAcceptable(err, err.Error()), m.processor.InstanceGetV1)
 		return

internal/api/activitypub/publickey/publickeyget.go

@@ -42,7 +42,7 @@ func (m *Module) PublicKeyGETHandler(c *gin.Context) {
 		return
 	}
 
-	format, err := apiutil.NegotiateAccept(c, apiutil.HTMLOrActivityPubHeaders...)
+	format, err := apiutil.NegotiateAccept(c, apiutil.ActivityPubOrHTMLHeaders...)
 	if err != nil {
 		apiutil.ErrorHandler(c, gtserror.NewErrorNotAcceptable(err, err.Error()), m.processor.InstanceGetV1)
 		return

internal/api/activitypub/users/featured.go

@@ -67,7 +67,7 @@ func (m *Module) FeaturedCollectionGETHandler(c *gin.Context) {
 		return
 	}
 
-	format, err := apiutil.NegotiateAccept(c, apiutil.HTMLOrActivityPubHeaders...)
+	format, err := apiutil.NegotiateAccept(c, apiutil.ActivityPubOrHTMLHeaders...)
 	if err != nil {
 		apiutil.ErrorHandler(c, gtserror.NewErrorNotAcceptable(err, err.Error()), m.processor.InstanceGetV1)
 		return

internal/api/activitypub/users/followers.go

@@ -38,7 +38,7 @@ func (m *Module) FollowersGETHandler(c *gin.Context) {
 		return
 	}
 
-	format, err := apiutil.NegotiateAccept(c, apiutil.HTMLOrActivityPubHeaders...)
+	format, err := apiutil.NegotiateAccept(c, apiutil.ActivityPubOrHTMLHeaders...)
 	if err != nil {
 		apiutil.ErrorHandler(c, gtserror.NewErrorNotAcceptable(err, err.Error()), m.processor.InstanceGetV1)
 		return

internal/api/activitypub/users/following.go

@@ -38,7 +38,7 @@ func (m *Module) FollowingGETHandler(c *gin.Context) {
 		return
 	}
 
-	format, err := apiutil.NegotiateAccept(c, apiutil.HTMLOrActivityPubHeaders...)
+	format, err := apiutil.NegotiateAccept(c, apiutil.ActivityPubOrHTMLHeaders...)
 	if err != nil {
 		apiutil.ErrorHandler(c, gtserror.NewErrorNotAcceptable(err, err.Error()), m.processor.InstanceGetV1)
 		return

internal/api/activitypub/users/outboxget.go

@@ -93,7 +93,7 @@ func (m *Module) OutboxGETHandler(c *gin.Context) {
 		return
 	}
 
-	format, err := apiutil.NegotiateAccept(c, apiutil.HTMLOrActivityPubHeaders...)
+	format, err := apiutil.NegotiateAccept(c, apiutil.ActivityPubOrHTMLHeaders...)
 	if err != nil {
 		apiutil.ErrorHandler(c, gtserror.NewErrorNotAcceptable(err, err.Error()), m.processor.InstanceGetV1)
 		return

internal/api/activitypub/users/repliesget.go

@@ -108,7 +108,7 @@ func (m *Module) StatusRepliesGETHandler(c *gin.Context) {
 		return
 	}
 
-	format, err := apiutil.NegotiateAccept(c, apiutil.HTMLOrActivityPubHeaders...)
+	format, err := apiutil.NegotiateAccept(c, apiutil.ActivityPubOrHTMLHeaders...)
 	if err != nil {
 		apiutil.ErrorHandler(c, gtserror.NewErrorNotAcceptable(err, err.Error()), m.processor.InstanceGetV1)
 		return

internal/api/activitypub/users/statusget.go

@@ -46,7 +46,7 @@ func (m *Module) StatusGETHandler(c *gin.Context) {
 		return
 	}
 
-	format, err := apiutil.NegotiateAccept(c, apiutil.HTMLOrActivityPubHeaders...)
+	format, err := apiutil.NegotiateAccept(c, apiutil.ActivityPubOrHTMLHeaders...)
 	if err != nil {
 		apiutil.ErrorHandler(c, gtserror.NewErrorNotAcceptable(err, err.Error()), m.processor.InstanceGetV1)
 		return

internal/api/activitypub/users/userget.go

@@ -46,7 +46,7 @@ func (m *Module) UsersGETHandler(c *gin.Context) {
 		return
 	}
 
-	format, err := apiutil.NegotiateAccept(c, apiutil.HTMLOrActivityPubHeaders...)
+	format, err := apiutil.NegotiateAccept(c, apiutil.ActivityPubOrHTMLHeaders...)
 	if err != nil {
 		apiutil.ErrorHandler(c, gtserror.NewErrorNotAcceptable(err, err.Error()), m.processor.InstanceGetV1)
 		return

internal/api/auth.go

@@ -43,13 +43,16 @@ func (a *Auth) Route(r router.Router, m ...gin.HandlerFunc) {
 
 	// instantiate + attach shared, non-global middlewares to both of these groups
 	var (
-		cacheControlMiddleware = middleware.CacheControl("private", "max-age=120")
+		ccMiddleware = middleware.CacheControl(middleware.CacheControlConfig{
+			Directives: []string{"private", "max-age=120"},
+			Vary:       []string{"Accept", "Accept-Encoding"},
+		})
 		sessionMiddleware = middleware.Session(a.sessionName, a.routerSession.Auth, a.routerSession.Crypt)
 	)
 	authGroup.Use(m...)
 	oauthGroup.Use(m...)
-	authGroup.Use(cacheControlMiddleware, sessionMiddleware)
-	oauthGroup.Use(cacheControlMiddleware, sessionMiddleware)
+	authGroup.Use(ccMiddleware, sessionMiddleware)
+	oauthGroup.Use(ccMiddleware, sessionMiddleware)
 
 	a.auth.RouteAuth(authGroup.Handle)
 	a.auth.RouteOauth(oauthGroup.Handle)

internal/api/client.go

@@ -83,7 +83,10 @@ func (c *Client) Route(r router.Router, m ...gin.HandlerFunc) {
 	apiGroup.Use(m...)
 	apiGroup.Use(
 		middleware.TokenCheck(c.db, c.processor.OAuthValidateBearerToken),
-		middleware.CacheControl("no-store"), // never cache api responses
+		middleware.CacheControl(middleware.CacheControlConfig{
+			// Never cache client api responses.
+			Directives: []string{"no-store"},
+		}),
 	)
 
 	// for each client api module, pass it the Handle function

internal/api/fileserver.go

@@ -36,8 +36,8 @@ func (f *Fileserver) Route(r router.Router, m ...gin.HandlerFunc) {
 	// Attach middlewares appropriate for this group.
 	fileserverGroup.Use(m...)
 	// If we're using local storage or proxying s3, we can set a
-	// long max-age on all file requests to reflect that we
-	// never host different files at the same URL (since
+	// long max-age + immutable on all file requests 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.
 	//
@@ -45,9 +45,18 @@ func (f *Fileserver) Route(r router.Router, m ...gin.HandlerFunc) {
 	// must be set dynamically within the request handler,
 	// based on how long the signed URL has left to live before
 	// it expires. This ensures that clients won't cache expired
-	// links. This is done within fileserver/servefile.go.
+	// links. This is done within fileserver/servefile.go, so we
+	// should not set the middleware here in that case.
+	//
+	// See:
+	//
+	// - https://developer.mozilla.org/en-US/docs/Web/HTTP/Caching#avoiding_revalidation
+	// - https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control#immutable
 	if config.GetStorageBackend() == "local" || config.GetStorageS3Proxy() {
-		fileserverGroup.Use(middleware.CacheControl("private", "max-age=604800")) // 7d
+		fileserverGroup.Use(middleware.CacheControl(middleware.CacheControlConfig{
+			Directives: []string{"private", "max-age=604800", "immutable"},
+			Vary:       []string{"Range"}, // Cache partial ranges separately.
+		}))
 	}
 
 	f.fileserver.Route(fileserverGroup.Handle)

internal/api/fileserver/servefile.go

@@ -95,7 +95,7 @@ func (m *Module) ServeFile(c *gin.Context) {
 		// Derive the max-age value from how long the link has left until
 		// it expires.
 		maxAge := int(time.Until(content.URL.Expiry).Seconds())
-		c.Header("Cache-Control", "private,max-age="+strconv.Itoa(maxAge))
+		c.Header("Cache-Control", "private, max-age="+strconv.Itoa(maxAge)+", immutable")
 		c.Redirect(http.StatusFound, content.URL.String())
 		return
 	}

internal/api/nodeinfo.go

@@ -36,8 +36,11 @@ func (w *NodeInfo) Route(r router.Router, m ...gin.HandlerFunc) {
 	// attach middlewares appropriate for this group
 	nodeInfoGroup.Use(m...)
 	nodeInfoGroup.Use(
-		// allow cache for 2 minutes
-		middleware.CacheControl("public", "max-age=120"),
+		// Allow public cache for 2 minutes.
+		middleware.CacheControl(middleware.CacheControlConfig{
+			Directives: []string{"public", "max-age=120"},
+			Vary:       []string{"Accept-Encoding"},
+		}),
 	)
 
 	w.nodeInfo.Route(nodeInfoGroup.Handle)

internal/api/util/negotiate.go

@@ -25,12 +25,6 @@
 	"github.com/gin-gonic/gin"
 )
 
-// ActivityPubAcceptHeaders represents the Accept headers mentioned here:
-var ActivityPubAcceptHeaders = []MIME{
-	AppActivityJSON,
-	AppActivityLDJSON,
-}
-
 // JSONAcceptHeaders is a slice of offers that just contains application/json types.
 var JSONAcceptHeaders = []MIME{
 	AppJSON,
@@ -59,12 +53,34 @@
 }
 
 // HTMLOrActivityPubHeaders matches text/html first, then activitypub types.
-// This is useful for user URLs that a user might go to in their browser.
+// This is useful for user URLs that a user might go to in their browser,
+// but which should also be able to serve ActivityPub as a fallback.
+//
 // https://www.w3.org/TR/activitypub/#retrieving-objects
 var HTMLOrActivityPubHeaders = []MIME{
 	TextHTML,
-	AppActivityJSON,
 	AppActivityLDJSON,
+	AppActivityJSON,
+}
+
+// ActivityPubOrHTMLHeaders matches activitypub types first, then text/html.
+// This is useful for URLs that should serve ActivityPub by default, but
+// which a user might also go to in their browser sometimes.
+//
+// https://www.w3.org/TR/activitypub/#retrieving-objects
+var ActivityPubOrHTMLHeaders = []MIME{
+	AppActivityLDJSON,
+	AppActivityJSON,
+	TextHTML,
+}
+
+// ActivityPubHeaders matches only activitypub Accept headers.
+// This is useful for URLs should only serve ActivityPub.
+//
+// https://www.w3.org/TR/activitypub/#retrieving-objects
+var ActivityPubHeaders = []MIME{
+	AppActivityLDJSON,
+	AppActivityJSON,
 }
 
 var HostMetaHeaders = []MIME{

internal/api/wellknown.go

@@ -40,8 +40,11 @@ func (w *WellKnown) Route(r router.Router, m ...gin.HandlerFunc) {
 	// attach middlewares appropriate for this group
 	wellKnownGroup.Use(m...)
 	wellKnownGroup.Use(
-		// allow .well-known responses to be cached for 2 minutes
-		middleware.CacheControl("public", "max-age=120"),
+		// Allow public cache for 2 minutes.
+		middleware.CacheControl(middleware.CacheControlConfig{
+			Directives: []string{"public", "max-age=120"},
+			Vary:       []string{"Accept-Encoding"},
+		}),
 	)
 
 	w.nodeInfo.Route(wellKnownGroup.Handle)

internal/middleware/cachecontrol.go

@@ -23,12 +23,53 @@
 	"github.com/gin-gonic/gin"
 )
 
-// CacheControl returns a new gin middleware which allows callers to control cache settings on response headers.
+type CacheControlConfig struct {
+	// Slice of Cache-Control directives, which will be
+	// joined comma-separated and served as the value of
+	// the Cache-Control header.
 	//
-// For directives, see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control
-func CacheControl(directives ...string) gin.HandlerFunc {
-	ccHeader := strings.Join(directives, ", ")
+	// If no directives are set, the Cache-Control header
+	// will not be sent in the response at all.
+	//
+	// For possible Cache-Control directive values, see:
+	// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control
+	Directives []string
+
+	// Slice of Vary header values, which will be joined
+	// comma-separated and served as the value of the Vary
+	// header in the response.
+	//
+	// If no Vary header values are supplied, then the
+	// Vary header will be omitted in the response.
+	//
+	// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Vary
+	Vary []string
+}
+
+// CacheControl returns a new gin middleware which allows
+// routes to control cache settings on response headers.
+func CacheControl(config CacheControlConfig) gin.HandlerFunc {
+	if len(config.Directives) == 0 {
+		// No Cache-Control directives provided,
+		// return empty/stub function.
+		return func(c *gin.Context) {}
+	}
+
+	// Cache control is usually done on hot paths so
+	// parse vars outside of the returned function.
+	var (
+		ccHeader   = strings.Join(config.Directives, ", ")
+		varyHeader = strings.Join(config.Vary, ", ")
+	)
+
+	if varyHeader == "" {
 		return func(c *gin.Context) {
 			c.Header("Cache-Control", ccHeader)
 		}
 	}
+
+	return func(c *gin.Context) {
+		c.Header("Cache-Control", ccHeader)
+		c.Header("Vary", varyHeader)
+	}
+}

internal/web/web.go

@@ -91,7 +91,9 @@ func (m *Module) Route(r router.Router, mi ...gin.HandlerFunc) {
 	// can still be served
 	profileGroup := r.AttachGroup(profileGroupPath)
 	profileGroup.Use(mi...)
-	profileGroup.Use(middleware.SignatureCheck(m.isURIBlocked), middleware.CacheControl("no-store"))
+	profileGroup.Use(middleware.SignatureCheck(m.isURIBlocked), middleware.CacheControl(middleware.CacheControlConfig{
+		Directives: []string{"no-store"},
+	}))
 	profileGroup.Handle(http.MethodGet, "", m.profileGETHandler) // use empty path here since it's the base of the group
 	profileGroup.Handle(http.MethodGet, statusPath, m.threadGETHandler)