From 71d29728adc8300866fe68fd7da6447f30ffe1f6 Mon Sep 17 00:00:00 2001
From: Aaron Hurt <ahurt@anbcs.com>
Date: Thu, 15 Mar 2018 19:26:22 -0500
Subject: [PATCH] Further improvements to redirect loop detection.

We now generate the redirectURL in the lookup function
and use it for loop detection.  We also cache the redirectURL
in the target for later use to prevent re-generation.
---
 proxy/http_proxy.go  |  3 +--
 route/table.go       | 12 ++++++++----
 route/target.go      | 31 +++++++++++++++++--------------
 route/target_test.go |  6 +++---
 4 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/proxy/http_proxy.go b/proxy/http_proxy.go
index 0898d7afe..95758bbe1 100644
--- a/proxy/http_proxy.go
+++ b/proxy/http_proxy.go
@@ -99,8 +99,7 @@ func (p *HTTPProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if t.RedirectCode != 0 {
-		redirectURL := t.GetRedirectURL(requestURL)
-		http.Redirect(w, r, redirectURL.String(), t.RedirectCode)
+		http.Redirect(w, r, t.RedirectURL.String(), t.RedirectCode)
 		if t.Timer != nil {
 			t.Timer.Update(0)
 		}
diff --git a/route/table.go b/route/table.go
index 5429c2406..c417e2c34 100644
--- a/route/table.go
+++ b/route/table.go
@@ -325,10 +325,14 @@ func (t Table) Lookup(req *http.Request, trace string, pick picker, match matche
 	hosts = append(hosts, "")
 	for _, h := range hosts {
 		if target = t.lookup(h, req.URL.Path, trace, pick, match); target != nil {
-			if target.RedirectCode != 0 && target.URL.Scheme == req.Header.Get("X-Forwarded-Proto") &&
-				target.URL.Hostname() == req.URL.Hostname() && target.URL.Path == req.URL.Path {
-				log.Print("[TRACE] Skipping redirect with same upstream scheme, host and path as target")
-				continue
+			if target.RedirectCode != 0 {
+				target.BuildRedirectURL(req.URL) // build redirect url and cache in target
+				if target.RedirectURL.Scheme == req.Header.Get("X-Forwarded-Proto") &&
+					target.RedirectURL.Host == req.Host &&
+					target.RedirectURL.Path == req.URL.Path {
+					log.Print("[INFO] Skipping redirect with same scheme, host and path")
+					continue
+				}
 			}
 			break
 		}
diff --git a/route/target.go b/route/target.go
index b5b63452a..e91fdc0a9 100644
--- a/route/target.go
+++ b/route/target.go
@@ -38,6 +38,10 @@ type Target struct {
 	// When set to a value > 0 the client is redirected to the target url.
 	RedirectCode int
 
+	// RedirectURL is the redirect target based on the request.
+	// This is cached here to prevent multiple generations per request.
+	RedirectURL *url.URL
+
 	// FixedWeight is the weight assigned to this target.
 	// If the value is 0 the targets weight is dynamic.
 	FixedWeight float64
@@ -55,28 +59,27 @@ type Target struct {
 	accessRules map[string][]interface{}
 }
 
-func (t *Target) GetRedirectURL(requestURL *url.URL) *url.URL {
-	redirectURL := &url.URL{
+func (t *Target) BuildRedirectURL(requestURL *url.URL) {
+	t.RedirectURL = &url.URL{
 		Scheme:   t.URL.Scheme,
 		Host:     t.URL.Host,
 		Path:     t.URL.Path,
 		RawQuery: t.URL.RawQuery,
 	}
-	if strings.HasSuffix(redirectURL.Host, "$path") {
-		redirectURL.Host = redirectURL.Host[:len(redirectURL.Host)-len("$path")]
-		redirectURL.Path = "$path"
+	if strings.HasSuffix(t.RedirectURL.Host, "$path") {
+		t.RedirectURL.Host = t.RedirectURL.Host[:len(t.RedirectURL.Host)-len("$path")]
+		t.RedirectURL.Path = "$path"
 	}
-	if strings.Contains(redirectURL.Path, "/$path") {
-		redirectURL.Path = strings.Replace(redirectURL.Path, "/$path", "$path", 1)
+	if strings.Contains(t.RedirectURL.Path, "/$path") {
+		t.RedirectURL.Path = strings.Replace(t.RedirectURL.Path, "/$path", "$path", 1)
 	}
-	if strings.Contains(redirectURL.Path, "$path") {
-		redirectURL.Path = strings.Replace(redirectURL.Path, "$path", requestURL.Path, 1)
-		if t.StripPath != "" && strings.HasPrefix(redirectURL.Path, t.StripPath) {
-			redirectURL.Path = redirectURL.Path[len(t.StripPath):]
+	if strings.Contains(t.RedirectURL.Path, "$path") {
+		t.RedirectURL.Path = strings.Replace(t.RedirectURL.Path, "$path", requestURL.Path, 1)
+		if t.StripPath != "" && strings.HasPrefix(t.RedirectURL.Path, t.StripPath) {
+			t.RedirectURL.Path = t.RedirectURL.Path[len(t.StripPath):]
 		}
-		if redirectURL.RawQuery == "" && requestURL.RawQuery != "" {
-			redirectURL.RawQuery = requestURL.RawQuery
+		if t.RedirectURL.RawQuery == "" && requestURL.RawQuery != "" {
+			t.RedirectURL.RawQuery = requestURL.RawQuery
 		}
 	}
-	return redirectURL
 }
diff --git a/route/target_test.go b/route/target_test.go
index 7d9c7794b..21ad2bc42 100644
--- a/route/target_test.go
+++ b/route/target_test.go
@@ -5,7 +5,7 @@ import (
 	"testing"
 )
 
-func TestTarget_GetRedirectURL(t *testing.T) {
+func TestTarget_BuildRedirectURL(t *testing.T) {
 	type routeTest struct {
 		req  string
 		want string
@@ -95,8 +95,8 @@ func TestTarget_GetRedirectURL(t *testing.T) {
 		target := route.Targets[0]
 		for _, rt := range tt.tests {
 			reqURL, _ := url.Parse("http://foo.com" + rt.req)
-			got := target.GetRedirectURL(reqURL)
-			if got.String() != rt.want {
+			target.BuildRedirectURL(reqURL)
+			if got := target.RedirectURL.String(); got != rt.want {
 				t.Errorf("Got %s, wanted %s", got, rt.want)
 			}
 		}