Handle URL-encoded NUL bytes (%00) in URL path and query (#877)
<!-- Provide a brief summary of your changes --> ## Motivation and Context <!-- Why is this change needed? What problem does it solve? --> Follow up on #869 ## How Has This Been Tested? <!-- Have you tested this in a real application? Which scenarios were tested? --> ## Breaking Changes <!-- Will users need to update their code or configurations? --> ## Types of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update ## Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply. --> - [ ] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [ ] My code follows the repository's style guidelines - [ ] New and existing tests pass locally - [ ] I have added appropriate error handling - [ ] I have added or updated documentation as needed ## Additional context <!-- Add any other context, implementation notes, or design decisions --> Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
This commit is contained in:
parent
c6856a818b
commit
ad207f2390
1 changed files with 125 additions and 8 deletions
|
|
@ -52,6 +52,10 @@ func TestNulByteValidationMiddleware(t *testing.T) {
|
|||
if !strings.Contains(w.Body.String(), "URL path contains null bytes") {
|
||||
t.Errorf("expected body to contain error message, got %q", w.Body.String())
|
||||
}
|
||||
// Verify JSON response format
|
||||
if w.Header().Get("Content-Type") != "application/json" {
|
||||
t.Errorf("expected Content-Type application/json, got %q", w.Header().Get("Content-Type"))
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("query with NUL byte should return 400", func(t *testing.T) {
|
||||
|
|
@ -79,6 +83,90 @@ func TestNulByteValidationMiddleware(t *testing.T) {
|
|||
t.Errorf("expected status %d, got %d", http.StatusBadRequest, w.Code)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("query with URL-encoded NUL byte (%00) should return 400", func(t *testing.T) {
|
||||
// This is the exact case from issue #862: ?cursor=%00
|
||||
req := httptest.NewRequest(http.MethodGet, "/v0/servers?cursor=%00", nil)
|
||||
w := httptest.NewRecorder()
|
||||
middleware.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("expected status %d, got %d", http.StatusBadRequest, w.Code)
|
||||
}
|
||||
if !strings.Contains(w.Body.String(), "query parameters contain null bytes") {
|
||||
t.Errorf("expected body to contain error message, got %q", w.Body.String())
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("query with URL-encoded NUL byte followed by text should return 400", func(t *testing.T) {
|
||||
req := httptest.NewRequest(http.MethodGet, "/v0/servers?cursor=%00test", nil)
|
||||
w := httptest.NewRecorder()
|
||||
middleware.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("expected status %d, got %d", http.StatusBadRequest, w.Code)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("query with embedded URL-encoded NUL byte should return 400", func(t *testing.T) {
|
||||
req := httptest.NewRequest(http.MethodGet, "/v0/servers?cursor=abc%00def", nil)
|
||||
w := httptest.NewRecorder()
|
||||
middleware.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("expected status %d, got %d", http.StatusBadRequest, w.Code)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("query with double-encoded NUL byte (%2500) should pass through", func(t *testing.T) {
|
||||
// %2500 decodes to %00 (literal string), not a NUL byte
|
||||
// This is intentionally allowed - double-decoding is the caller's responsibility
|
||||
req := httptest.NewRequest(http.MethodGet, "/v0/servers?cursor=%2500", nil)
|
||||
w := httptest.NewRecorder()
|
||||
middleware.ServeHTTP(w, req)
|
||||
|
||||
// This should pass - %2500 is not a NUL byte injection attempt
|
||||
// When decoded once: %2500 -> %00 (the string "%00", not a NUL byte)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Errorf("expected status %d, got %d (double-encoded should pass)", http.StatusOK, w.Code)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("query with valid percent-encoding should pass through", func(t *testing.T) {
|
||||
// Ensure we don't false-positive on valid encodings like %20 (space)
|
||||
req := httptest.NewRequest(http.MethodGet, "/v0/servers?search=hello%20world", nil)
|
||||
w := httptest.NewRecorder()
|
||||
middleware.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Errorf("expected status %d, got %d", http.StatusOK, w.Code)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("path with URL-encoded NUL byte (%00) should return 400", func(t *testing.T) {
|
||||
// Handlers call url.PathUnescape() which would decode %00 to \x00
|
||||
req := httptest.NewRequest(http.MethodGet, "/v0/servers/%00/versions", nil)
|
||||
w := httptest.NewRecorder()
|
||||
middleware.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("expected status %d, got %d", http.StatusBadRequest, w.Code)
|
||||
}
|
||||
if !strings.Contains(w.Body.String(), "URL path contains null bytes") {
|
||||
t.Errorf("expected body to contain error message, got %q", w.Body.String())
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("path with URL-encoded NUL byte among other encodings should return 400", func(t *testing.T) {
|
||||
// %0a is newline, %00 is NUL - should still catch the NUL
|
||||
req := httptest.NewRequest(http.MethodGet, "/v0/servers/test%0a%00name/versions", nil)
|
||||
w := httptest.NewRecorder()
|
||||
middleware.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("expected status %d, got %d", http.StatusBadRequest, w.Code)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestTrailingSlashMiddleware(t *testing.T) {
|
||||
|
|
|
|||
Loading…
Reference in a new issue