Preserve original HTTP cache headers in sendurl

There are a few cases for serving uploads:

1. File storage. Rails asks Workhorse to serve the file.
2. Object storage. Rails redirects (via SendFileUpload) to the object
   storage host.
2. Object storage with `proxy_download` enabled. Rails asks Workhorse to
   proxy the download from the object storage host.

Rails also sets caching headers for uploads. In case 1, the reverse
proxy will keep those headers. In case 2, the headers are whatever the
object storage provider sets.

Case 3 is changed here. Previously, it would use the cache headers from
the object storage provider. Now, it keeps the cache headers from Rails
instead. This is better because:

1. Cache headers on the object storage provider can be hard to
   configure.
2. Even if we ask users to manually configure them, they may get it
   wrong and inadvertently allow private resources to be cached by
   proxies.
3. Even if we ask users to manually configure them and they get it
   right, they will also need to track any updates the Rails application
   makes to the cache headers it sends.

We could solve these by trying to automatically set the metadata policy
on the object storage bucket, which would also help with case 2
above. However, that has its own pitfalls. We could, for instance, say
that `uploads/-/system/user` is public with an expiry of five minutes,
and that's fairly straightforward. But then if we need to update that
policy in future to make it public with an expiry of one minute, we are
introducing coordination issues.

This would get even more complicated if we allowed caching uploads from
public projects. If the project's visibility changed, we'd need to
update the object storage metadata too. So it's a tricky problem, and
this is a relatively small code change to at least solve one case.

CHANGELOG

@@ -2,6 +2,10 @@
 
 Formerly known as 'gitlab-git-http-server'.
 
+Next
+
+- Preserve original HTTP cache headers when proxying with sendurl
+
 v8.12.0
 
 - Fix health checks routes incorrectly intercepting errors !424

internal/sendurl/sendurl.go

@@ -36,6 +36,14 @@ var rangeHeaderKeys = []string{
 	"Range",
 }
 
+// Keep cache headers from the original response, not the proxied response. The
+// original response comes from the Rails application, which should be the
+// source of truth for caching.
+var preserveHeaderKeys = map[string]bool{
+	"Cache-Control": true,
+	"Expires":       true,
+}
+
 // httpTransport defines a http.Transport with values
 // that are more restrictive than for http.DefaultTransport,
 // they define shorter TLS Handshake, and more aggressive connection closing
@@ -138,9 +146,11 @@ func (e *entry) Inject(w http.ResponseWriter, r *http.Request, sendData string)
 		return
 	}
 
-	// copy response headers and body
+	// copy response headers and body, except the headers from preserveHeaderKeys
 	for key, value := range resp.Header {
-		w.Header()[key] = value
+		if !preserveHeaderKeys[key] {
+			w.Header()[key] = value
+		}
 	}
 	w.WriteHeader(resp.StatusCode)
 

internal/sendurl/sendurl_test.go

@@ -30,6 +30,8 @@ func testEntryServer(t *testing.T, requestURL string, httpHeaders http.Header, a
 
 		// The server returns a Content-Disposition
 		w.Header().Set("Content-Disposition", "attachment; filename=\"archive.txt\"")
+		w.Header().Set("Cache-Control", "no-store")
+		w.Header().Set("Expires", "")
 
 		SendURL.Inject(w, r, data)
 	}
@@ -44,6 +46,8 @@ func testEntryServer(t *testing.T, requestURL string, httpHeaders http.Header, a
 		require.NoError(t, err)
 
 		w.Header().Set("Etag", testDataEtag)
+		w.Header().Set("Cache-Control", "public")
+		w.Header().Set("Expires", "Wed, 21 Oct 2015 07:28:00 GMT")
 
 		http.ServeContent(w, r, "archive.txt", time.Now(), tempFile)
 	}
@@ -160,6 +164,18 @@ func TestAccessingAllowedRedirectWithChunkOfDataWithSendURL(t *testing.T) {
 	testhelper.AssertResponseBody(t, response, "23")
 }
 
+func TestOriginalCacheHeadersPreservedWithSendURL(t *testing.T) {
+	response := testEntryServer(t, "/get/redirect", nil, true)
+	testhelper.AssertResponseCode(t, response, http.StatusOK)
+
+	testhelper.AssertResponseWriterHeader(t, response,
+		"Cache-Control",
+		"no-store")
+	testhelper.AssertResponseWriterHeader(t, response,
+		"Expires",
+		"")
+}
+
 func TestDownloadingNonExistingFileUsingSendURL(t *testing.T) {
 	response := testEntryServer(t, "/invalid/path", nil, false)
 	testhelper.AssertResponseCode(t, response, http.StatusNotFound)