From de395b2806024af3846c93b3f10c9dc9416cd83d Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Fri, 10 Mar 2017 15:21:21 -0800
Subject: [PATCH] Fix and clarify redirect HTTP logic
redirect-http seemed to suggest the Pages daemon would redirect from HTTPS to
HTTP, but it seems that the opposite was implied.
Fixes issue manifested by https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests/1348
---
acceptance_test.go | 55 ++++++++++++++++++++++++++++++++++++++--------
app.go | 2 +-
helpers_test.go | 20 ++++++++++++++---
main.go | 2 +-
4 files changed, 65 insertions(+), 14 deletions(-)
diff --git a/acceptance_test.go b/acceptance_test.go
index e1a7167..24c0104 100644
--- a/acceptance_test.go
+++ b/acceptance_test.go
@@ -24,6 +24,11 @@ var listeners = []ListenSpec{
{"proxy", "::1", "37002"},
}
+var (
+ httpListener = listeners[0]
+ httpsListener = listeners[2]
+)
+
func skipUnlessEnabled(t *testing.T) {
if testing.Short() {
t.Log("Acceptance tests disabled")
@@ -38,16 +43,15 @@ func skipUnlessEnabled(t *testing.T) {
func TestUnknownHostReturnsNotFound(t *testing.T) {
skipUnlessEnabled(t)
- teardown := RunPagesProcess(t, *pagesBinary, listeners, "")
+ teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-redirect-http=false")
defer teardown()
for _, spec := range listeners {
rsp, err := GetPageFromListener(t, spec, "invalid.invalid", "")
- if assert.NoError(t, err) {
- rsp.Body.Close()
- assert.Equal(t, http.StatusNotFound, rsp.StatusCode)
- }
+ assert.NoError(t, err)
+ rsp.Body.Close()
+ assert.Equal(t, http.StatusNotFound, rsp.StatusCode)
}
}
@@ -59,13 +63,46 @@ func TestKnownHostReturns200(t *testing.T) {
for _, spec := range listeners {
rsp, err := GetPageFromListener(t, spec, "group.gitlab-example.com", "project/")
- if assert.NoError(t, err) {
- rsp.Body.Close()
- assert.Equal(t, http.StatusOK, rsp.StatusCode)
- }
+ assert.NoError(t, err)
+ rsp.Body.Close()
+ assert.Equal(t, http.StatusOK, rsp.StatusCode)
}
}
+func TestHttpToHttpsRedirectDisabled(t *testing.T) {
+ skipUnlessEnabled(t)
+ teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-redirect-http=false")
+ defer teardown()
+
+ rsp, err := GetRedirectPage(t, httpListener, "group.gitlab-example.com", "project/")
+ assert.NoError(t, err)
+ defer rsp.Body.Close()
+ assert.Equal(t, http.StatusOK, rsp.StatusCode)
+
+ rsp, err = GetPageFromListener(t, httpsListener, "group.gitlab-example.com", "project/")
+ assert.NoError(t, err)
+ defer rsp.Body.Close()
+ assert.Equal(t, http.StatusOK, rsp.StatusCode)
+}
+
+func TestHttpToHttpsRedirectEnabled(t *testing.T) {
+ skipUnlessEnabled(t)
+ teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-redirect-http=true")
+ defer teardown()
+
+ rsp, err := GetRedirectPage(t, httpListener, "group.gitlab-example.com", "project/")
+ assert.NoError(t, err)
+ defer rsp.Body.Close()
+ assert.Equal(t, http.StatusTemporaryRedirect, rsp.StatusCode)
+ assert.Equal(t, 1, len(rsp.Header["Location"]))
+ assert.Equal(t, "https://group.gitlab-example.com/project/", rsp.Header.Get("Location"))
+
+ rsp, err = GetPageFromListener(t, httpsListener, "group.gitlab-example.com", "project/")
+ assert.NoError(t, err)
+ defer rsp.Body.Close()
+ assert.Equal(t, http.StatusOK, rsp.StatusCode)
+}
+
func TestPrometheusMetricsCanBeScraped(t *testing.T) {
skipUnlessEnabled(t)
listener := []ListenSpec{{"http", "127.0.0.1", "37003"}}
diff --git a/app.go b/app.go
index 4ef5e73..351441d 100644
--- a/app.go
+++ b/app.go
@@ -52,7 +52,7 @@ func (a *theApp) serveContent(ww http.ResponseWriter, r *http.Request, https boo
defer metrics.SessionsActive.Dec()
// Add auto redirect
- if https && !a.RedirectHTTP {
+ if !https && a.RedirectHTTP {
u := *r.URL
u.Scheme = "https"
u.Host = r.Host
diff --git a/helpers_test.go b/helpers_test.go
index 99a44ea..60d0430 100644
--- a/helpers_test.go
+++ b/helpers_test.go
@@ -136,11 +136,11 @@ func (l ListenSpec) JoinHostPort() string {
// GetPageFromProcess to do a HTTP GET against a listener.
//
// If run as root via sudo, the gitlab-pages process will drop privileges
-func RunPagesProcess(t *testing.T, pagesPath string, listeners []ListenSpec, promPort string) (teardown func()) {
+func RunPagesProcess(t *testing.T, pagesPath string, listeners []ListenSpec, promPort string, extraArgs ...string) (teardown func()) {
_, err := os.Stat(pagesPath)
assert.NoError(t, err)
- args, tempfiles := getPagesArgs(t, listeners, promPort)
+ args, tempfiles := getPagesArgs(t, listeners, promPort, extraArgs)
cmd := exec.Command(pagesPath, args...)
cmd.Stdout = &tWriter{t}
cmd.Stderr = &tWriter{t}
@@ -168,7 +168,7 @@ func RunPagesProcess(t *testing.T, pagesPath string, listeners []ListenSpec, pro
}
}
-func getPagesArgs(t *testing.T, listeners []ListenSpec, promPort string) (args, tempfiles []string) {
+func getPagesArgs(t *testing.T, listeners []ListenSpec, promPort string, extraArgs []string) (args, tempfiles []string) {
var hasHTTPS bool
for _, spec := range listeners {
@@ -196,6 +196,8 @@ func getPagesArgs(t *testing.T, listeners []ListenSpec, promPort string) (args,
args = append(args, "-daemon-gid", "65534") // Root user can switch to "nobody"
}
+ args = append(args, extraArgs...)
+
return
}
@@ -214,3 +216,15 @@ func GetPageFromListener(t *testing.T, spec ListenSpec, host, urlsuffix string)
return InsecureHTTPSClient.Do(req)
}
+
+func GetRedirectPage(t *testing.T, spec ListenSpec, host, urlsuffix string) (*http.Response, error) {
+ url := spec.URL(urlsuffix)
+ req, err := http.NewRequest("GET", url, nil)
+ if err != nil {
+ return nil, err
+ }
+
+ req.Host = host
+
+ return InsecureHTTPSClient.Transport.RoundTrip(req)
+}
diff --git a/main.go b/main.go
index 172d41e..f79361b 100644
--- a/main.go
+++ b/main.go
@@ -16,7 +16,7 @@ var REVISION = "HEAD"
var (
pagesRootCert = flag.String("root-cert", "", "The default path to file certificate to serve static pages")
pagesRootKey = flag.String("root-key", "", "The default path to file certificate to serve static pages")
- redirectHTTP = flag.Bool("redirect-http", true, "Serve the pages under HTTP")
+ redirectHTTP = flag.Bool("redirect-http", false, "Redirect pages from HTTP to HTTPS")
useHTTP2 = flag.Bool("use-http2", true, "Enable HTTP2 support")
pagesRoot = flag.String("pages-root", "shared/pages", "The directory where pages are stored")
pagesDomain = flag.String("pages-domain", "gitlab-example.com", "The domain to serve static pages")
--
GitLab