From f843f260ee6a1dbb99e800aa951929658bc98421 Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Tue, 3 Sep 2019 20:32:03 +0200 Subject: [PATCH] fix: stripPrefix and stripPrefixRegex. --- docs/content/middlewares/stripprefixregex.md | 19 ++---- .../fixtures/https/https_redirect.toml | 4 +- pkg/middlewares/stripprefix/strip_prefix.go | 2 +- .../stripprefix/strip_prefix_test.go | 8 ++- .../stripprefixregex/strip_prefix_regex.go | 61 +++++++++---------- .../strip_prefix_regex_test.go | 13 +++- 6 files changed, 55 insertions(+), 52 deletions(-) diff --git a/docs/content/middlewares/stripprefixregex.md b/docs/content/middlewares/stripprefixregex.md index 84e55d979..4daee17c4 100644 --- a/docs/content/middlewares/stripprefixregex.md +++ b/docs/content/middlewares/stripprefixregex.md @@ -3,20 +3,16 @@ Removing Prefixes From the Path Before Forwarding the Request (Using a Regex) {: .subtitle } -`TODO: add schema` - Remove the matching prefixes from the URL path. ## Configuration Examples ```yaml tab="Docker" -# Replace the path by /foo labels: -- "traefik.http.middlewares.test-stripprefixregex.stripprefixregex.regex=^/foo/(.*)", +- "traefik.http.middlewares.test-stripprefixregex.stripprefixregex.regex=/foo/[a-z0-9]+/[0-9]+/", ``` ```yaml tab="Kubernetes" -# Replace the path by /foo apiVersion: traefik.containo.us/v1alpha1 kind: Middleware metadata: @@ -24,36 +20,33 @@ metadata: spec: stripPrefixRegex: regex: - - "^/foo/(.*)" + - "/foo/[a-z0-9]+/[0-9]+/" ``` ```json tab="Marathon" "labels": { - "traefik.http.middlewares.test-stripprefixregex.stripprefixregex.regex": "^/foo/(.*)" + "traefik.http.middlewares.test-stripprefixregex.stripprefixregex.regex": "/foo/[a-z0-9]+/[0-9]+/" } ``` ```yaml tab="Rancher" -# Replace the path by /foo labels: -- "traefik.http.middlewares.test-stripprefixregex.stripprefixregex.regex=^/foo/(.*)", +- "traefik.http.middlewares.test-stripprefixregex.stripprefixregex.regex=/foo/[a-z0-9]+/[0-9]+/", ``` ```toml tab="File (TOML)" -# Replace the path by /foo [http.middlewares] [http.middlewares.test-stripprefixregex.stripPrefixRegex] - regex = ["^/foo/(.*)"] + regex = ["/foo/[a-z0-9]+/[0-9]+/"] ``` ```yaml tab="File (YAML)" -# Replace the path by /foo http: middlewares: test-stripprefixregex: stripPrefixRegex: regex: - - "^/foo/(.*)" + - "/foo/[a-z0-9]+/[0-9]+/" ``` ## Configuration Options diff --git a/integration/fixtures/https/https_redirect.toml b/integration/fixtures/https/https_redirect.toml index e9e9f78bc..3147488c4 100644 --- a/integration/fixtures/https/https_redirect.toml +++ b/integration/fixtures/https/https_redirect.toml @@ -150,9 +150,9 @@ [http.middlewares.foo-slash-add-prefix.addPrefix] prefix = "/foo/" [http.middlewares.id-strip-regex-prefix.stripPrefixRegex] - regex = ["/{id:[a-z]+}"] + regex = ["/[a-z]+"] [http.middlewares.id-slash-strip-regex-prefix.stripPrefixRegex] - regex = ["/{id:[a-z]+}/"] + regex = ["/[a-z]+/"] [http.middlewares.api-regex-replace.replacePathRegex] regex = "/api" replacement = "/" diff --git a/pkg/middlewares/stripprefix/strip_prefix.go b/pkg/middlewares/stripprefix/strip_prefix.go index 42a113206..31bc5ae90 100644 --- a/pkg/middlewares/stripprefix/strip_prefix.go +++ b/pkg/middlewares/stripprefix/strip_prefix.go @@ -49,7 +49,7 @@ func (s *stripPrefix) ServeHTTP(rw http.ResponseWriter, req *http.Request) { return } } - http.NotFound(rw, req) + s.next.ServeHTTP(rw, req) } func (s *stripPrefix) serveRequest(rw http.ResponseWriter, req *http.Request, prefix string) { diff --git a/pkg/middlewares/stripprefix/strip_prefix_test.go b/pkg/middlewares/stripprefix/strip_prefix_test.go index 31d417162..449720902 100644 --- a/pkg/middlewares/stripprefix/strip_prefix_test.go +++ b/pkg/middlewares/stripprefix/strip_prefix_test.go @@ -28,7 +28,8 @@ func TestStripPrefix(t *testing.T) { Prefixes: []string{}, }, path: "/noprefixes", - expectedStatusCode: http.StatusNotFound, + expectedStatusCode: http.StatusOK, + expectedPath: "/noprefixes", }, { desc: "wildcard (.*) requests", @@ -76,7 +77,8 @@ func TestStripPrefix(t *testing.T) { Prefixes: []string{"/stat/"}, }, path: "/status", - expectedStatusCode: http.StatusNotFound, + expectedStatusCode: http.StatusOK, + expectedPath: "/status", }, { desc: "general prefix on matching path", @@ -149,6 +151,8 @@ func TestStripPrefix(t *testing.T) { require.NoError(t, err) req := testhelpers.MustNewRequest(http.MethodGet, "http://localhost"+test.path, nil) + req.RequestURI = req.URL.RequestURI() + resp := &httptest.ResponseRecorder{Code: http.StatusOK} handler.ServeHTTP(resp, req) diff --git a/pkg/middlewares/stripprefixregex/strip_prefix_regex.go b/pkg/middlewares/stripprefixregex/strip_prefix_regex.go index c2c98328e..6cc82f178 100644 --- a/pkg/middlewares/stripprefixregex/strip_prefix_regex.go +++ b/pkg/middlewares/stripprefixregex/strip_prefix_regex.go @@ -3,13 +3,13 @@ package stripprefixregex import ( "context" "net/http" + "regexp" "strings" "github.com/containous/traefik/v2/pkg/config/dynamic" "github.com/containous/traefik/v2/pkg/middlewares" "github.com/containous/traefik/v2/pkg/middlewares/stripprefix" "github.com/containous/traefik/v2/pkg/tracing" - "github.com/gorilla/mux" "github.com/opentracing/opentracing-go/ext" ) @@ -19,9 +19,9 @@ const ( // StripPrefixRegex is a middleware used to strip prefix from an URL request. type stripPrefixRegex struct { - next http.Handler - router *mux.Router - name string + next http.Handler + expressions []*regexp.Regexp + name string } // New builds a new StripPrefixRegex middleware. @@ -29,13 +29,16 @@ func New(ctx context.Context, next http.Handler, config dynamic.StripPrefixRegex middlewares.GetLogger(ctx, name, typeName).Debug("Creating middleware") stripPrefix := stripPrefixRegex{ - next: next, - router: mux.NewRouter(), - name: name, + next: next, + name: name, } - for _, prefix := range config.Regex { - stripPrefix.router.PathPrefix(prefix) + for _, exp := range config.Regex { + reg, err := regexp.Compile(strings.TrimSpace(exp)) + if err != nil { + return nil, err + } + stripPrefix.expressions = append(stripPrefix.expressions, reg) } return &stripPrefix, nil @@ -46,32 +49,28 @@ func (s *stripPrefixRegex) GetTracingInformation() (string, ext.SpanKindEnum) { } func (s *stripPrefixRegex) ServeHTTP(rw http.ResponseWriter, req *http.Request) { - var match mux.RouteMatch - if s.router.Match(req, &match) { - params := make([]string, 0, len(match.Vars)*2) - for key, val := range match.Vars { - params = append(params, key) - params = append(params, val) - } + for _, exp := range s.expressions { + parts := exp.FindStringSubmatch(req.URL.Path) + if len(parts) > 0 && len(parts[0]) > 0 { + prefix := parts[0] + if !strings.HasPrefix(req.URL.Path, prefix) { + continue + } - prefix, err := match.Route.URL(params...) - if err != nil || len(prefix.Path) > len(req.URL.Path) { - logger := middlewares.GetLogger(req.Context(), s.name, typeName) - logger.Error("Error in stripPrefix middleware", err) + req.Header.Add(stripprefix.ForwardedPrefixHeader, prefix) + + req.URL.Path = strings.Replace(req.URL.Path, prefix, "", 1) + if req.URL.RawPath != "" { + req.URL.RawPath = req.URL.RawPath[len(prefix):] + } + + req.RequestURI = ensureLeadingSlash(req.URL.RequestURI()) + s.next.ServeHTTP(rw, req) return } - - req.URL.Path = req.URL.Path[len(prefix.Path):] - if req.URL.RawPath != "" { - req.URL.RawPath = req.URL.RawPath[len(prefix.Path):] - } - req.Header.Add(stripprefix.ForwardedPrefixHeader, prefix.Path) - req.RequestURI = ensureLeadingSlash(req.URL.RequestURI()) - - s.next.ServeHTTP(rw, req) - return } - http.NotFound(rw, req) + + s.next.ServeHTTP(rw, req) } func ensureLeadingSlash(str string) string { diff --git a/pkg/middlewares/stripprefixregex/strip_prefix_regex_test.go b/pkg/middlewares/stripprefixregex/strip_prefix_regex_test.go index df67d9f57..297fa7192 100644 --- a/pkg/middlewares/stripprefixregex/strip_prefix_regex_test.go +++ b/pkg/middlewares/stripprefixregex/strip_prefix_regex_test.go @@ -15,7 +15,7 @@ import ( func TestStripPrefixRegex(t *testing.T) { testPrefixRegex := dynamic.StripPrefixRegex{ - Regex: []string{"/a/api/", "/b/{regex}/", "/c/{category}/{id:[0-9]+}/"}, + Regex: []string{"/a/api/", "/b/([a-z0-9]+)/", "/c/[a-z0-9]+/[0-9]+/"}, } testCases := []struct { @@ -27,7 +27,13 @@ func TestStripPrefixRegex(t *testing.T) { }{ { path: "/a/test", - expectedStatusCode: http.StatusNotFound, + expectedStatusCode: http.StatusOK, + expectedPath: "/a/test", + }, + { + path: "/a/test", + expectedStatusCode: http.StatusOK, + expectedPath: "/a/test", }, { path: "/a/api/test", @@ -65,7 +71,8 @@ func TestStripPrefixRegex(t *testing.T) { }, { path: "/c/api/abc/test4", - expectedStatusCode: http.StatusNotFound, + expectedStatusCode: http.StatusOK, + expectedPath: "/c/api/abc/test4", }, { path: "/a/api/a%2Fb",