1
0
Fork 0

fix: stripPrefix middleware with empty resulting path.

This commit is contained in:
Ludovic Fernandez 2019-11-14 10:32:05 +01:00 committed by Traefiker Bot
parent cdb2446e32
commit 7afd2dbd20
14 changed files with 426 additions and 245 deletions

View file

@ -355,7 +355,13 @@ type Retry struct {
// StripPrefix holds the StripPrefix configuration.
type StripPrefix struct {
Prefixes []string `json:"prefixes,omitempty" toml:"prefixes,omitempty" yaml:"prefixes,omitempty"`
Prefixes []string `json:"prefixes,omitempty" toml:"prefixes,omitempty" yaml:"prefixes,omitempty"`
ForceSlash bool `json:"forceSlash,omitempty" toml:"forceSlash,omitempty" yaml:"forceSlash,omitempty"` // Deprecated
}
// SetDefaults Default values for a StripPrefix.
func (s *StripPrefix) SetDefaults() {
s.ForceSlash = true
}
// +k8s:deepcopy-gen=true

View file

@ -368,6 +368,7 @@ func TestDecodeConfiguration(t *testing.T) {
"foobar",
"fiibar",
},
ForceSlash: true,
},
},
"Middleware18": {
@ -771,6 +772,7 @@ func TestEncodeConfiguration(t *testing.T) {
"foobar",
"fiibar",
},
ForceSlash: true,
},
},
"Middleware18": {
@ -1091,6 +1093,7 @@ func TestEncodeConfiguration(t *testing.T) {
"traefik.HTTP.Middlewares.Middleware15.ReplacePathRegex.Replacement": "foobar",
"traefik.HTTP.Middlewares.Middleware16.Retry.Attempts": "42",
"traefik.HTTP.Middlewares.Middleware17.StripPrefix.Prefixes": "foobar, fiibar",
"traefik.HTTP.Middlewares.Middleware17.StripPrefix.ForceSlash": "true",
"traefik.HTTP.Middlewares.Middleware18.StripPrefixRegex.Regex": "foobar, fiibar",
"traefik.HTTP.Middlewares.Middleware19.Compress": "true",

View file

@ -41,23 +41,35 @@ func New(ctx context.Context, next http.Handler, config dynamic.AddPrefix, name
return result, nil
}
func (ap *addPrefix) GetTracingInformation() (string, ext.SpanKindEnum) {
return ap.name, tracing.SpanKindNoneEnum
func (a *addPrefix) GetTracingInformation() (string, ext.SpanKindEnum) {
return a.name, tracing.SpanKindNoneEnum
}
func (ap *addPrefix) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
logger := log.FromContext(middlewares.GetLoggerCtx(req.Context(), ap.name, typeName))
func (a *addPrefix) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
logger := log.FromContext(middlewares.GetLoggerCtx(req.Context(), a.name, typeName))
oldURLPath := req.URL.Path
req.URL.Path = ap.prefix + req.URL.Path
req.URL.Path = ensureLeadingSlash(a.prefix + req.URL.Path)
logger.Debugf("URL.Path is now %s (was %s).", req.URL.Path, oldURLPath)
if req.URL.RawPath != "" {
oldURLRawPath := req.URL.RawPath
req.URL.RawPath = ap.prefix + req.URL.RawPath
req.URL.RawPath = ensureLeadingSlash(a.prefix + req.URL.RawPath)
logger.Debugf("URL.RawPath is now %s (was %s).", req.URL.RawPath, oldURLRawPath)
}
req.RequestURI = req.URL.RequestURI()
ap.next.ServeHTTP(rw, req)
a.next.ServeHTTP(rw, req)
}
func ensureLeadingSlash(str string) string {
if str == "" {
return str
}
if str[0] == '/' {
return str
}
return "/" + str
}

View file

@ -7,7 +7,6 @@ import (
"github.com/containous/traefik/v2/pkg/config/dynamic"
"github.com/containous/traefik/v2/pkg/testhelpers"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@ -47,7 +46,6 @@ func TestNewAddPrefix(t *testing.T) {
}
func TestAddPrefix(t *testing.T) {
logrus.SetLevel(logrus.DebugLevel)
testCases := []struct {
desc string
prefix dynamic.AddPrefix
@ -61,6 +59,12 @@ func TestAddPrefix(t *testing.T) {
path: "/b",
expectedPath: "/a/b",
},
{
desc: "Works with missing leading slash",
prefix: dynamic.AddPrefix{Prefix: "a"},
path: "/",
expectedPath: "/a/",
},
{
desc: "Works with a raw path",
prefix: dynamic.AddPrefix{Prefix: "/a"},

View file

@ -20,18 +20,20 @@ const (
// stripPrefix is a middleware used to strip prefix from an URL request.
type stripPrefix struct {
next http.Handler
prefixes []string
name string
next http.Handler
prefixes []string
forceSlash bool // TODO Must be removed (breaking), the default behavior must be forceSlash=false
name string
}
// New creates a new strip prefix middleware.
func New(ctx context.Context, next http.Handler, config dynamic.StripPrefix, name string) (http.Handler, error) {
log.FromContext(middlewares.GetLoggerCtx(ctx, name, typeName)).Debug("Creating middleware")
return &stripPrefix{
prefixes: config.Prefixes,
next: next,
name: name,
prefixes: config.Prefixes,
forceSlash: config.ForceSlash,
next: next,
name: name,
}, nil
}
@ -42,9 +44,9 @@ func (s *stripPrefix) GetTracingInformation() (string, ext.SpanKindEnum) {
func (s *stripPrefix) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
for _, prefix := range s.prefixes {
if strings.HasPrefix(req.URL.Path, prefix) {
req.URL.Path = getPrefixStripped(req.URL.Path, prefix)
req.URL.Path = s.getPrefixStripped(req.URL.Path, prefix)
if req.URL.RawPath != "" {
req.URL.RawPath = getPrefixStripped(req.URL.RawPath, prefix)
req.URL.RawPath = s.getPrefixStripped(req.URL.RawPath, prefix)
}
s.serveRequest(rw, req, strings.TrimSpace(prefix))
return
@ -59,10 +61,25 @@ func (s *stripPrefix) serveRequest(rw http.ResponseWriter, req *http.Request, pr
s.next.ServeHTTP(rw, req)
}
func getPrefixStripped(s, prefix string) string {
return ensureLeadingSlash(strings.TrimPrefix(s, prefix))
func (s *stripPrefix) getPrefixStripped(urlPath, prefix string) string {
if s.forceSlash {
// Only for compatibility reason with the previous behavior,
// but the previous behavior is wrong.
// This needs to be removed in the next breaking version.
return "/" + strings.TrimPrefix(strings.TrimPrefix(urlPath, prefix), "/")
}
return ensureLeadingSlash(strings.TrimPrefix(urlPath, prefix))
}
func ensureLeadingSlash(str string) string {
return "/" + strings.TrimPrefix(str, "/")
if str == "" {
return str
}
if str[0] == '/' {
return str
}
return "/" + str
}

View file

@ -31,6 +31,17 @@ func TestStripPrefix(t *testing.T) {
expectedStatusCode: http.StatusOK,
expectedPath: "/noprefixes",
},
{
desc: "wildcard (.*) requests (ForceSlash)",
config: dynamic.StripPrefix{
Prefixes: []string{"/"},
ForceSlash: true,
},
path: "/",
expectedStatusCode: http.StatusOK,
expectedPath: "/",
expectedHeader: "/",
},
{
desc: "wildcard (.*) requests",
config: dynamic.StripPrefix{
@ -38,9 +49,20 @@ func TestStripPrefix(t *testing.T) {
},
path: "/",
expectedStatusCode: http.StatusOK,
expectedPath: "/",
expectedPath: "",
expectedHeader: "/",
},
{
desc: "prefix and path matching (ForceSlash)",
config: dynamic.StripPrefix{
Prefixes: []string{"/stat"},
ForceSlash: true,
},
path: "/stat",
expectedStatusCode: http.StatusOK,
expectedPath: "/",
expectedHeader: "/stat",
},
{
desc: "prefix and path matching",
config: dynamic.StripPrefix{
@ -48,9 +70,20 @@ func TestStripPrefix(t *testing.T) {
},
path: "/stat",
expectedStatusCode: http.StatusOK,
expectedPath: "/",
expectedPath: "",
expectedHeader: "/stat",
},
{
desc: "path prefix on exactly matching path (ForceSlash)",
config: dynamic.StripPrefix{
Prefixes: []string{"/stat/"},
ForceSlash: true,
},
path: "/stat/",
expectedStatusCode: http.StatusOK,
expectedPath: "/",
expectedHeader: "/stat/",
},
{
desc: "path prefix on exactly matching path",
config: dynamic.StripPrefix{
@ -58,7 +91,7 @@ func TestStripPrefix(t *testing.T) {
},
path: "/stat/",
expectedStatusCode: http.StatusOK,
expectedPath: "/",
expectedPath: "",
expectedHeader: "/stat/",
},
{
@ -101,6 +134,17 @@ func TestStripPrefix(t *testing.T) {
expectedPath: "/us",
expectedHeader: "/stat",
},
{
desc: "later prefix matching (ForceSlash)",
config: dynamic.StripPrefix{
Prefixes: []string{"/mismatch", "/stat"},
ForceSlash: true,
},
path: "/stat",
expectedStatusCode: http.StatusOK,
expectedPath: "/",
expectedHeader: "/stat",
},
{
desc: "later prefix matching",
config: dynamic.StripPrefix{
@ -108,7 +152,7 @@ func TestStripPrefix(t *testing.T) {
},
path: "/stat",
expectedStatusCode: http.StatusOK,
expectedPath: "/",
expectedPath: "",
expectedHeader: "/stat",
},
{
@ -162,12 +206,15 @@ func TestStripPrefix(t *testing.T) {
assert.Equal(t, test.expectedRawPath, actualRawPath, "Unexpected raw path.")
assert.Equal(t, test.expectedHeader, actualHeader, "Unexpected '%s' header.", ForwardedPrefixHeader)
expectedURI := test.expectedPath
expectedRequestURI := test.expectedPath
if test.expectedRawPath != "" {
// go HTTP uses the raw path when existent in the RequestURI
expectedURI = test.expectedRawPath
expectedRequestURI = test.expectedRawPath
}
assert.Equal(t, expectedURI, requestURI, "Unexpected request URI.")
if test.expectedPath == "" {
expectedRequestURI = "/"
}
assert.Equal(t, expectedRequestURI, requestURI, "Unexpected request URI.")
})
}
}

View file

@ -60,12 +60,12 @@ func (s *stripPrefixRegex) ServeHTTP(rw http.ResponseWriter, req *http.Request)
req.Header.Add(stripprefix.ForwardedPrefixHeader, prefix)
req.URL.Path = strings.Replace(req.URL.Path, prefix, "", 1)
req.URL.Path = ensureLeadingSlash(strings.Replace(req.URL.Path, prefix, "", 1))
if req.URL.RawPath != "" {
req.URL.RawPath = req.URL.RawPath[len(prefix):]
req.URL.RawPath = ensureLeadingSlash(req.URL.RawPath[len(prefix):])
}
req.RequestURI = ensureLeadingSlash(req.URL.RequestURI())
req.RequestURI = req.URL.RequestURI()
s.next.ServeHTTP(rw, req)
return
}
@ -75,5 +75,13 @@ func (s *stripPrefixRegex) ServeHTTP(rw http.ResponseWriter, req *http.Request)
}
func ensureLeadingSlash(str string) string {
return "/" + strings.TrimPrefix(str, "/")
if str == "" {
return str
}
if str[0] == '/' {
return str
}
return "/" + str
}

View file

@ -31,42 +31,66 @@ func TestStripPrefixRegex(t *testing.T) {
expectedPath: "/a/test",
},
{
path: "/a/test",
path: "/a/test/",
expectedStatusCode: http.StatusOK,
expectedPath: "/a/test",
expectedPath: "/a/test/",
},
{
path: "/a/api/",
expectedStatusCode: http.StatusOK,
expectedPath: "",
expectedHeader: "/a/api/",
},
{
path: "/a/api/test",
expectedStatusCode: http.StatusOK,
expectedPath: "test",
expectedPath: "/test",
expectedHeader: "/a/api/",
},
{
path: "/a/api/test/",
expectedStatusCode: http.StatusOK,
expectedPath: "/test/",
expectedHeader: "/a/api/",
},
{
path: "/b/api/",
expectedStatusCode: http.StatusOK,
expectedPath: "",
expectedHeader: "/b/api/",
},
{
path: "/b/api",
expectedStatusCode: http.StatusOK,
expectedPath: "/b/api",
},
{
path: "/b/api/test1",
expectedStatusCode: http.StatusOK,
expectedPath: "test1",
expectedPath: "/test1",
expectedHeader: "/b/api/",
},
{
path: "/b/api2/test2",
expectedStatusCode: http.StatusOK,
expectedPath: "test2",
expectedPath: "/test2",
expectedHeader: "/b/api2/",
},
{
path: "/c/api/123/",
expectedStatusCode: http.StatusOK,
expectedPath: "",
expectedHeader: "/c/api/123/",
},
{
path: "/c/api/123",
expectedStatusCode: http.StatusOK,
expectedPath: "/c/api/123",
},
{
path: "/c/api/123/test3",
expectedStatusCode: http.StatusOK,
expectedPath: "test3",
expectedPath: "/test3",
expectedHeader: "/c/api/123/",
},
{
@ -77,8 +101,8 @@ func TestStripPrefixRegex(t *testing.T) {
{
path: "/a/api/a%2Fb",
expectedStatusCode: http.StatusOK,
expectedPath: "a/b",
expectedRawPath: "a%2Fb",
expectedPath: "/a/b",
expectedRawPath: "/a%2Fb",
expectedHeader: "/a/api/",
},
}
@ -88,11 +112,12 @@ func TestStripPrefixRegex(t *testing.T) {
t.Run(test.path, func(t *testing.T) {
t.Parallel()
var actualPath, actualRawPath, actualHeader string
var actualPath, actualRawPath, actualHeader, requestURI string
handlerPath := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
actualPath = r.URL.Path
actualRawPath = r.URL.RawPath
actualHeader = r.Header.Get(stripprefix.ForwardedPrefixHeader)
requestURI = r.RequestURI
})
handler, err := New(context.Background(), handlerPath, testPrefixRegex, "foo-strip-prefix-regex")
require.NoError(t, err)
@ -106,6 +131,18 @@ func TestStripPrefixRegex(t *testing.T) {
assert.Equal(t, test.expectedPath, actualPath, "Unexpected path.")
assert.Equal(t, test.expectedRawPath, actualRawPath, "Unexpected raw path.")
assert.Equal(t, test.expectedHeader, actualHeader, "Unexpected '%s' header.", stripprefix.ForwardedPrefixHeader)
if test.expectedPath != test.path {
expectedRequestURI := test.expectedPath
if test.expectedRawPath != "" {
// go HTTP uses the raw path when existent in the RequestURI
expectedRequestURI = test.expectedRawPath
}
if test.expectedPath == "" {
expectedRequestURI = "/"
}
assert.Equal(t, expectedRequestURI, requestURI, "Unexpected request URI.")
}
})
}
}