make redirect protection more generic

route/table.go

@@ -312,22 +312,22 @@ func (t Table) matchingHosts(req *http.Request) (hosts []string) {
 // and if none matches then it falls back to generic routes without
 // a host. This is useful for a catch-all '/' rule.
 func (t Table) Lookup(req *http.Request, trace string, pick picker, match matcher) (target *Target) {
-	path := req.URL.Path
 	if trace != "" {
 		if len(trace) > 16 {
 			trace = trace[:15]
 		}
-		log.Printf("[TRACE] %s Tracing %s%s", trace, req.Host, path)
+		log.Printf("[TRACE] %s Tracing %s%s", trace, req.Host, req.URL.Path)
 	}
 
 	// find matching hosts for the request
 	// and add "no host" as the fallback option
 	hosts := t.matchingHosts(req)
 	hosts = append(hosts, "")
 	for _, h := range hosts {
-		if target = t.lookup(h, path, trace, pick, match); target != nil {
-			if target.RedirectCode != 0 && target.URL.Scheme == "https" && req.Header.Get("X-Forwarded-Proto") == "https" {
-				log.Print("[TRACE] Skipping https redirect from https upstream")
+		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
 			}
 			break

route/table_test.go

@@ -487,8 +487,10 @@ func TestNormalizeHost(t *testing.T) {
 // for more information on the issue and purpose of this test
 func TestTableLookupIssue448(t *testing.T) {
 	s := `
-	route add http-redirect foo.com:80/ https://foo.com/ opts "redirect=301"
-	route add mock-service / http://local.svc/
+	route add mock-0 foo.com:80/ https://foo.com/ opts "redirect=301"
+	route add mock-2 aaa.com:80/ http://bbb.com/ opts "redirect=301"
+	route add mock-3 ccc.com:443/bar https://ccc.com/baz opts "redirect=301"
+	route add mock-4 / http://foo.com/
 	`
 
 	tbl, err := NewTable(s)
@@ -500,23 +502,41 @@ func TestTableLookupIssue448(t *testing.T) {
 		req *http.Request
 		dst string
 	}{
-		// upstream http request should get https redirect
 		{
-			&http.Request{
+			req: &http.Request{
+				Host: "foo.com",
+				URL:  mustParse("/"),
+			},
+			dst: "https://foo.com/",
+			// empty upstream header should follow redirect - standard behavior
+		},
+		{
+			req: &http.Request{
 				Host:   "foo.com",
 				URL:    mustParse("/"),
 				Header: http.Header{"X-Forwarded-Proto": {"http"}},
 			},
-			"https://foo.com/",
+			dst: "https://foo.com/",
+			// upstream http request to same https host and path should follow redirect
 		},
-		// upstream https request should skip https redirect
 		{
-			&http.Request{
-				Host:   "foo.com",
+			req: &http.Request{
+				Host:   "aaa.com",
 				URL:    mustParse("/"),
+				Header: http.Header{"X-Forwarded-Proto": {"http"}},
+			},
+			dst: "http://bbb.com/",
+			// upstream http request to different http host should follow redirect
+		},
+		{
+			req: &http.Request{
+				Host:   "ccc.com",
+				URL:    mustParse("/bar"),
 				Header: http.Header{"X-Forwarded-Proto": {"https"}},
+				TLS:    &tls.ConnectionState{},
 			},
-			"http://local.svc/",
+			dst: "https://ccc.com/baz",
+			// upstream https request to same https host with different path should follow redirect"
 		},
 	}