CAMEL-24409: keep binary bodies intact in rest client request validation - #26024
Merged
Conversation
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 562 tested, 26 compile-only — current: 561 all testedMaveniverse Scalpel detected 588 affected modules (current approach: 561).
|
davsclaus
approved these changes
Sep 2, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Review
I built core/camel-support and components/camel-jetty from this branch and ran the tests directly rather than only reading the diff:
- With the fix: all 4 tests in
RestJettyRequiredBodyTestpass, including the newtestJettyBinaryBodyNotCorrupted. - With the fix reverted (restoring the
setBody(body)write-back), the new test fails as expected (array lengths differ, expected: <8> but was: <16>), confirming the test genuinely catches the described regression.
I also verified:
MessageHelper.extractBodyAsStringalready converts the body to aStreamCacheand resets it, so dropping the write-back does not lose re-readability — matches the PR description.- The only other
RestClientRequestValidatorimplementation (OpenApiRestClientRequestValidator) never had this write-back pattern, so no similar fix is needed there. RestBindingAdviceperforms its own separateextractBodyAsString/setBody(with a properDataType) for JSON/XML binding paths further downstream, so this change only affects non-JSON/XML (e.g. binary) bodies as intended, with no regression risk to JSON/XML binding.
Commit message follows the CAMEL-XXXX: description convention with proper AI co-authorship attribution, and the upgrade guide entry is accurate and correctly scoped to 4.23.
No issues found.
About the CI failure
Both failing checks (build (17, false), build (25, false)) are unrelated to this PR. They trace back to a single root cause: camel-jms timed out in its Surefire fork (There was a timeout in the fork) after ~44 minutes wall-clock in the JDK 17 job. The JDK 25 job was cancelled as a side effect of that matrix leg failing (The operation was canceled), not an independent failure. camel-jms has nothing to do with the files this PR touches (camel-jetty, core/camel-support, docs). This looks like CI flakiness/infra timeout — recommend re-running the checks.
This review does not replace specialized AI review tools (CodeRabbit, Sourcery) or static analysis (SonarCloud).
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Checking that a required body is present read the body as a String and wrote that String back onto the message, which corrupted binary payloads such as application/octet-stream. extractBodyAsString already leaves the body as a re-readable StreamCache, so the write back was not needed to keep the body readable, only to lose the bytes. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
davsclaus
force-pushed
the
fix/CAMEL-24409
branch
from
September 2, 2026 05:59
fc7c227 to
eb11558
Compare
Croway
approved these changes
Sep 2, 2026
gnodet
approved these changes
Sep 2, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
Clean, well-targeted bugfix. The root cause analysis is sound — MessageHelper.extractBodyAsString() already replaces stream-based bodies with a StreamCache and resets it after reading. The removed exchange.getIn().setBody(body) call was overwriting the re-readable StreamCache with a lossy String, corrupting bytes that aren't valid UTF-8 (replaced with U+FFFD).
The fix is minimal and correct: removing the write-back preserves the original binary payload while keeping the required-body check intact. The JSON validation path is unaffected since it calls extractBodyAsString again, which re-reads from the StreamCache.
The test is well-designed with BINARY_BODY including bytes 0x00, 0xFF, 0xFE, and the invalid UTF-8 sequence 0xC3 0x28. The upgrade guide entry correctly documents the behavioral change for anyone who relied on the body being converted to a String.
📋 PR Metadata
| Aspect | Current | Suggested |
|---|---|---|
| Labels | components, core, docs |
+ bug |
| Milestone | (none) | 4.23.0 |
🔀 Backport Status
main but no backport PRs were found. The bug originates from commit 6872780 (CAMEL-20832, June 2024) and exists on camel-4.18.x and camel-4.22.x. Consider creating backport PRs.
🤖 This review was generated by Claude Code on behalf of Guillaume Nodet
This was referenced Sep 3, 2026
davsclaus
added a commit
that referenced
this pull request
Sep 3, 2026
…o 4.22 and 4.18 guides (#26072) The binary-body fix (PR #26024) is backported to camel-4.22.x (PR #26070) and camel-4.18.x (PR #26071). Per the backport upgrade-guide policy, main's version-specific guides are the canonical history, so add the matching note to the 4.22 and 4.18 upgrade guides on main. Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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
4 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.
Description
With
clientRequestValidationenabled and a REST service that declares a required body, binarypayloads came through corrupted. Posting
application/octet-streamdata ended up with large partsof the content replaced by
EF BF BD, the Unicode replacement character.DefaultRestClientRequestValidatorreads the body as a String to check that it is present, and thenwrote that String back onto the message:
The write back is what destroys the payload. Bytes that are not valid UTF-8 do not survive the trip
through a String, so the route downstream receives replacement characters instead of the original
data. The write back is also not needed to keep the body readable:
MessageHelper.extractBodyAsStringalready converts the body to a
StreamCache, sets that on the message and resets it, so the bodystays re-readable on its own. Removing the two lines keeps the required body check and leaves the
payload alone.
This was raised on Zulip and James Netherton pointed at these lines as the likely cause:
https://camel.zulipchat.com/#narrow/channel/257302-camel-quarkus/topic/Binary.20data.20issue.20using.20Rest.20DSL/with/617455026
RestJettyRequiredBodyTest#testJettyBinaryBodyNotCorruptedposts a small byte array that is notvalid UTF-8 to an
application/octet-streamservice with a required body, and asserts the bytescome back unchanged. With the fix reverted the test fails, since the payload comes back with
replacement characters.
Also added a short note to the 4.23 upgrade guide, because the message body type after validation
changes for anyone who relied on it being a String.
Target
mainbranch)Tracking
CAMEL-24409
Apache Camel coding standards and style
I checked that each commit in the pull request has a meaningful subject line and body.
I have run
mvn clean install -DskipTestslocally from root folder and I have committed all auto-generated changes.AI-assisted contributions
Co-authored-bytrailers) and the PR description identifies the AI tool used.Claude Code on behalf of chala2001