Merge branch 'error-page-content-length' into 'master'

Bug fix and counters for static error pages Closes #134 See merge request !169

Merge branch 'error-page-content-length' into 'master'
Bug fix and counters for static error pages Closes #134 See merge request !169
9a5eed65 · Nick Thomas · 1efdfcff · 254e1220 · 9a5eed65 · 9a5eed65
Commit 9a5eed65 authored Jun 09, 2017 by Nick Thomas
Hide whitespace changes
Inline Side-by-side

Showing with 48 additions and 0 deletions

internal/staticpages/error_pages.go internal/staticpages/error_pages.go +19 -0

main_test.go main_test.go +29 -0

No files found.
--- a/internal/staticpages/error_pages.go
+++ b/internal/staticpages/error_pages.go
@@ -7,8 +7,24 @@ import (
 	"path/filepath"

 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
+
+	"github.com/prometheus/client_golang/prometheus"
+)
+
+var (
+	staticErrorResponses = prometheus.NewCounterVec(
+		prometheus.CounterOpts{
+			Name: "gitlab_workhorse_static_error_responses",
+			Help: "How many HTTP responses have been changed to a static error page, by HTTP status code.",
+		},
+		[]string{"code"},
+	)
 )

+func init() {
+	prometheus.MustRegister(staticErrorResponses)
+}
+
 type errorPageResponseWriter struct {
 	rw       http.ResponseWriter
 	status   int
@@ -43,9 +59,12 @@ func (s *errorPageResponseWriter) WriteHeader(status int) {
 		// check if custom error page exists, serve this page instead
 		if data, err := ioutil.ReadFile(errorPageFile); err == nil {
 			s.hijacked = true
+			staticErrorResponses.WithLabelValues(fmt.Sprintf("%d", s.status)).Inc()

 			helper.SetNoCacheHeaders(s.rw.Header())
 			s.rw.Header().Set("Content-Type", "text/html; charset=utf-8")
+			s.rw.Header().Set("Content-Length", fmt.Sprintf("%d", len(data)))
+			s.rw.Header().Del("Transfer-Encoding")
 			s.rw.WriteHeader(s.status)
 			s.rw.Write(data)
 			return

--- a/main_test.go
+++ b/main_test.go
@@ -293,6 +293,35 @@ func TestDeniedPublicUploadsFile(t *testing.T) {
 	}
 }

+func TestStaticErrorPage(t *testing.T) {
+	errorPageBody := `<html>
+<body>
+This is a static error page for code 499
+</body>
+</html>
+`
+	require.NoError(t, setupStaticFile("499.html", errorPageBody))
+	ts := testhelper.TestServerWithHandler(nil, func(w http.ResponseWriter, _ *http.Request) {
+		upstreamError := "499"
+		// This is the point of the test: the size of the upstream response body
+		// should be overridden.
+		require.NotEqual(t, len(upstreamError), len(errorPageBody))
+		w.WriteHeader(499)
+		_, err := w.Write([]byte(upstreamError))
+		require.NoError(t, err)
+	})
+	defer ts.Close()
+
+	ws := startWorkhorseServer(ts.URL)
+	defer ws.Close()
+
+	resourcePath := "/error-499"
+	resp, body := httpGet(t, ws.URL+resourcePath)
+
+	assert.Equal(t, 499, resp.StatusCode, "GET %q: status code", resourcePath)
+	assert.Equal(t, string(errorPageBody), body, "GET %q: response body", resourcePath)
+}
+
 var sendDataHeader = "Gitlab-Workhorse-Send-Data"

 func sendDataResponder(command string, literalJSON string) *httptest.Server {