CAMEL-24001: Fix RestBindingAdvice setting Content-Type on body-less responses#24597
Merged
Conversation
…responses Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet
approved these changes
Jul 11, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
Clean, correct bug fix. Two issues addressed:
- Content-Type no longer set on body-less responses — the null-body early return is moved before
ensureHeaderContentType, so 204/no-content responses don't get a spurious Content-Type header. - Multi-value
producesno longer used as Content-Type — the refactoredensureHeaderContentTypecorrectly prefers concrete single media types (application/json/application/xml) over the rawproducesstring when it contains multiple values.
The new FromRestGetContentTypeEmptyBodyTest covers all three scenarios well. No regressions found — binding=off path and response validation are both preserved correctly.
This review evaluates the PR against the project's contribution rules and conventions. It does not replace specialized review tools (CodeRabbit, SonarCloud) or static analysis.
Reviewed with Claude Code on behalf of gnodet. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 549 tested, 29 compile-only — current: 549 all testedMaveniverse Scalpel detected 578 affected modules (current approach: 549).
|
oscerd
approved these changes
Jul 11, 2026
Contributor
Author
|
Summary: PR 24597's test failure is a false negative — the flaky JpaPollingConsumerLockEntityTest in camel-jpa has nothing to do with the REST binding Content-Type fix. Re-running CI should pass (unless the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Claude Code on behalf of davsclaus
Fixes two bugs in
RestBindingAdvice.marshal()related to Content-Type handling:Bug 1 - Content-Type set on body-less responses:
ensureHeaderContentType()was called before the null-body check, so responses with no body (e.g. 204 No Content) got a spurious Content-Type header. Moved the null-body check to before theensureHeaderContentType()call.Bug 2 - Multi-value
producesused as Content-Type:ensureHeaderContentType()set the raw comma-separatedproducesstring (e.g.application/json,text/plain) as Content-Type before theisJson/isXmlfallback branches had a chance to set a proper single media type. Reordered the priority soisJson/isXmlare checked first (settingapplication/jsonorapplication/xml), with the rawproducesstring only used as a last resort.Test plan
FromRestGetContentTypeEmptyBodyTestwith 3 test cases:testNoContentTypeOnEmptyBody- verifies no Content-Type on null-body responsetestContentTypeSetOnNonEmptyBody- verifiesapplication/jsonset on response with bodytestContentTypeJsonWhenMultiValueProduces- verifiesapplication/json(not multi-value) whenproducescontains multiple media typesFromRest*tests pass (35 tests, 0 failures)camel-coretest suite passes (7264 tests, 0 failures)🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com