Reject requests with different paths after StripPrefix and StripPrefixRegex normalisation

This commit is contained in:
Romain 2026-05-28 15:56:25 +02:00 committed by GitHub
parent 5026ca97d0
commit 892bcc288b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 48 additions and 24 deletions

View file

@ -856,3 +856,20 @@ and [Kubernetes Gateway](../providers/kubernetes-gateway.md#crossprovidernamespa
From version `v2.11.47` onwards, the BasicAuth middleware requires a non-empty users configuration in order to be built successfully.
Previously, the middleware would be built successfully but always return a 401 status code for any request.
Now, an error occurs and any routers using it will be unmounted. For the same request, a 404 status code is served instead of a 401 status code.
### StripPrefix and StripPrefixRegex Middleware
From version `v2.11.47` onwards, the StripPrefix middleware and the StripPrefixRegex middleware reject requests (`400 Bad Request`)
when stripping the configured prefix produces a path that differs from its normalised form
(i.e. a path containing `.` or `..` segments that would be collapsed by normalisation).
This prevents the stripped path from being interpreted as a different resource by the upstream service.
Examples with a configured prefix of `/api`:
| Request path | Path after strip | Normalised path | Result |
|--------------|------------------|-----------------|--------------|
| `/api/foo` | `/foo` | `/foo` | `200` (sent) |
| `/api/` | `/` | `/` | `200` (sent) |
| `/api./foo` | `/./foo` | `/foo` | `400` |
| `/api../foo` | `/../foo` | `/foo` | `400` |

View file

@ -48,6 +48,8 @@ func (s *stripPrefix) GetTracingInformation() (string, ext.SpanKindEnum) {
}
func (s *stripPrefix) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
logger := log.FromContext(middlewares.GetLoggerCtx(req.Context(), s.name, typeName))
for _, prefix := range s.prefixes {
if strings.HasPrefix(req.URL.Path, prefix) {
req.URL.Path = s.getPathStripped(req.URL.Path, prefix)
@ -58,10 +60,18 @@ func (s *stripPrefix) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
// Here we are sanitizing the URL when the path is not empty,
// as the JoinPath method is adding a leading slash if the path is empty
// to be aligned with ensureLeadingSlash behavior.
if req.URL.Path != "" {
path := req.URL.Path
if path != "" {
req.URL = req.URL.JoinPath()
}
// Stop here if the normalization of the path produces a different path.
if path != req.URL.Path {
logger.Debugf("Rejecting request, sanitized path: %q is not equivalent to stripped path: %q", path, req.URL.Path)
http.Error(rw, http.StatusText(http.StatusBadRequest), http.StatusBadRequest)
return
}
req.Header.Add(ForwardedPrefixHeader, prefix)
req.RequestURI = req.URL.RequestURI()
break

View file

@ -191,10 +191,7 @@ func TestStripPrefix(t *testing.T) {
Prefixes: []string{"/api"},
},
path: "/api./foo",
expectedStatusCode: http.StatusOK,
expectedPath: "/foo",
expectedRawPath: "",
expectedHeader: "/api",
expectedStatusCode: http.StatusBadRequest,
},
{
desc: "multiple dots in the path not stripped by the prefix",
@ -202,10 +199,7 @@ func TestStripPrefix(t *testing.T) {
Prefixes: []string{"/api"},
},
path: "/api../foo",
expectedStatusCode: http.StatusOK,
expectedPath: "/foo",
expectedRawPath: "",
expectedHeader: "/api",
expectedStatusCode: http.StatusBadRequest,
},
{
desc: "multiple dots in the path not stripped by the prefix with forceSlash",
@ -214,10 +208,7 @@ func TestStripPrefix(t *testing.T) {
ForceSlash: true,
},
path: "/api../foo",
expectedStatusCode: http.StatusOK,
expectedPath: "/foo",
expectedRawPath: "",
expectedHeader: "/api",
expectedStatusCode: http.StatusBadRequest,
},
}
@ -244,6 +235,10 @@ func TestStripPrefix(t *testing.T) {
handler.ServeHTTP(resp, req)
assert.Equal(t, test.expectedStatusCode, resp.Code, "Unexpected status code.")
if test.expectedStatusCode != http.StatusOK {
return
}
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.", ForwardedPrefixHeader)

View file

@ -50,6 +50,8 @@ func (s *stripPrefixRegex) GetTracingInformation() (string, ext.SpanKindEnum) {
}
func (s *stripPrefixRegex) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
logger := log.FromContext(middlewares.GetLoggerCtx(req.Context(), s.name, typeName))
for _, exp := range s.expressions {
parts := exp.FindStringSubmatch(req.URL.Path)
if len(parts) > 0 && len(parts[0]) > 0 {
@ -68,10 +70,18 @@ func (s *stripPrefixRegex) ServeHTTP(rw http.ResponseWriter, req *http.Request)
// Here we are sanitizing the URL when the path is not empty,
// as the JoinPath method is adding a leading slash if the path is empty
// to be aligned with ensureLeadingSlash behavior.
if req.URL.Path != "" {
path := req.URL.Path
if path != "" {
req.URL = req.URL.JoinPath()
}
// Stop here if the normalization of the path produces a different path.
if path != req.URL.Path {
logger.Debugf("Rejecting request, sanitized path: %q is not equivalent to stripped path: %q", path, req.URL.Path)
http.Error(rw, http.StatusText(http.StatusBadRequest), http.StatusBadRequest)
return
}
req.RequestURI = req.URL.RequestURI()
break
}

View file

@ -201,21 +201,13 @@ func TestStripPrefixRegex(t *testing.T) {
desc: "/api./foo",
config: dynamic.StripPrefixRegex{Regex: []string{"/api"}},
path: "/api./foo",
expectedStatusCode: http.StatusOK,
expectedPath: "/foo",
expectedRawPath: "",
expectedRequestURI: "/foo",
expectedHeader: "/api",
expectedStatusCode: http.StatusBadRequest,
},
{
desc: "/api../foo",
config: dynamic.StripPrefixRegex{Regex: []string{"/api"}},
path: "/api../foo",
expectedStatusCode: http.StatusOK,
expectedPath: "/foo",
expectedRawPath: "",
expectedRequestURI: "/foo",
expectedHeader: "/api",
expectedStatusCode: http.StatusBadRequest,
},
}