CAMEL-24184: Fix 19 medium-severity findings from camel-cxf code review#24941
Merged
Conversation
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 20, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
Thorough analysis of 19 medium-severity CXF fixes — each one targeted and well-reasoned. This follows the high-severity fixes already merged in PRs #24929–#24934. Verified each fix against the existing codebase patterns:
Correctness bugs (critical fixes):
- #1
replaceMultiPartContentType: Afterpart.substring(5)stripstype=, the else branch didpart.substring(5)again — double-stripping turnedtext/xml→xml. Fix toresult = partis correct. - #5 & #6 Void.class vs Void.TYPE: Classic Java reflection bug.
Method.getReturnType()returnsvoid.class(i.e.,Void.TYPE), neverVoid.class. The checks never matched, so InOnly MEP was never selected forvoidmethods. Fix correctly checks both. - #11 CxfConverter entity null:
Response.getEntity()returns null for 204/empty responses → NPE in the fallback converter. ReturningMISS_VALUEis correct.
NPE guards (defensive fixes):
- #2 CxfClientCallback:
getOutMessage()can be null during failover — guard added. - #10 setProtocolHeaders: Rewrites the guard to early-return instead of falling through to use
getOutMessage()unconditionally later. - #13 asyncInvokeFromWorkQueue:
instanceof PhaseInterceptorChaininstead of hard cast, plus null guard on fault observer. The LOG.error fallback is appropriate — silent swallowing would be worse.
Resource leaks:
- #7 InOnly Response leak: When
isOutCapable()is false, the Response was never closed — connection stays pinned until GC. Fix correctly closes it. - #12 CamelConduit use-after-stop:
volatile boolean closedflag + check inprepare()prevents use afterclose(). Simple and effective.
Header handling (data-loss fixes):
- #8 Multi-valued Camel→CXF:
toString()on a List produced"[a, b]"literal. Fix preserves List values viaCastUtils.cast. - #9 CXF→Camel truncation:
get(0)drops all values after the first. Fix preserves multi-valued headers as List whensize() > 1— good backward-compatible approach. - #3 Protocol headers on fault path: Headers stored only on the out-message (which doesn't exist yet in the fault path). Fix also stores on the CXF exchange so headers survive into fault handling.
Thread safety and lifecycle:
- #4
busvolatile: Prevents visibility issues in lazy init (thegetBus()method creates a bus if null). - #15-16 enableSpringBusShutdownGracefully: Three improvements: (a) now called from
setApplicationContext()for shared bus case (was only called fromgetBus()for created bus), (b) identity-based listener matching instead of class-name string search, (c) thread only spawned forContextClosedEventinstead of everyApplicationEvent.
Spring integration:
- #17 performInvocation: Propagated to
SpringJAXRSServerFactoryBeanvia override. - #18 setProperties merge:
putAllinstead of replace preservesNullFaultListenerfrom constructor. - #19 SpringCamelContext cast:
instanceofpattern matching prevents ClassCastException in camel-main, with null-safeconfigurerusage downstream.
Code cleanup:
- #14 getBasePath regex: Combined two
replaceFirstcalls (second was unreachable) into a single regex with capture group. Clean.
All fixes follow existing patterns and are well-scoped. This complements the high-severity CXF fixes in #24929–#24934 nicely.
LGTM ✅
Claude Code on behalf of gnodet — AI-generated review
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 22 tested, 29 compile-only — current: 22 all testedMaveniverse Scalpel detected 51 affected modules (current approach: 22).
|
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
2 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
Fixes all 19 medium-severity findings from the July 2026 AI-assisted code review of the
components/camel-cxf/component family (CAMEL-24184).The high-severity findings (CAMEL-24176 through CAMEL-24183) were already fixed in separate PRs. This PR addresses the remaining medium-severity items across 6 CXF sub-modules (15 files changed).
camel-cxf-soap / camel-cxf-common
replaceMultiPartContentTypedouble-stripstype=prefix, corrupting Content-Type for unquoted MTOM/multipartCxfClientCallbackNPE when out message is absent during failoverpopulateCxfHeaderFromCamelExchangeBeforeCheckErrorprotocol headers never reach fault response (dead code)CxfEndpoint.busfield madevolatileto prevent visibility issues in lazy initcamel-cxf-rest
CxfRsInvokerchecksVoid.classinstead ofVoid.TYPEforvoidmethods — InOnly MEP never selectedSubResourceClassInvocationHandlersameVoid.classvsVoid.TYPEconfusion — NPE on void methodscompleted()callbacks leak JAX-RSResponsefor InOnly exchanges (connection pinned until GC)"[a, b]"viatoString()inCxfHeaderHelperget(0)) in 3 locationssetProtocolHeadersNPE when CXF out message is absent (oneway/fault paths)CxfConverterfallback NPE whenResponse.getEntity()returns null (204/empty responses)camel-cxf-transport
CamelConduitproducer leak on abandoned conduit + use-after-stop raceasyncInvokeFromWorkQueueNPE when no fault observer is registered; unsafePhaseInterceptorChaincastgetBasePathregex: secondreplaceFirstunreachable; combined into single patterncamel-cxf-spring-*
enableSpringBusShutdownGracefullywas dead code (never called in standard path), spawned a thread for everyApplicationEvent, and matched listeners unreliably. Fixed: call fromsetApplicationContext, match by identity, only spawn thread forContextClosedEventperformInvocationnot propagated toSpringJAXRSServerFactoryBean— interface-model resources not filtered in Spring variantSpringJAXRSServerFactoryBeanmissingsetPropertiesmerge override —<cxf:properties>wipesNullFaultListenerCxfRsSpringEndpointhard-castsCamelContexttoSpringCamelContext—ClassCastExceptionin camel-mainTest plan
mvn testin camel-cxf-common — passedmvn testin camel-cxf-soap — passedmvn testin camel-cxf-rest — passedmvn testin camel-cxf-transport — passedmvn testin camel-cxf-spring-soap — passedmvn testin camel-cxf-spring-rest — passedClaude Code on behalf of davsclaus
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com