Skip to content
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.

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

Extend https redirection tests, and fix incorrect behavior #3742

Merged
merged 4 commits into from
Aug 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 36 additions & 2 deletions integration/fixtures/https/https_redirect.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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/"

132 changes: 50 additions & 82 deletions integration/https_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package integration
import (
"bytes"
"crypto/tls"
"fmt"
"net"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -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"))

This comment was marked as outdated.

c.Assert(err, checker.IsNil)

client := &http.Client{
Expand All @@ -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)
}
}
}
18 changes: 6 additions & 12 deletions middlewares/redirect/redirect.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
}
}

Expand All @@ -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)
Expand Down
11 changes: 9 additions & 2 deletions middlewares/replace_path.go
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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()
Expand Down
2 changes: 2 additions & 0 deletions middlewares/replace_path_regex.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package middlewares

import (
"context"
"net/http"
"regexp"
"strings"
Expand Down Expand Up @@ -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()
Expand Down
12 changes: 4 additions & 8 deletions middlewares/stripPrefix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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)
Expand Down
8 changes: 3 additions & 5 deletions middlewares/stripPrefixRegex.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down