volatiletech / abcweb

Go web app framework and generator. Inspired by Rails.
BSD 3-Clause "New" or "Revised" License
265 stars 15 forks source link

http to https redirect fails when not on test port #20

Closed nullbio closed 7 years ago

nullbio commented 7 years ago

Review patch:

+++ b/vendor/github.com/volatiletech/abcweb/abcserver/server.go
@@ -1,6 +1,7 @@
 package abcserver

 import (
+       "bytes"
        "context"
        "crypto/tls"
        "fmt"
@@ -9,6 +10,7 @@ import (
        "net/http"
        "os"
        "os/signal"
+       "strings"

        "github.com/go-chi/chi"
        "github.com/pkg/errors"
@@ -108,13 +110,23 @@ func Redirect(cfg abcconfig.ServerConfig, logger *zap.Logger) {
                ErrorLog:     log.New(serverErrLogger{logger}, "", 0),
                Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
                        // Remove port if it exists so we can replace it with https port
-                       var httpHost string
-                       httpHost, _, err = net.SplitHostPort(r.Host)
-                       if err != nil {
-                               log.Fatal("failed to get http host from request", zap.Error(err))
+                       httpHost := r.Host
+                       if strings.ContainsRune(r.Host, ':') {
+                               httpHost, _, err = net.SplitHostPort(r.Host)
+                               if err != nil {
+                                       log.Fatal("failed to get http host from request", zap.Error(err))
+                               }
                        }
-
-                       url := fmt.Sprintf("https://%s:%s%s", httpHost, httpsPort, r.RequestURI)
+                       b := bytes.Buffer{}
+                       b.WriteString("https://")
+                       b.WriteString(httpHost)
+                       if httpsPort != "443" {
+                               b.WriteByte(':')
+                               b.WriteString(httpsPort)
+                       }
+                       b.WriteString(r.RequestURI)
+                       url := b.String()
+                       logger.Info("redirect", zap.String("origin", r.Host), zap.String("origin-path", r.URL.String()), zap.String("url", url))
                        http.Redirect(w, r, url, http.StatusMovedPermanently)
                }),
        }
nullbio commented 7 years ago

Fixed in v3.0.2