Merge branch 'preserve-cache-headers-in-sendurl' into 'master'

Preserve original HTTP cache headers in sendurl

See merge request gitlab-org/gitlab-workhorse!428

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,10 +146,12 @@ 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 {
+		if !preserveHeaderKeys[key] {
 			w.Header()[key] = value
 		}
+	}
 	w.WriteHeader(resp.StatusCode)
 
 	defer resp.Body.Close()

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)