Skip to content

Avoid redirect loop in HTTPSRedirect middleware #1058

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 2, 2018
Merged

Avoid redirect loop in HTTPSRedirect middleware #1058

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions middleware/redirect.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ type RedirectConfig struct {
Code int `yaml:"code"`
}

// redirectLogic represents a function that given a tls flag, host and uri
// redirectLogic represents a function that given a scheme, host and uri
// can both: 1) determine if redirect is needed (will set ok accordingly) and
// 2) return the appropriate redirect url.
type redirectLogic func(tls bool, scheme, host, uri string) (ok bool, url string)
type redirectLogic func(scheme, host, uri string) (ok bool, url string)

const www = "www"

Expand All @@ -40,8 +40,8 @@ func HTTPSRedirect() echo.MiddlewareFunc {
// HTTPSRedirectWithConfig returns an HTTPSRedirect middleware with config.
// See `HTTPSRedirect()`.
func HTTPSRedirectWithConfig(config RedirectConfig) echo.MiddlewareFunc {
return redirect(config, func(tls bool, _, host, uri string) (ok bool, url string) {
if ok = !tls; ok {
return redirect(config, func(scheme, host, uri string) (ok bool, url string) {
if ok = scheme != "https"; ok {
url = "https://" + host + uri
}
return
Expand All @@ -59,8 +59,8 @@ func HTTPSWWWRedirect() echo.MiddlewareFunc {
// HTTPSWWWRedirectWithConfig returns an HTTPSRedirect middleware with config.
// See `HTTPSWWWRedirect()`.
func HTTPSWWWRedirectWithConfig(config RedirectConfig) echo.MiddlewareFunc {
return redirect(config, func(tls bool, _, host, uri string) (ok bool, url string) {
if ok = !tls && host[:3] != www; ok {
return redirect(config, func(scheme, host, uri string) (ok bool, url string) {
if ok = scheme != "https" && host[:3] != www; ok {
url = "https://www." + host + uri
}
return
Expand All @@ -78,8 +78,8 @@ func HTTPSNonWWWRedirect() echo.MiddlewareFunc {
// HTTPSNonWWWRedirectWithConfig returns an HTTPSRedirect middleware with config.
// See `HTTPSNonWWWRedirect()`.
func HTTPSNonWWWRedirectWithConfig(config RedirectConfig) echo.MiddlewareFunc {
return redirect(config, func(tls bool, _, host, uri string) (ok bool, url string) {
if ok = !tls; ok {
return redirect(config, func(scheme, host, uri string) (ok bool, url string) {
if ok = scheme != "https"; ok {
if host[:3] == www {
host = host[4:]
}
Expand All @@ -100,7 +100,7 @@ func WWWRedirect() echo.MiddlewareFunc {
// WWWRedirectWithConfig returns an HTTPSRedirect middleware with config.
// See `WWWRedirect()`.
func WWWRedirectWithConfig(config RedirectConfig) echo.MiddlewareFunc {
return redirect(config, func(_ bool, scheme, host, uri string) (ok bool, url string) {
return redirect(config, func(scheme, host, uri string) (ok bool, url string) {
if ok = host[:3] != www; ok {
url = scheme + "://www." + host + uri
}
Expand All @@ -119,7 +119,7 @@ func NonWWWRedirect() echo.MiddlewareFunc {
// NonWWWRedirectWithConfig returns an HTTPSRedirect middleware with config.
// See `NonWWWRedirect()`.
func NonWWWRedirectWithConfig(config RedirectConfig) echo.MiddlewareFunc {
return redirect(config, func(tls bool, scheme, host, uri string) (ok bool, url string) {
return redirect(config, func(scheme, host, uri string) (ok bool, url string) {
if ok = host[:3] == www; ok {
url = scheme + "://" + host[4:] + uri
}
Expand All @@ -143,7 +143,7 @@ func redirect(config RedirectConfig, cb redirectLogic) echo.MiddlewareFunc {

req, scheme := c.Request(), c.Scheme()
host := req.Host
if ok, url := cb(c.IsTLS(), scheme, host, req.RequestURI); ok {
if ok, url := cb(scheme, host, req.RequestURI); ok {
return c.Redirect(config.Code, url)
}

Expand Down
39 changes: 33 additions & 6 deletions middleware/redirect_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,47 +12,74 @@ import (
type middlewareGenerator func() echo.MiddlewareFunc

func TestRedirectHTTPSRedirect(t *testing.T) {
res := redirectTest(HTTPSRedirect, "labstack.com")
res := redirectTest(HTTPSRedirect, "labstack.com", nil)

assert.Equal(t, http.StatusMovedPermanently, res.Code)
assert.Equal(t, "https://labstack.com/", res.Header().Get(echo.HeaderLocation))
}

func TestHTTPSRedirectBehindTLSTerminationProxy(t *testing.T) {
header := http.Header{}
header.Set(echo.HeaderXForwardedProto, "https")
res := redirectTest(HTTPSRedirect, "labstack.com", header)

assert.Equal(t, http.StatusOK, res.Code)
}

func TestRedirectHTTPSWWWRedirect(t *testing.T) {
res := redirectTest(HTTPSWWWRedirect, "labstack.com")
res := redirectTest(HTTPSWWWRedirect, "labstack.com", nil)

assert.Equal(t, http.StatusMovedPermanently, res.Code)
assert.Equal(t, "https://www.labstack.com/", res.Header().Get(echo.HeaderLocation))
}

func TestRedirectHTTPSWWWRedirectBehindTLSTerminationProxy(t *testing.T) {
header := http.Header{}
header.Set(echo.HeaderXForwardedProto, "https")
res := redirectTest(HTTPSWWWRedirect, "labstack.com", header)

assert.Equal(t, http.StatusOK, res.Code)
}

func TestRedirectHTTPSNonWWWRedirect(t *testing.T) {
res := redirectTest(HTTPSNonWWWRedirect, "www.labstack.com")
res := redirectTest(HTTPSNonWWWRedirect, "www.labstack.com", nil)

assert.Equal(t, http.StatusMovedPermanently, res.Code)
assert.Equal(t, "https://labstack.com/", res.Header().Get(echo.HeaderLocation))
}

func TestRedirectHTTPSNonWWWRedirectBehindTLSTerminationProxy(t *testing.T) {
header := http.Header{}
header.Set(echo.HeaderXForwardedProto, "https")
res := redirectTest(HTTPSNonWWWRedirect, "www.labstack.com", header)

assert.Equal(t, http.StatusOK, res.Code)
}

func TestRedirectWWWRedirect(t *testing.T) {
res := redirectTest(WWWRedirect, "labstack.com")
res := redirectTest(WWWRedirect, "labstack.com", nil)

assert.Equal(t, http.StatusMovedPermanently, res.Code)
assert.Equal(t, "http://www.labstack.com/", res.Header().Get(echo.HeaderLocation))
}

func TestRedirectNonWWWRedirect(t *testing.T) {
res := redirectTest(NonWWWRedirect, "www.labstack.com")
res := redirectTest(NonWWWRedirect, "www.labstack.com", nil)

assert.Equal(t, http.StatusMovedPermanently, res.Code)
assert.Equal(t, "http://labstack.com/", res.Header().Get(echo.HeaderLocation))
}

func redirectTest(fn middlewareGenerator, host string) *httptest.ResponseRecorder {
func redirectTest(fn middlewareGenerator, host string, header http.Header) *httptest.ResponseRecorder {
e := echo.New()
next := func(c echo.Context) (err error) {
return c.NoContent(http.StatusOK)
}
req := httptest.NewRequest(echo.GET, "/", nil)
req.Host = host
if header != nil {
req.Header = header
}
res := httptest.NewRecorder()
c := e.NewContext(req, res)

Expand Down