diff --git a/route/table.go b/route/table.go index ab87840d4..5429c2406 100644 --- a/route/table.go +++ b/route/table.go @@ -312,12 +312,11 @@ 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 @@ -325,9 +324,10 @@ func (t Table) Lookup(req *http.Request, trace string, pick picker, match matche 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 diff --git a/route/table_test.go b/route/table_test.go index 175737d77..deea8c249 100644 --- a/route/table_test.go +++ b/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" }, }