From 52ec229a3cf3f5df82e31f00119259669113317c Mon Sep 17 00:00:00 2001
From: Jaime Martinez <jmartinez@gitlab.com>
Date: Mon, 17 Feb 2020 14:02:21 +1100
Subject: Remove ctxHTTPSKey from the context completely

---
 internal/request/request.go      | 31 +++++++++++--------------------
 internal/request/request_test.go | 26 +++-----------------------
 2 files changed, 14 insertions(+), 43 deletions(-)

(limited to 'internal')

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())
 		})
 	}
 
@@ -82,10 +66,6 @@ func TestPanics(t *testing.T) {
 	r, err := http.NewRequest("GET", "/", nil)
 	require.NoError(t, err)
 
-	require.Panics(t, func() {
-		IsHTTPS(r)
-	})
-
 	require.Panics(t, func() {
 		GetHost(r)
 	})
-- 
cgit v1.2.3