diff --git a/caddyhttp/httpserver/https.go b/caddyhttp/httpserver/https.go index 3d1de7499..ae3c4e902 100644 --- a/caddyhttp/httpserver/https.go +++ b/caddyhttp/httpserver/https.go @@ -159,29 +159,38 @@ func hostHasOtherPort(allConfigs []*SiteConfig, thisConfigIdx int, otherPort str // be the HTTPS configuration. The returned configuration is set // to listen on HTTPPort. The TLS field of cfg must not be nil. func redirPlaintextHost(cfg *SiteConfig) *SiteConfig { + redirPort := cfg.Addr.Port + if redirPort == HTTPSPort { + // By default, HTTPSPort should be DefaultHTTPSPort, + // which of course doesn't need to be explicitly stated + // in the Location header. Even if HTTPSPort is changed + // so that it is no longer DefaultHTTPSPort, we shouldn't + // append it to the URL in the Location because changing + // the HTTPS port is assumed to be an internal-only change + // (in other words, we assume port forwarding is going on); + // but redirects go back to a presumably-external client. + // (If redirect clients are also internal, that is more + // advanced, and the user should configure HTTP->HTTPS + // redirects themselves.) + redirPort = "" + } + redirMiddleware := func(next Handler) Handler { return HandlerFunc(func(w http.ResponseWriter, r *http.Request) (int, error) { // Construct the URL to which to redirect. Note that the Host in a - // request might contain a port, but we just need the hostname. + // request might contain a port, but we just need the hostname from + // it; and we'll set the port if needed. toURL := "https://" requestHost, _, err := net.SplitHostPort(r.Host) if err != nil { - requestHost = r.Host // host did not contain a port; okay + requestHost = r.Host // Host did not contain a port, so use the whole value + } + if redirPort == "" { + toURL += requestHost + } else { + toURL += net.JoinHostPort(requestHost, redirPort) } - // The rest of the URL will consist of the hostname and the URI. - // We do not append a port because if the HTTPSPort is changed - // from the default value, it is probably because there is port - // forwarding going on; and we do not need to specify the default - // HTTPS port in the redirect. Serving HTTPS on a port other than - // 443 is unusual, and is considered an advanced use case. If port - // forwarding IS happening, then redirecting the external client to - // this internal port will cause the connection to fail; and it - // definitely causes ACME HTTP-01 challenges to fail, because it - // only allows redirecting to port 80 or 443 (as of Feb 2018). - // If a user wants to redirect HTTP to HTTPS on an external port - // other than 443, they can easily configure that themselves. - toURL += requestHost toURL += r.URL.RequestURI() w.Header().Set("Connection", "close") diff --git a/caddyhttp/httpserver/https_test.go b/caddyhttp/httpserver/https_test.go index 3e9fe915a..043249445 100644 --- a/caddyhttp/httpserver/https_test.go +++ b/caddyhttp/httpserver/https_test.go @@ -53,7 +53,7 @@ func TestRedirPlaintextHost(t *testing.T) { }, { Host: "foohost", - Port: "443", // since this is the default HTTPS port, should not be included in Location value + Port: HTTPSPort, // since this is the 'default' HTTPS port, should not be included in Location value }, { Host: "*.example.com",