Merge branch 'sh-fix-redirect-logic' into 'master'

Fix and clarify redirect HTTP logic

See merge request !21

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"}}

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

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)
+}

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")