From 870755e90d0f358ed859dc2f0cca32d84b344e42 Mon Sep 17 00:00:00 2001 From: Daniel Tomcej Date: Tue, 14 Aug 2018 10:38:04 -0600 Subject: [PATCH] Extend https redirection tests, and fix incorrect behavior --- .../fixtures/https/https_redirect.toml | 38 ++++- integration/https_test.go | 132 +++++++----------- middlewares/redirect/redirect.go | 18 +-- middlewares/replace_path.go | 11 +- middlewares/replace_path_regex.go | 2 + middlewares/stripPrefix.go | 12 +- middlewares/stripPrefixRegex.go | 8 +- 7 files changed, 110 insertions(+), 111 deletions(-) diff --git a/integration/fixtures/https/https_redirect.toml b/integration/fixtures/https/https_redirect.toml index 498d14f89..fc0cdf67c 100644 --- a/integration/fixtures/https/https_redirect.toml +++ b/integration/fixtures/https/https_redirect.toml @@ -22,15 +22,49 @@ defaultEntryPoints = ["http", "https"] weight = 1 [frontends] + [frontends.frontend1] backend = "backend1" [frontends.frontend1.routes.test_1] rule = "Host: example.com; PathPrefixStrip: /api" - [frontends.frontend2] + [frontends.frontend2] backend = "backend1" [frontends.frontend2.routes.test_1] - rule = "Host: test.com; AddPrefix: /foo" + rule = "Host: example2.com; PathPrefixStrip: /api/" + [frontends.frontend3] backend = "backend1" [frontends.frontend3.routes.test_1] + rule = "Host: test.com; AddPrefix: /foo" + [frontends.frontend4] + backend = "backend1" + [frontends.frontend4.routes.test_1] + rule = "Host: test2.com; AddPrefix: /foo/" + + [frontends.frontend5] + backend = "backend1" + [frontends.frontend5.routes.test_1] rule = "Host: foo.com; PathPrefixStripRegex: /{id:[a-z]+}" + [frontends.frontend6] + backend = "backend1" + [frontends.frontend6.routes.test_1] + rule = "Host: foo2.com; PathPrefixStripRegex: /{id:[a-z]+}/" + + [frontends.frontend7] + backend = "backend1" + [frontends.frontend7.routes.test_1] + rule = "Host: bar.com; ReplacePathRegex: /api /" + [frontends.frontend8] + backend = "backend1" + [frontends.frontend8.routes.test_1] + rule = "Host: bar2.com; ReplacePathRegex: /api/ /" + + [frontends.frontend9] + backend = "backend1" + [frontends.frontend9.routes.test_1] + rule = "Host: pow.com; ReplacePath: /api" + [frontends.frontend10] + backend = "backend1" + [frontends.frontend10.routes.test_1] + rule = "Host: pow2.com; ReplacePath: /api/" + diff --git a/integration/https_test.go b/integration/https_test.go index 5db8a54ab..cf6dee11e 100644 --- a/integration/https_test.go +++ b/integration/https_test.go @@ -3,6 +3,7 @@ package integration import ( "bytes" "crypto/tls" + "fmt" "net" "net/http" "net/http/httptest" @@ -740,7 +741,7 @@ func (s *HTTPSSuite) TestEntrypointHttpsRedirectAndPathModification(c *check.C) defer cmd.Process.Kill() // wait for Traefik - err = try.GetRequest("http://127.0.0.1:8080/api/providers", 500*time.Millisecond, try.BodyContains("Host: example.com")) + err = try.GetRequest("http://127.0.0.1:8080/api/providers", 1000*time.Millisecond, try.BodyContains("Host: example.com")) c.Assert(err, checker.IsNil) client := &http.Client{ @@ -750,115 +751,82 @@ func (s *HTTPSSuite) TestEntrypointHttpsRedirectAndPathModification(c *check.C) } testCases := []struct { - desc string - host string - sourceURL string - expectedURL string + desc string + hosts []string + path string }{ { - desc: "Stripped URL redirect", - host: "example.com", - sourceURL: "http://127.0.0.1:8888/api", - expectedURL: "https://example.com:8443/api", + desc: "Stripped URL redirect", + hosts: []string{"example.com", "foo.com", "bar.com"}, + path: "/api", }, { - desc: "Stripped URL with trailing slash redirect", - host: "example.com", - sourceURL: "http://127.0.0.1:8888/api/", - expectedURL: "https://example.com:8443/api/", + desc: "Stripped URL with trailing slash redirect", + hosts: []string{"example.com", "example2.com", "foo.com", "foo2.com", "bar.com", "bar2.com"}, + path: "/api/", }, { - desc: "Stripped URL with double trailing slash redirect", - host: "example.com", - sourceURL: "http://127.0.0.1:8888/api//", - expectedURL: "https://example.com:8443/api//", + desc: "Stripped URL with double trailing slash redirect", + hosts: []string{"example.com", "example2.com", "foo.com", "foo2.com", "bar.com", "bar2.com"}, + path: "/api//", }, { - desc: "Stripped URL with path redirect", - host: "example.com", - sourceURL: "http://127.0.0.1:8888/api/bacon", - expectedURL: "https://example.com:8443/api/bacon", + desc: "Stripped URL with path redirect", + hosts: []string{"example.com", "example2.com", "foo.com", "foo2.com", "bar.com", "bar2.com"}, + path: "/api/bacon", }, { - desc: "Stripped URL with path and trailing slash redirect", - host: "example.com", - sourceURL: "http://127.0.0.1:8888/api/bacon/", - expectedURL: "https://example.com:8443/api/bacon/", + desc: "Stripped URL with path and trailing slash redirect", + hosts: []string{"example.com", "example2.com", "foo.com", "foo2.com", "bar.com", "bar2.com"}, + path: "/api/bacon/", }, { - desc: "Stripped URL with path and double trailing slash redirect", - host: "example.com", - sourceURL: "http://127.0.0.1:8888/api/bacon//", - expectedURL: "https://example.com:8443/api/bacon//", + desc: "Stripped URL with path and double trailing slash redirect", + hosts: []string{"example.com", "example2.com", "foo.com", "foo2.com", "bar.com", "bar2.com"}, + path: "/api/bacon//", }, { - desc: "Root Path with redirect", - host: "test.com", - sourceURL: "http://127.0.0.1:8888/", - expectedURL: "https://test.com:8443/", + desc: "Root Path with redirect", + hosts: []string{"test.com", "test2.com", "pow.com", "pow2.com"}, + path: "/", }, { - desc: "Root Path with double trailing slash redirect", - host: "test.com", - sourceURL: "http://127.0.0.1:8888//", - expectedURL: "https://test.com:8443//", + desc: "Root Path with double trailing slash redirect", + hosts: []string{"test.com", "test2.com", "pow.com", "pow2.com"}, + path: "//", }, { - desc: "AddPrefix with redirect", - host: "test.com", - sourceURL: "http://127.0.0.1:8888/wtf", - expectedURL: "https://test.com:8443/wtf", + desc: "Path modify with redirect", + hosts: []string{"test.com", "test2.com", "pow.com", "pow2.com"}, + path: "/wtf", }, { - desc: "AddPrefix with trailing slash redirect", - host: "test.com", - sourceURL: "http://127.0.0.1:8888/wtf/", - expectedURL: "https://test.com:8443/wtf/", + desc: "Path modify with trailing slash redirect", + hosts: []string{"test.com", "test2.com", "pow.com", "pow2.com"}, + path: "/wtf/", }, { - desc: "AddPrefix with matching path segment redirect", - host: "test.com", - sourceURL: "http://127.0.0.1:8888/wtf/foo", - expectedURL: "https://test.com:8443/wtf/foo", - }, - { - desc: "Stripped URL Regex redirect", - host: "foo.com", - sourceURL: "http://127.0.0.1:8888/api", - expectedURL: "https://foo.com:8443/api", - }, - { - desc: "Stripped URL Regex with trailing slash redirect", - host: "foo.com", - sourceURL: "http://127.0.0.1:8888/api/", - expectedURL: "https://foo.com:8443/api/", - }, - { - desc: "Stripped URL Regex with path redirect", - host: "foo.com", - sourceURL: "http://127.0.0.1:8888/api/bacon", - expectedURL: "https://foo.com:8443/api/bacon", - }, - { - desc: "Stripped URL Regex with path and trailing slash redirect", - host: "foo.com", - sourceURL: "http://127.0.0.1:8888/api/bacon/", - expectedURL: "https://foo.com:8443/api/bacon/", + desc: "Path modify with matching path segment redirect", + hosts: []string{"test.com", "test2.com", "pow.com", "pow2.com"}, + path: "/wtf/foo", }, } for _, test := range testCases { - test := test + sourceURL := fmt.Sprintf("http://127.0.0.1:8888%s", test.path) + for _, host := range test.hosts { + req, err := http.NewRequest("GET", sourceURL, nil) + c.Assert(err, checker.IsNil) + req.Host = host - req, err := http.NewRequest("GET", test.sourceURL, nil) - c.Assert(err, checker.IsNil) - req.Host = test.host + resp, err := client.Do(req) + c.Assert(err, checker.IsNil) + defer resp.Body.Close() - resp, err := client.Do(req) - c.Assert(err, checker.IsNil) - defer resp.Body.Close() + location := resp.Header.Get("Location") + expected := fmt.Sprintf("https://%s:8443%s", host, test.path) - location := resp.Header.Get("Location") - c.Assert(location, checker.Equals, test.expectedURL) + c.Assert(location, checker.Equals, expected) + } } } diff --git a/middlewares/redirect/redirect.go b/middlewares/redirect/redirect.go index 1b1dfe7dd..506d7db3f 100644 --- a/middlewares/redirect/redirect.go +++ b/middlewares/redirect/redirect.go @@ -88,19 +88,7 @@ func (h *handler) ServeHTTP(rw http.ResponseWriter, req *http.Request, next http if stripPrefix, stripPrefixOk := req.Context().Value(middlewares.StripPrefixKey).(string); stripPrefixOk { if len(stripPrefix) > 0 { - tempPath := parsedURL.Path parsedURL.Path = stripPrefix - if len(tempPath) > 0 && tempPath != "/" { - parsedURL.Path = stripPrefix + tempPath - } - - if trailingSlash, trailingSlashOk := req.Context().Value(middlewares.StripPrefixSlashKey).(bool); trailingSlashOk { - if trailingSlash { - if !strings.HasSuffix(parsedURL.Path, "/") { - parsedURL.Path = fmt.Sprintf("%s/", parsedURL.Path) - } - } - } } } @@ -110,6 +98,12 @@ func (h *handler) ServeHTTP(rw http.ResponseWriter, req *http.Request, next http } } + if replacePath, replacePathOk := req.Context().Value(middlewares.ReplacePathKey).(string); replacePathOk { + if len(replacePath) > 0 { + parsedURL.Path = replacePath + } + } + if newURL != oldURL { handler := &moveHandler{location: parsedURL, permanent: h.permanent} handler.ServeHTTP(rw, req) diff --git a/middlewares/replace_path.go b/middlewares/replace_path.go index 40211c771..d5bd03eac 100644 --- a/middlewares/replace_path.go +++ b/middlewares/replace_path.go @@ -1,11 +1,17 @@ package middlewares import ( + "context" "net/http" ) -// ReplacedPathHeader is the default header to set the old path to -const ReplacedPathHeader = "X-Replaced-Path" +const ( + // ReplacePathKey is the key within the request context used to + // store the replaced path + ReplacePathKey key = "ReplacePath" + // ReplacedPathHeader is the default header to set the old path to + ReplacedPathHeader = "X-Replaced-Path" +) // ReplacePath is a middleware used to replace the path of a URL request type ReplacePath struct { @@ -14,6 +20,7 @@ type ReplacePath struct { } func (s *ReplacePath) ServeHTTP(w http.ResponseWriter, r *http.Request) { + r = r.WithContext(context.WithValue(r.Context(), ReplacePathKey, r.URL.Path)) r.Header.Add(ReplacedPathHeader, r.URL.Path) r.URL.Path = s.Path r.RequestURI = r.URL.RequestURI() diff --git a/middlewares/replace_path_regex.go b/middlewares/replace_path_regex.go index 4d97c0de5..d753e86c0 100644 --- a/middlewares/replace_path_regex.go +++ b/middlewares/replace_path_regex.go @@ -1,6 +1,7 @@ package middlewares import ( + "context" "net/http" "regexp" "strings" @@ -30,6 +31,7 @@ func NewReplacePathRegexHandler(regex string, replacement string, handler http.H func (s *ReplacePathRegex) ServeHTTP(w http.ResponseWriter, r *http.Request) { if s.Regexp != nil && len(s.Replacement) > 0 && s.Regexp.MatchString(r.URL.Path) { + r = r.WithContext(context.WithValue(r.Context(), ReplacePathKey, r.URL.Path)) r.Header.Add(ReplacedPathHeader, r.URL.Path) r.URL.Path = s.Regexp.ReplaceAllString(r.URL.Path, s.Replacement) r.RequestURI = r.URL.RequestURI() diff --git a/middlewares/stripPrefix.go b/middlewares/stripPrefix.go index f5295d94c..222eb33cd 100644 --- a/middlewares/stripPrefix.go +++ b/middlewares/stripPrefix.go @@ -10,9 +10,6 @@ const ( // StripPrefixKey is the key within the request context used to // store the stripped prefix StripPrefixKey key = "StripPrefix" - // StripPrefixSlashKey is the key within the request context used to - // store the stripped slash - StripPrefixSlashKey key = "StripPrefixSlash" // ForwardedPrefixHeader is the default header to set prefix ForwardedPrefixHeader = "X-Forwarded-Prefix" ) @@ -26,21 +23,20 @@ type StripPrefix struct { func (s *StripPrefix) ServeHTTP(w http.ResponseWriter, r *http.Request) { for _, prefix := range s.Prefixes { if strings.HasPrefix(r.URL.Path, prefix) { - trailingSlash := r.URL.Path == prefix+"/" + rawReqPath := r.URL.Path r.URL.Path = stripPrefix(r.URL.Path, prefix) if r.URL.RawPath != "" { r.URL.RawPath = stripPrefix(r.URL.RawPath, prefix) } - s.serveRequest(w, r, strings.TrimSpace(prefix), trailingSlash) + s.serveRequest(w, r, strings.TrimSpace(prefix), rawReqPath) return } } http.NotFound(w, r) } -func (s *StripPrefix) serveRequest(w http.ResponseWriter, r *http.Request, prefix string, trailingSlash bool) { - r = r.WithContext(context.WithValue(r.Context(), StripPrefixSlashKey, trailingSlash)) - r = r.WithContext(context.WithValue(r.Context(), StripPrefixKey, prefix)) +func (s *StripPrefix) serveRequest(w http.ResponseWriter, r *http.Request, prefix string, rawReqPath string) { + r = r.WithContext(context.WithValue(r.Context(), StripPrefixKey, rawReqPath)) r.Header.Add(ForwardedPrefixHeader, prefix) r.RequestURI = r.URL.RequestURI() s.Handler.ServeHTTP(w, r) diff --git a/middlewares/stripPrefixRegex.go b/middlewares/stripPrefixRegex.go index d86733dd6..c249f0fb2 100644 --- a/middlewares/stripPrefixRegex.go +++ b/middlewares/stripPrefixRegex.go @@ -39,16 +39,14 @@ func (s *StripPrefixRegex) ServeHTTP(w http.ResponseWriter, r *http.Request) { log.Error("Error in stripPrefix middleware", err) return } - - trailingSlash := r.URL.Path == prefix.Path+"/" + rawReqPath := r.URL.Path r.URL.Path = r.URL.Path[len(prefix.Path):] if r.URL.RawPath != "" { r.URL.RawPath = r.URL.RawPath[len(prefix.Path):] } - r = r.WithContext(context.WithValue(r.Context(), StripPrefixSlashKey, trailingSlash)) - r = r.WithContext(context.WithValue(r.Context(), StripPrefixKey, prefix.Path)) + r = r.WithContext(context.WithValue(r.Context(), StripPrefixKey, rawReqPath)) r.Header.Add(ForwardedPrefixHeader, prefix.Path) - r.RequestURI = r.URL.RequestURI() + r.RequestURI = ensureLeadingSlash(r.URL.RequestURI()) s.Handler.ServeHTTP(w, r) return }