Merge pull request #1577 from aantono/Issue1569

Fixed ReplacePath rule executing out of order, when combined with PathPrefixStrip
This commit is contained in:
Emile Vauge 2017-05-15 23:21:53 +02:00 committed by GitHub
commit 30aa5a82b3
4 changed files with 127 additions and 9 deletions

View file

@ -191,6 +191,25 @@ backend = "backend2"
rule = "Path:/test1,/test2" rule = "Path:/test1,/test2"
``` ```
### Rules Order
When combining `Modifier` rules with `Matcher` rules, it is important to remember that `Modifier` rules **ALWAYS** apply after the `Matcher` rules.
The following rules are both `Matchers` and `Modifiers`, so the `Matcher` portion of the rule will apply first, and the `Modifier` will apply later.
- `PathStrip`
- `PathStripRegex`
- `PathPrefixStrip`
- `PathPrefixStripRegex`
`Modifiers` will be applied in a pre-determined order regardless of their order in the `rule` configuration section.
1. `PathStrip`
2. `PathPrefixStrip`
3. `PathStripRegex`
4. `PathPrefixStripRegex`
5. `AddPrefix`
6. `ReplacePath`
### Priorities ### Priorities
By default, routes will be sorted (in descending order) using rules length (to avoid path overlap): By default, routes will be sorted (in descending order) using rules length (to avoid path overlap):

View file

@ -776,7 +776,17 @@ func (server *Server) loadConfig(configurations configs, globalConfiguration Glo
} }
func (server *Server) wireFrontendBackend(serverRoute *serverRoute, handler http.Handler) { func (server *Server) wireFrontendBackend(serverRoute *serverRoute, handler http.Handler) {
// add prefix // path replace - This needs to always be the very last on the handler chain (first in the order in this function)
// -- Replacing Path should happen at the very end of the Modifier chain, after all the Matcher+Modifiers ran
if len(serverRoute.replacePath) > 0 {
handler = &middlewares.ReplacePath{
Path: serverRoute.replacePath,
Handler: handler,
}
}
// add prefix - This needs to always be right before ReplacePath on the chain (second in order in this function)
// -- Adding Path Prefix should happen after all *Strip Matcher+Modifiers ran, but before Replace (in case it's configured)
if len(serverRoute.addPrefix) > 0 { if len(serverRoute.addPrefix) > 0 {
handler = &middlewares.AddPrefix{ handler = &middlewares.AddPrefix{
Prefix: serverRoute.addPrefix, Prefix: serverRoute.addPrefix,
@ -797,14 +807,6 @@ func (server *Server) wireFrontendBackend(serverRoute *serverRoute, handler http
handler = middlewares.NewStripPrefixRegex(handler, serverRoute.stripPrefixesRegex) handler = middlewares.NewStripPrefixRegex(handler, serverRoute.stripPrefixesRegex)
} }
// path replace
if len(serverRoute.replacePath) > 0 {
handler = &middlewares.ReplacePath{
Path: serverRoute.replacePath,
Handler: handler,
}
}
serverRoute.route.Handler(handler) serverRoute.route.Handler(handler)
} }

View file

@ -2,13 +2,16 @@ package server
import ( import (
"fmt" "fmt"
"net/http"
"net/url" "net/url"
"reflect" "reflect"
"testing" "testing"
"time" "time"
"github.com/containous/flaeg" "github.com/containous/flaeg"
"github.com/containous/mux"
"github.com/containous/traefik/healthcheck" "github.com/containous/traefik/healthcheck"
"github.com/containous/traefik/testhelpers"
"github.com/containous/traefik/types" "github.com/containous/traefik/types"
"github.com/vulcand/oxy/roundrobin" "github.com/vulcand/oxy/roundrobin"
) )
@ -27,6 +30,85 @@ func (lb *testLoadBalancer) Servers() []*url.URL {
return []*url.URL{} return []*url.URL{}
} }
func TestServerMultipleFrontendRules(t *testing.T) {
cases := []struct {
expression string
requestURL string
expectedURL string
}{
{
expression: "Host:foo.bar",
requestURL: "http://foo.bar",
expectedURL: "http://foo.bar",
},
{
expression: "PathPrefix:/management;ReplacePath:/health",
requestURL: "http://foo.bar/management",
expectedURL: "http://foo.bar/health",
},
{
expression: "Host:foo.bar;AddPrefix:/blah",
requestURL: "http://foo.bar/baz",
expectedURL: "http://foo.bar/blah/baz",
},
{
expression: "PathPrefixStripRegex:/one/{two}/{three:[0-9]+}",
requestURL: "http://foo.bar/one/some/12345/four",
expectedURL: "http://foo.bar/four",
},
{
expression: "PathPrefixStripRegex:/one/{two}/{three:[0-9]+};AddPrefix:/zero",
requestURL: "http://foo.bar/one/some/12345/four",
expectedURL: "http://foo.bar/zero/four",
},
{
expression: "AddPrefix:/blah;ReplacePath:/baz",
requestURL: "http://foo.bar/hello",
expectedURL: "http://foo.bar/baz",
},
{
expression: "PathPrefixStrip:/management;ReplacePath:/health",
requestURL: "http://foo.bar/management",
expectedURL: "http://foo.bar/health",
},
}
for _, test := range cases {
test := test
t.Run(test.expression, func(t *testing.T) {
t.Parallel()
router := mux.NewRouter()
route := router.NewRoute()
serverRoute := &serverRoute{route: route}
rules := &Rules{route: serverRoute}
expression := test.expression
routeResult, err := rules.Parse(expression)
if err != nil {
t.Fatalf("Error while building route for %s: %+v", expression, err)
}
request := testhelpers.MustNewRequest(http.MethodGet, test.requestURL, nil)
routeMatch := routeResult.Match(request, &mux.RouteMatch{Route: routeResult})
if !routeMatch {
t.Fatalf("Rule %s doesn't match", expression)
}
server := new(Server)
server.wireFrontendBackend(serverRoute, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.String() != test.expectedURL {
t.Fatalf("got URL %s, expected %s", r.URL.String(), test.expectedURL)
}
}))
serverRoute.route.GetHandler().ServeHTTP(nil, request)
})
}
}
func TestServerLoadConfigHealthCheckOptions(t *testing.T) { func TestServerLoadConfigHealthCheckOptions(t *testing.T) {
healthChecks := []*types.HealthCheck{ healthChecks := []*types.HealthCheck{
nil, nil,

View file

@ -1,6 +1,21 @@
package testhelpers package testhelpers
import (
"fmt"
"io"
"net/http"
)
// Intp returns a pointer to the given integer value. // Intp returns a pointer to the given integer value.
func Intp(i int) *int { func Intp(i int) *int {
return &i return &i
} }
// MustNewRequest creates a new http get request or panics if it can't
func MustNewRequest(method, urlStr string, body io.Reader) *http.Request {
request, err := http.NewRequest(method, urlStr, body)
if err != nil {
panic(fmt.Sprintf("failed to create HTTP %s Request for '%s': %s", method, urlStr, err))
}
return request
}