Return HTTP 400 for malformed Content-Type header - #50109
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses Keycloak’s handling of syntactically malformed Content-Type request headers by ensuring they result in a 400 Bad Request instead of triggering an unhandled exception during response filter processing (which previously produced HTTP 500 and ERROR logs).
Changes:
- Adds a new pre-matching request filter to reject invalid
Content-Typeheader values with HTTP 400. - Adds defensive handling in
DefaultSecurityHeadersProviderto avoidIllegalArgumentExceptionduring request media type access. - Adds an integration test covering malformed
Content-Typebehavior for UserInfo and well-known endpoints.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
tests/base/src/test/java/org/keycloak/tests/oidc/MalformedContentTypeHeaderTest.java |
Adds integration tests asserting malformed Content-Type returns 400 and valid values are not rejected. |
services/src/main/java/org/keycloak/services/filters/InvalidContentTypeHeaderFilter.java |
Introduces a pre-matching request filter that validates Content-Type and rejects invalid values with 400. |
services/src/main/java/org/keycloak/headers/DefaultSecurityHeadersProvider.java |
Wraps request media type retrieval to prevent malformed headers from causing an unhandled exception during header processing. |
|
As of CONTRIBUTING.md, "new contributors may have at most 2 open pull requests at a time". Closing this one due to that. |
|
Hi @ahus1, since my other PR is now merged, could we reopen this one for review? Thanks! |
|
This PR is now in draft mode. It misses a DCO signoff, and there are also some GitHub copilot comments. Please fix and review, rebase, and then mark as ready-for-review. |
6077b60 to
c1c4455
Compare
c1c4455 to
68c2fbf
Compare
68c2fbf to
3bc7258
Compare
3bc7258 to
a3b0fa5
Compare
6129bbe to
a85eb79
Compare
|
Hey @jordandunmire97-ai, could you take a look at this PR when you have some time? I think it's healthy and ready to merge. ): |
|
Hey @jordandunmire97-ai, @ahus1 would you mind taking a look? Is this PR mergeable, or is there anything else needed from my side? |
a85eb79 to
7279f81
Compare
Reject syntactically invalid Content-Type values early in a request filter and add defensive parsing in DefaultSecurityHeadersProvider to prevent IllegalArgumentException from surfacing as HTTP 500. Register the new oidc test package in Base2TestSuite so CI validation passes. Closes keycloak#49785 Signed-off-by: MdTanwer <tanw9004167@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
7279f81 to
8c6b40b
Compare
Summary
Content-Typeheaders with HTTP 400 via a new request filterDefaultSecurityHeadersProviderto preventIllegalArgumentExceptionfrom becoming HTTP 500Closes #49785