Add rewrites for ssl so apache behaves properly (#1124)

* Add rewrites for ssl so apache behaves properly
* Refactor testcommon.GetLocalHTTPResponse() to return resp for extra checking
* Add TestHttpsRedirection()

containers/ddev-webserver/files/etc/apache2/apache-site-default.conf

@@ -8,6 +8,16 @@
 	# However, you must set it for any further virtual host explicitly.
 	#ServerName www.example.com
 
+    # Workaround from https://mail-archives.apache.org/mod_mbox/httpd-users/201403.mbox/%3C49404A24C7FAD94BB7B45E86A9305F6214D04652@MSGEXSV21103.ent.wfb.bank.corp%3E
+    # See also https://gist.github.com/nurtext/b6ac07ac7d8c372bc8eb
+
+    RewriteEngine On
+    RewriteCond %{HTTP:X-Forwarded-Proto} =https
+    RewriteCond    %{DOCUMENT_ROOT}%{REQUEST_FILENAME} -d
+    RewriteRule    ^(.+[^/])$           https://%{HTTP_HOST}$1/ [redirect,last]
+
+    SetEnvIf X-Forwarded-Proto "https" HTTPS=on
+
 	ServerAdmin webmaster@localhost
 	DocumentRoot $WEBSERVER_DOCROOT
 	<Directory "$WEBSERVER_DOCROOT/">

pkg/ddevapp/ddevapp_test.go

@@ -1327,6 +1327,88 @@ func TestListWithoutDir(t *testing.T) {
 	assert.NoError(err)
 }
 
+type URLRedirectExpectations struct {
+	scheme              string
+	uri                 string
+	expectedRedirectURI string
+}
+
+// TestHttpsRedirection tests to make sure that webserver and php redirect to correct
+// scheme (http or https).
+func TestHttpsRedirection(t *testing.T) {
+	// Set up tests and give ourselves a working directory.
+	assert := asrt.New(t)
+	testcommon.ClearDockerEnv()
+	packageDir, _ := os.Getwd()
+
+	testDir := testcommon.CreateTmpDir("TestHttpsRedirection")
+	defer testcommon.CleanupDir(testDir)
+	appDir := filepath.Join(testDir, "proj")
+	err := fileutil.CopyDir(filepath.Join(packageDir, "testdata", "TestHttpsRedirection"), appDir)
+	assert.NoError(err)
+	err = os.Chdir(appDir)
+	assert.NoError(err)
+
+	app, err := ddevapp.NewApp(appDir, ddevapp.DefaultProviderName)
+	assert.NoError(err)
+	app.Name = "proj"
+	app.Type = "php"
+
+	expectations := []URLRedirectExpectations{
+		{"https", "/subdir", "/subdir/"},
+		{"https", "/redir_abs.php", "/landed.php"},
+		{"https", "/redir_relative.php", "/landed.php"},
+		{"http", "/subdir", "/subdir/"},
+		{"http", "/redir_abs.php", "/landed.php"},
+		{"http", "/redir_relative.php", "/landed.php"},
+	}
+
+	for _, webserverType := range []string{"nginx-fpm", "apache-fpm", "apache-cgi"} {
+		app.WebserverType = webserverType
+		err = app.WriteConfig()
+		assert.NoError(err)
+
+		// Do a start on the configured site.
+		app, err = ddevapp.GetActiveApp("")
+		assert.NoError(err)
+		err = app.Start()
+		assert.NoError(err)
+
+		// Test for directory redirects under https and http
+		for _, parts := range expectations {
+
+			reqURL := parts.scheme + "://" + app.GetHostname() + parts.uri
+			// nolint: vetshadow
+			_, resp, err := testcommon.GetLocalHTTPResponse(t, reqURL)
+			assert.Error(err)
+			assert.NotNil(resp, "resp was nil for webserver_type=%s url=%s", webserverType, reqURL)
+			if resp != nil {
+				locHeader := resp.Header.Get("Location")
+
+				expectedRedirect := parts.expectedRedirectURI
+				// However, if we're hitting redir_abs.php (or apache hitting directory), the redirect will be the whole url.
+				if strings.Contains(parts.uri, "redir_abs.php") || webserverType != "nginx-fpm" {
+					expectedRedirect = parts.scheme + "://" + app.GetHostname() + parts.expectedRedirectURI
+				}
+				// Except the php relative redirect is always relative.
+				if strings.Contains(parts.uri, "redir_relative.php") {
+					expectedRedirect = parts.expectedRedirectURI
+				}
+
+				assert.EqualValues(locHeader, expectedRedirect, "For webserver_type %s url %s expected redirect %s != actual %s", webserverType, reqURL, expectedRedirect, locHeader)
+			}
+		}
+	}
+
+	err = app.Down(true, false)
+	assert.NoError(err)
+
+	// Change back to package dir. Lots of things will have to be cleaned up
+	// in defers, and for windows we have to not be sitting in them.
+	err = os.Chdir(packageDir)
+	assert.NoError(err)
+}
+
 // TestMultipleComposeFiles checks to see if a set of docker-compose files gets
 // properly loaded in the right order, with docker-compose.yaml first and
 // with docker-compose.override.yaml last.

pkg/ddevapp/testdata/TestHttpsRedirection/.htaccess

@@ -0,0 +1,9 @@
+# DirectorySlash off
+# DirectoryIndex index.php
+# DirectorySlash off
+# DirectoryIndexRedirect on
+# Redirect to HTTPS before Apache mod_dir DirectorySlash redirect to HTTP
+RewriteCond %{HTTP:X-Forwarded-Proto} =https
+RewriteCond %{LA-U:REQUEST_FILENAME} -d
+RewriteRule ^/(.*[^/])$ https://%{HTTP_HOST}/$1/ [R=301,L,QSA]
+

pkg/ddevapp/testdata/TestHttpsRedirection/index.php

@@ -0,0 +1,5 @@
+<?php
+
+echo "Hi there, this is /index.php<br/>";
+
+echo 'Here are links to <a href="/landed.php">landed.php</a> and <a href="subdir">subdir</a><br/>';

pkg/ddevapp/testdata/TestHttpsRedirection/landed.php

@@ -0,0 +1,14 @@
+<?php
+
+
+echo "You landed at ${_SERVER['REQUEST_URI']}<br/>";
+echo "HTTPS is {$_SERVER['HTTPS']}<br/>";
+
+echo 'You can go to <a href="redir_abs.php">redir_abs.php</a> or to <a href="redir_relative.php">redir_relative.php</a><br/>';
+
+
+if (!empty($_SERVER['HTTPS']) && $_SERVER["HTTPS"] == "on") {
+    echo "You can  <a href='http://${_SERVER['HTTP_HOST']}/landed.php'>switch from https to http</a><br/>";
+} else {
+    echo "You can <a href='https://${_SERVER['HTTP_HOST']}/landed.php'>switch from http to https</a><br/>";
+}

pkg/ddevapp/testdata/TestHttpsRedirection/redir_abs.php

@@ -0,0 +1,5 @@
+<?php
+
+$scheme = (!empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] == "on") ? "https://" : "http://";
+$url = $scheme . $_SERVER['HTTP_HOST'] . "/landed.php";
+header('Location: ' . $url, true, 302);              // Use either 301 or 302

pkg/ddevapp/testdata/TestHttpsRedirection/redir_relative.php

@@ -0,0 +1,3 @@
+<?php
+
+header('Location: ' . "/landed.php", true, 302);

pkg/ddevapp/testdata/TestHttpsRedirection/subdir/index.html

@@ -0,0 +1,2 @@
+This is subdir/index.html; When you get here you should still have the scheme where you came from (shouldn't switch between http and https).<br/>
+<a href="/">Go back to the top.</a>

pkg/testcommon/testcommon.go

@@ -2,6 +2,7 @@ package testcommon
 
 import (
 	"bytes"
+	"crypto/tls"
 	"io"
 	"io/ioutil"
 	"os"
@@ -343,7 +344,7 @@ func GetCachedArchive(siteName string, prefixString string, internalExtractionPa
 
 // GetLocalHTTPResponse takes a URL, hits the local docker for it, returns result
 // Returns error (with the body) if not 200 status code.
-func GetLocalHTTPResponse(t *testing.T, rawurl string) (string, error) {
+func GetLocalHTTPResponse(t *testing.T, rawurl string) (string, *http.Response, error) {
 	assert := asrt.New(t)
 
 	u, err := url.Parse(rawurl)
@@ -359,44 +360,51 @@ func GetLocalHTTPResponse(t *testing.T, rawurl string) (string, error) {
 	localAddress := u.String()
 
 	timeout := time.Duration(10 * time.Second)
+
+	// Ignore https cert failure, since we are in testing environment.
+	insecureTransport := &http.Transport{
+		TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
+	}
+
 	// Do not follow redirects, https://stackoverflow.com/a/38150816/215713
 	client := &http.Client{
 		CheckRedirect: func(req *http.Request, via []*http.Request) error {
 			return http.ErrUseLastResponse
 		},
-		Timeout: timeout,
+		Transport: insecureTransport,
+		Timeout:   timeout,
 	}
 
 	req, err := http.NewRequest("GET", localAddress, nil)
 
 	if err != nil {
-		return "", fmt.Errorf("Failed to NewRequest GET %s: %v", localAddress, err)
+		return "", nil, fmt.Errorf("Failed to NewRequest GET %s: %v", localAddress, err)
 	}
 	req.Host = fakeHost
 
 	resp, err := client.Do(req)
 	if err != nil {
-		return "", err
+		return "", resp, err
 	}
 
 	//nolint: errcheck
 	defer resp.Body.Close()
 	bodyBytes, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
-		return "", fmt.Errorf("unable to ReadAll resp.body: %v", err)
+		return "", resp, fmt.Errorf("unable to ReadAll resp.body: %v", err)
 	}
 	bodyString := string(bodyBytes)
 	if resp.StatusCode != 200 {
-		return bodyString, fmt.Errorf("http status code was %d, not 200", resp.StatusCode)
+		return bodyString, resp, fmt.Errorf("http status code was %d, not 200", resp.StatusCode)
 	}
-	return bodyString, nil
+	return bodyString, resp, nil
 }
 
 // EnsureLocalHTTPContent will verify a URL responds with a 200 and expected content string
 func EnsureLocalHTTPContent(t *testing.T, rawurl string, expectedContent string) {
 	assert := asrt.New(t)
 
-	body, err := GetLocalHTTPResponse(t, rawurl)
+	body, _, err := GetLocalHTTPResponse(t, rawurl)
 	assert.NoError(err, "GetLocalHTTPResponse returned err on rawurl %s: %v", rawurl, err)
 	assert.Contains(body, expectedContent)
 }

pkg/testcommon/testcommon_test.go

@@ -170,7 +170,7 @@ func TestGetLocalHTTPResponse(t *testing.T) {
 	assert.NoError(err)
 
 	safeURL := app.GetHTTPURL() + site.Safe200URL
-	out, err := GetLocalHTTPResponse(t, safeURL)
+	out, _, err := GetLocalHTTPResponse(t, safeURL)
 	assert.NoError(err)
 	assert.Contains(out, "Famous 5-minute install")
 

pkg/version/version.go

@@ -25,7 +25,7 @@ var DockerComposeFileFormatVersion = "3.6"
 var WebImg = "drud/ddev-webserver"
 
 // WebTag defines the default web image tag for drud dev
-var WebTag = "20180922_upgrade_debian_stretch" // Note that this can be overridden by make
+var WebTag = "20180922_apache_https" // Note that this can be overridden by make
 
 // DBImg defines the default db image used for applications.
 var DBImg = "drud/ddev-dbserver"