diff --git a/pkg/middlewares/stripprefix/strip_prefix.go b/pkg/middlewares/stripprefix/strip_prefix.go index 406b13ab24..53e54c0ece 100644 --- a/pkg/middlewares/stripprefix/strip_prefix.go +++ b/pkg/middlewares/stripprefix/strip_prefix.go @@ -44,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 = s.getPrefixStripped(req.URL.Path, prefix) + req.URL.Path = s.getPathStripped(req.URL.Path, prefix) if req.URL.RawPath != "" { - req.URL.RawPath = s.getPrefixStripped(req.URL.RawPath, prefix) + req.URL.RawPath = s.getRawPathStripped(req.URL.RawPath, prefix) } s.serveRequest(rw, req, strings.TrimSpace(prefix)) return @@ -61,7 +61,7 @@ func (s *stripPrefix) serveRequest(rw http.ResponseWriter, req *http.Request, pr s.next.ServeHTTP(rw, req) } -func (s *stripPrefix) getPrefixStripped(urlPath, prefix string) string { +func (s *stripPrefix) getPathStripped(urlPath, prefix string) string { if s.forceSlash { // Only for compatibility reason with the previous behavior, // but the previous behavior is wrong. @@ -72,6 +72,33 @@ func (s *stripPrefix) getPrefixStripped(urlPath, prefix string) string { return ensureLeadingSlash(strings.TrimPrefix(urlPath, prefix)) } +func (s *stripPrefix) getRawPathStripped(rawPath, 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(rawPath[encodedPrefixLen(rawPath, prefix):], "/") + } + + return ensureLeadingSlash(rawPath[encodedPrefixLen(rawPath, prefix):]) +} + +// encodedPrefixLen returns the number of bytes in rawPath that correspond to +// the decoded prefix, advancing 3 bytes per %XX sequence and 1 byte otherwise. +func encodedPrefixLen(rawPath, decodedPrefix string) int { + decoded := 0 + i := 0 + for i < len(rawPath) && decoded < len(decodedPrefix) { + if rawPath[i] == '%' && i+2 < len(rawPath) { + i += 3 + } else { + i++ + } + decoded++ + } + return i +} + func ensureLeadingSlash(str string) string { if str == "" { return str diff --git a/pkg/middlewares/stripprefix/strip_prefix_test.go b/pkg/middlewares/stripprefix/strip_prefix_test.go index 8ed6ebe921..18d66c7145 100644 --- a/pkg/middlewares/stripprefix/strip_prefix_test.go +++ b/pkg/middlewares/stripprefix/strip_prefix_test.go @@ -174,6 +174,17 @@ func TestStripPrefix(t *testing.T) { expectedRawPath: "/a%2Fb", expectedHeader: "/stat", }, + { + desc: "encoded char in prefix segment of raw path", + config: dynamic.StripPrefix{ + Prefixes: []string{"/api/"}, + }, + path: "/ap%69/a%2Fb", + expectedStatusCode: http.StatusOK, + expectedPath: "/a/b", + expectedRawPath: "/a%2Fb", + expectedHeader: "/api/", + }, } for _, test := range testCases { diff --git a/pkg/middlewares/stripprefixregex/strip_prefix_regex.go b/pkg/middlewares/stripprefixregex/strip_prefix_regex.go index ee0722ad32..8a1997941f 100644 --- a/pkg/middlewares/stripprefixregex/strip_prefix_regex.go +++ b/pkg/middlewares/stripprefixregex/strip_prefix_regex.go @@ -62,7 +62,7 @@ func (s *stripPrefixRegex) ServeHTTP(rw http.ResponseWriter, req *http.Request) req.URL.Path = ensureLeadingSlash(strings.Replace(req.URL.Path, prefix, "", 1)) if req.URL.RawPath != "" { - req.URL.RawPath = ensureLeadingSlash(req.URL.RawPath[len(prefix):]) + req.URL.RawPath = ensureLeadingSlash(req.URL.RawPath[encodedPrefixLen(req.URL.RawPath, prefix):]) } req.RequestURI = req.URL.RequestURI() @@ -74,6 +74,22 @@ func (s *stripPrefixRegex) ServeHTTP(rw http.ResponseWriter, req *http.Request) s.next.ServeHTTP(rw, req) } +// encodedPrefixLen returns the number of bytes in rawPath that correspond to +// the decoded prefix, advancing 3 bytes per %XX sequence and 1 byte otherwise. +func encodedPrefixLen(rawPath, decodedPrefix string) int { + decoded := 0 + i := 0 + for i < len(rawPath) && decoded < len(decodedPrefix) { + if rawPath[i] == '%' && i+2 < len(rawPath) { + i += 3 + } else { + i++ + } + decoded++ + } + return i +} + func ensureLeadingSlash(str string) string { if str == "" { return str diff --git a/pkg/middlewares/stripprefixregex/strip_prefix_regex_test.go b/pkg/middlewares/stripprefixregex/strip_prefix_regex_test.go index 5a9b6d1156..c0caf73056 100644 --- a/pkg/middlewares/stripprefixregex/strip_prefix_regex_test.go +++ b/pkg/middlewares/stripprefixregex/strip_prefix_regex_test.go @@ -13,111 +13,204 @@ import ( ) func TestStripPrefixRegex(t *testing.T) { - testPrefixRegex := dynamic.StripPrefixRegex{ - Regex: []string{"/a/api/", "/b/([a-z0-9]+)/", "/c/[a-z0-9]+/[0-9]+/"}, - } - testCases := []struct { + desc string + config dynamic.StripPrefixRegex path string expectedStatusCode int expectedPath string expectedRawPath string + expectedRequestURI string expectedHeader string }{ { + desc: "/a/test", + config: dynamic.StripPrefixRegex{Regex: []string{"/a/api/"}}, path: "/a/test", expectedStatusCode: http.StatusOK, expectedPath: "/a/test", }, { + desc: "/a/test/", + config: dynamic.StripPrefixRegex{Regex: []string{"/a/api/"}}, path: "/a/test/", expectedStatusCode: http.StatusOK, expectedPath: "/a/test/", }, { + desc: "/a/api/", + config: dynamic.StripPrefixRegex{Regex: []string{"/a/api/"}}, path: "/a/api/", expectedStatusCode: http.StatusOK, + // ensureLeadingSlash do not add a slash when the path is empty. expectedPath: "", + expectedRequestURI: "/", expectedHeader: "/a/api/", }, { + desc: "/a/api/test", + config: dynamic.StripPrefixRegex{Regex: []string{"/a/api/"}}, path: "/a/api/test", expectedStatusCode: http.StatusOK, expectedPath: "/test", + expectedRequestURI: "/test", expectedHeader: "/a/api/", }, { + desc: "/a/api/test/", + config: dynamic.StripPrefixRegex{Regex: []string{"/a/api/"}}, path: "/a/api/test/", expectedStatusCode: http.StatusOK, expectedPath: "/test/", + expectedRequestURI: "/test/", expectedHeader: "/a/api/", }, { + desc: "/b/api/", + config: dynamic.StripPrefixRegex{Regex: []string{"/b/([a-z0-9]+)/"}}, path: "/b/api/", expectedStatusCode: http.StatusOK, expectedPath: "", + expectedRequestURI: "/", expectedHeader: "/b/api/", }, { + desc: "/b/api", + config: dynamic.StripPrefixRegex{Regex: []string{"/b/([a-z0-9]+)/"}}, path: "/b/api", expectedStatusCode: http.StatusOK, expectedPath: "/b/api", + // When the path do not match, the requestURI is not computed. + expectedRequestURI: "", }, { + desc: "/b/api/test1", + config: dynamic.StripPrefixRegex{Regex: []string{"/b/([a-z0-9]+)/"}}, path: "/b/api/test1", expectedStatusCode: http.StatusOK, expectedPath: "/test1", + expectedRequestURI: "/test1", expectedHeader: "/b/api/", }, { + desc: "/b/api2/test2", + config: dynamic.StripPrefixRegex{Regex: []string{"/b/([a-z0-9]+)/"}}, path: "/b/api2/test2", expectedStatusCode: http.StatusOK, expectedPath: "/test2", + expectedRequestURI: "/test2", expectedHeader: "/b/api2/", }, { + desc: "/c/api/123/", + config: dynamic.StripPrefixRegex{Regex: []string{"/c/[a-z0-9]+/[0-9]+/"}}, path: "/c/api/123/", expectedStatusCode: http.StatusOK, expectedPath: "", + expectedRequestURI: "/", expectedHeader: "/c/api/123/", }, { + desc: "/c/api/123", + config: dynamic.StripPrefixRegex{Regex: []string{"/c/[a-z0-9]+/[0-9]+/"}}, path: "/c/api/123", expectedStatusCode: http.StatusOK, expectedPath: "/c/api/123", + // When the path do not match, the requestURI is not computed. + expectedRequestURI: "", }, { + desc: "/c/api/123/test3", + config: dynamic.StripPrefixRegex{Regex: []string{"/c/[a-z0-9]+/[0-9]+/"}}, path: "/c/api/123/test3", expectedStatusCode: http.StatusOK, expectedPath: "/test3", + expectedRequestURI: "/test3", expectedHeader: "/c/api/123/", }, { + desc: "/c/api/abc/test4", + config: dynamic.StripPrefixRegex{Regex: []string{"/c/[a-z0-9]+/[0-9]+/"}}, path: "/c/api/abc/test4", expectedStatusCode: http.StatusOK, expectedPath: "/c/api/abc/test4", + // When the path do not match, the requestURI is not computed. + expectedRequestURI: "", }, { + desc: "/a/api/a2Fb", + config: dynamic.StripPrefixRegex{Regex: []string{"/a/api/"}}, path: "/a/api/a%2Fb", expectedStatusCode: http.StatusOK, expectedPath: "/a/b", expectedRawPath: "/a%2Fb", + expectedRequestURI: "/a%2Fb", expectedHeader: "/a/api/", }, + { + desc: "/b/ap69/test", + config: dynamic.StripPrefixRegex{Regex: []string{"/b/([a-z0-9]+)/"}}, + path: "/b/ap%69/test", + expectedStatusCode: http.StatusOK, + expectedPath: "/test", + expectedRawPath: "/test", + expectedRequestURI: "/test", + expectedHeader: "/b/api/", + }, + { + desc: "/b/ap69/a2Fb", + config: dynamic.StripPrefixRegex{Regex: []string{"/b/([a-z0-9]+)/"}}, + path: "/b/ap%69/a%2Fb", + expectedStatusCode: http.StatusOK, + expectedPath: "/a/b", + expectedRawPath: "/a%2Fb", + expectedRequestURI: "/a%2Fb", + expectedHeader: "/b/api/", + }, + { + desc: "/t2F/test/foo", + config: dynamic.StripPrefixRegex{Regex: []string{"/t /test"}}, + path: "/t%2F/test/foo", + expectedStatusCode: http.StatusOK, + expectedPath: "/t//test/foo", + expectedRawPath: "/t%2F/test/foo", + // When the path do not match, the requestURI is not computed. + expectedRequestURI: "", + }, + { + desc: "/t /test/a2Fb", + config: dynamic.StripPrefixRegex{Regex: []string{"/t /test"}}, + path: "/t /test/a%2Fb", + expectedStatusCode: http.StatusOK, + expectedPath: "/a/b", + expectedRawPath: "/a%2Fb", + expectedRequestURI: "/a%2Fb", + expectedHeader: "/t /test", + }, + { + desc: "/t20/test/a2Fb", + config: dynamic.StripPrefixRegex{Regex: []string{"/t /test"}}, + path: "/t%20/test/a%2Fb", + expectedStatusCode: http.StatusOK, + expectedPath: "/a/b", + expectedRawPath: "/a%2Fb", + expectedRequestURI: "/a%2Fb", + expectedHeader: "/t /test", + }, } for _, test := range testCases { - t.Run(test.path, func(t *testing.T) { + t.Run(test.desc, func(t *testing.T) { t.Parallel() - var actualPath, actualRawPath, actualHeader, requestURI string + var actualPath, actualRawPath, actualHeader, actualRequestURI 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 + actualRequestURI = r.RequestURI }) - handler, err := New(t.Context(), handlerPath, testPrefixRegex, "foo-strip-prefix-regex") + handler, err := New(t.Context(), handlerPath, test.config, "foo-strip-prefix-regex") require.NoError(t, err) req := testhelpers.MustNewRequest(http.MethodGet, "http://localhost"+test.path, nil) @@ -129,18 +222,7 @@ 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.") - } + assert.Equal(t, test.expectedRequestURI, actualRequestURI, "Unexpected request uri.") }) } }