Repository navigation
Send the response headers HttpServerUtil is given - #52363
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The corrected header behavior needs an automated regression test.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes HttpServerUtil.sendResponse so caller-provided headers reach clients.
Changes:
- Applies response headers before sending the HTTP header block.
- Documents the required ordering.
File summaries
| File | Description |
|---|---|
test-framework/core/src/main/java/org/keycloak/testframework/util/HttpServerUtil.java |
Corrects response-header sequencing. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
sendResponseHeaders transmits the header block, so the headers the caller passed were being added to the response after it had already gone out and never reached the client. The only current caller does not assert on them, which is why this went unnoticed. Closes keycloak#52362 Signed-off-by: lorexoff <rocca.lorenzo04@gmail.com>
5e8190c to
a8ea7ae
Compare
|
Fair point — a behaviour fix with no assertion is one refactor away from coming back. Added Verified it actually bites: with the two lines moved back below and passes with the fix in place. There is a second case for the no-headers, no-body response, so the null-argument path stays covered too. |
Closes #52362
HttpServerUtil.sendResponseadded the caller's headers toexchange.getResponseHeaders()after callingexchange.sendResponseHeaders(...), which has already transmitted the header block — so they never reached the client. This moves the two-line block above that call.The only current caller,
TrustMaterialIdentityProviderTest, passes aContent-Typeand does not assert on it, which is why the helper has been usable in spite of this.Verified with a handler written exactly as the helper writes it, read back with
java.net.http.HttpClient:Found while writing an integration test whose stub endpoint has to return a vendor-specific error header (#52361); that PR carries its own small helper so the two are independent.
Disclosure per CONTRIBUTING: AI agents were used in researching and writing this change.