diff options
author | Jaime Martinez <jmartinez@gitlab.com> | 2020-02-17 06:02:21 +0300 |
---|---|---|
committer | Jaime Martinez <jmartinez@gitlab.com> | 2020-02-17 06:02:21 +0300 |
commit | 52ec229a3cf3f5df82e31f00119259669113317c (patch) | |
tree | 3e499db93af04ac4d0d7cb38417b2201ad8d741d | |
parent | 7b41f6201a3cd3e828af5c09c3093ff791c32741 (diff) |
Remove ctxHTTPSKey from the context completely
-rw-r--r-- | internal/request/request.go | 31 | ||||
-rw-r--r-- | internal/request/request_test.go | 26 |
2 files changed, 14 insertions, 43 deletions
diff --git a/internal/request/request.go b/internal/request/request.go index ba56f45b..4e5c553d 100644 --- a/internal/request/request.go +++ b/internal/request/request.go @@ -5,15 +5,12 @@ import ( "net" "net/http" - log "github.com/sirupsen/logrus" - "gitlab.com/gitlab-org/gitlab-pages/internal/domain" ) type ctxKey string const ( - ctxHTTPSKey ctxKey = "https" ctxHostKey ctxKey = "host" ctxDomainKey ctxKey = "domain" @@ -25,28 +22,22 @@ const ( // WithHTTPSFlag saves https flag in request's context func WithHTTPSFlag(r *http.Request, https bool) *http.Request { - ctx := context.WithValue(r.Context(), ctxHTTPSKey, https) + // scheme should already be set but leaving this for testing scenarios that set this value explicitly + if r.URL.Scheme == "" { + if https { + r.URL.Scheme = SchemeHTTPS + } else { + r.URL.Scheme = SchemeHTTP + } + } - return r.WithContext(ctx) + return r } // IsHTTPS checks whether the request originated from HTTP or HTTPS. -// It reads the ctxHTTPSKey from the context and returns its value -// It also checks that r.URL.Scheme matches the value in ctxHTTPSKey for HTTPS requests -// TODO: remove the ctxHTTPSKey from the context https://gitlab.com/gitlab-org/gitlab-pages/issues/219 +// It checks the value from r.URL.Scheme func IsHTTPS(r *http.Request) bool { - https := r.Context().Value(ctxHTTPSKey).(bool) - - if https != (r.URL.Scheme == SchemeHTTPS) { - log.WithFields(log.Fields{ - "ctxHTTPSKey": https, - "scheme": r.URL.Scheme, - }).Warn("request: r.URL.Scheme does not match value in ctxHTTPSKey") - } - - // Returning the value of ctxHTTPSKey for now, can just switch to r.URL.Scheme == SchemeHTTPS later - // and later can remove IsHTTPS method altogether - return https + return r.URL.Scheme == SchemeHTTPS } // WithHostAndDomain saves host name and domain in the request's context diff --git a/internal/request/request_test.go b/internal/request/request_test.go index 3c0f970c..86601554 100644 --- a/internal/request/request_test.go +++ b/internal/request/request_test.go @@ -9,7 +9,6 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitlab-pages/internal/domain" - "gitlab.com/gitlab-org/gitlab-pages/internal/testhelpers" ) func TestWithHTTPSFlag(t *testing.T) { @@ -30,10 +29,9 @@ func TestIsHTTPS(t *testing.T) { hook := test.NewGlobal() tests := []struct { - name string - flag bool - scheme string - wantLogEntries string + name string + flag bool + scheme string }{ { name: "https", @@ -45,18 +43,6 @@ func TestIsHTTPS(t *testing.T) { flag: false, scheme: "http", }, - { - name: "scheme true but flag is false", - flag: false, - scheme: "https", - wantLogEntries: "request: r.URL.Scheme does not match value in ctxHTTPSKey", - }, - { - name: "scheme false but flag is true", - flag: true, - scheme: "http", - wantLogEntries: "request: r.URL.Scheme does not match value in ctxHTTPSKey", - }, } for _, tt := range tests { @@ -71,8 +57,6 @@ func TestIsHTTPS(t *testing.T) { got := IsHTTPS(httpsRequest) require.Equal(t, tt.flag, got) - - testhelpers.AssertLogContains(t, tt.wantLogEntries, hook.AllEntries()) }) } @@ -83,10 +67,6 @@ func TestPanics(t *testing.T) { require.NoError(t, err) require.Panics(t, func() { - IsHTTPS(r) - }) - - require.Panics(t, func() { GetHost(r) }) |