Skip to content

CAMEL-24573: CXF REST: UnsupportedOperationException when copying fac… - #25967

Merged
davsclaus merged 1 commit into
apache:mainfrom
mcarlett:CAMEL-24573
Aug 31, 2026
Merged

CAMEL-24573: CXF REST: UnsupportedOperationException when copying fac…#25967
davsclaus merged 1 commit into
apache:mainfrom
mcarlett:CAMEL-24573

Conversation

@mcarlett

Copy link
Copy Markdown
Contributor

…tory bean with fixed-size features list

Description

Target

  • [ x] I checked that the commit is targeting the correct branch (Camel 4 uses the main branch)

Tracking

  • [ x] If this is a large change, bug fix, or code improvement, I checked there is a JIRA issue filed for the change (usually before you start working on it).

Apache Camel coding standards and style

  • [ x] I checked that each commit in the pull request has a meaningful subject line and body.
  • [x ] I have run mvn clean install -DskipTests locally from root folder and I have committed all auto-generated changes.

AI-assisted contributions

  • [ x] If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., Co-authored-by trailers) and the PR description identifies the AI tool used.

…tory bean with fixed-size features list

When creating a CXF JAXRS client factory bean from a Spring bean with a
fixed-size features list (e.g., from Arrays.asList()), the shallow field copy
resulted in a reference to the immutable list. Subsequent attempts to append
endpoint features via addAll() threw UnsupportedOperationException.

Wrap the copied features list in a new ArrayList to ensure mutability while
preserving the original values.

Fixes CSB-10555

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

  • First-time contributors require MANUAL approval for the GitHub Actions to run
  • You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot although they are normally detected and executed by CI.
  • You can label PRs using skip-tests and test-dependents to fine-tune the checks executed by this PR.
  • Build and test logs are available in the summary page. Only Apache Camel committers have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct fix for UnsupportedOperationException when shallowCopyFieldState copies a fixed-size features list. The root cause is that ReflectionUtils.shallowCopyFieldState(bean, cfb) copies the features field via reflection, inheriting the original unmodifiable list reference. When setupCommonFactoryProperties() later calls factory.getFeatures().addAll(...) and factory.getFeatures().add(loggingFeature), both fail on a fixed-size list. Wrapping in new ArrayList<>() is the correct and minimal fix.

Test properly reproduces the issue.

Note: The same shallowCopyFieldState pattern could cause a similar issue with the properties map if someone passes an unmodifiable map (factory.getProperties().putAll(...) at line 393). Not in scope for this PR but worth noting for future hardening.

📋 PR Metadata

Aspect Current Suggested
Labels components + bug
Milestone (none) 4.23.0

🔀 Backport Status

⚠️ This bug fix targets main but no backport PRs were found for:

  • camel-4.22.x — same vulnerable code in newInstanceWithCommonProperties()
  • camel-4.18.x — same vulnerable code

Consider creating backport PRs or adding backport-to-camel-4.22.x / backport-to-camel-4.18.x labels.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

@github-actions

Copy link
Copy Markdown
Contributor

🧪 CI tested the following changed modules:

  • components/camel-cxf/camel-cxf-spring-rest

🔬 Scalpel shadow comparison — Scalpel: 9 tested, 27 compile-only — current: 9 all tested

Maveniverse Scalpel detected 36 affected modules (current approach: 9).

⚠️ Modules only in Scalpel (27)
  • apache-camel
  • camel-allcomponents
  • camel-catalog
  • camel-catalog-console
  • camel-catalog-maven
  • camel-catalog-suggest
  • camel-componentdsl
  • camel-endpointdsl
  • camel-endpointdsl-support
  • camel-itest
  • camel-jbang-core
  • camel-jbang-it
  • camel-jbang-main
  • camel-jbang-plugin-edit
  • camel-jbang-plugin-generate
  • camel-jbang-plugin-kubernetes
  • camel-jbang-plugin-test
  • camel-kamelet-main
  • camel-launcher
  • camel-report-maven-plugin
  • camel-route-parser
  • camel-yaml-dsl
  • camel-yaml-dsl-deserializers
  • camel-yaml-dsl-maven-plugin
  • coverage
  • docs
  • dummy-component

Skip-tests mode would test 9 modules (1 direct + 8 downstream), skip tests for 27 (generated code, meta-modules)

Modules Scalpel would test (9)
  • camel-cxf-spring-rest
  • camel-jbang-mcp
  • camel-jbang-plugin-mcp
  • camel-jbang-plugin-route-parser
  • camel-jbang-plugin-tui
  • camel-jbang-plugin-validate
  • camel-launcher-container
  • camel-yaml-dsl-validator
  • camel-yaml-dsl-validator-maven-plugin
Modules with tests skipped (27)
  • apache-camel
  • camel-allcomponents
  • camel-catalog
  • camel-catalog-console
  • camel-catalog-maven
  • camel-catalog-suggest
  • camel-componentdsl
  • camel-endpointdsl
  • camel-endpointdsl-support
  • camel-itest
  • camel-jbang-core
  • camel-jbang-it
  • camel-jbang-main
  • camel-jbang-plugin-edit
  • camel-jbang-plugin-generate
  • camel-jbang-plugin-kubernetes
  • camel-jbang-plugin-test
  • camel-kamelet-main
  • camel-launcher
  • camel-report-maven-plugin
  • camel-route-parser
  • camel-yaml-dsl
  • camel-yaml-dsl-deserializers
  • camel-yaml-dsl-maven-plugin
  • coverage
  • docs
  • dummy-component

ℹ️ Shadow mode — Scalpel observes but does not affect test execution. Learn more

All tested modules (36 modules)
  • Camel :: All Components Sync point
  • Camel :: Assembly
  • Camel :: CXF :: REST :: Spring
  • Camel :: Catalog :: Camel Catalog
  • Camel :: Catalog :: Camel Report Maven Plugin
  • Camel :: Catalog :: Camel Route Parser
  • Camel :: Catalog :: Console
  • Camel :: Catalog :: Dummy Component
  • Camel :: Catalog :: Maven
  • Camel :: Catalog :: Suggest
  • Camel :: Component DSL
  • Camel :: Coverage
  • Camel :: Docs
  • Camel :: Endpoint DSL
  • Camel :: Endpoint DSL :: Support
  • Camel :: Integration Tests
  • Camel :: JBang :: Core
  • Camel :: JBang :: Integration tests
  • Camel :: JBang :: MCP
  • Camel :: JBang :: Main
  • Camel :: JBang :: Plugin :: Edit
  • Camel :: JBang :: Plugin :: Generate
  • Camel :: JBang :: Plugin :: Kubernetes
  • Camel :: JBang :: Plugin :: MCP
  • Camel :: JBang :: Plugin :: Route Parser
  • Camel :: JBang :: Plugin :: TUI
  • Camel :: JBang :: Plugin :: Testing
  • Camel :: JBang :: Plugin :: Validate
  • Camel :: Kamelet Main
  • Camel :: Launcher
  • Camel :: Launcher :: Container
  • Camel :: YAML DSL
  • Camel :: YAML DSL :: Deserializers
  • Camel :: YAML DSL :: Maven Plugins
  • Camel :: YAML DSL :: Validator
  • Camel :: YAML DSL :: Validator Maven Plugin

⚙️ View full build and test results

@davsclaus davsclaus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, tight fix — thanks for tracking this down. Verified independently: built camel-cxf-spring-rest and ran CxfRsSpringEndpointTest in an isolated worktree, all 4 tests pass including the new testCreateCxfRsClientFactoryBeanWithFixedSizeFeatures.

The root cause traces back cleanly: setupCommonFactoryProperties()'s factory.getFeatures().addAll(getFeatures()) (added in CAMEL-24183) combined with the older shallow-copy of the features field to produce the crash whenever the source bean's list is fixed-size (Arrays.asList()). Rewrapping only the copy (cfb) rather than mutating the original Spring bean's list is the right, minimal call.

As a bonus, this also happens to fix a latent aliasing issue: before this change, cfb.getFeatures() and the endpoint's own getFeatures() were literally the same List instance, so addAll() was silently self-appending to a list shared with the original bean — repeated createJAXRSClientFactoryBean() calls on the same endpoint would have kept growing that shared list. Not something this PR needs to address further, just worth calling out.

One small, non-blocking note: the commit message ends with Fixes CSB-10555 alongside the CAMEL-24573 reference — looks like it might be an internal tracker ID that isn't meaningful to external readers. Not a blocker, just flagging in case it wasn't intentional to include.

No test-coverage gaps, no scope drift, no upgrade-guide or catalog regeneration needed for this change.

This review does not replace specialized tools such as CodeRabbit, Sourcery, or SonarCloud.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

@davsclaus davsclaus added this to the 4.23.0 milestone Aug 31, 2026
@davsclaus davsclaus added the bug Something isn't working label Aug 31, 2026
@davsclaus
davsclaus merged commit 98615a5 into apache:main Aug 31, 2026
5 checks passed
davsclaus added a commit that referenced this pull request Aug 31, 2026
…tory bean with fixed-size features list

Backports #25967 to camel-4.22.x, a straight cherry-pick already reviewed and
merged on main. CxfRsSpringEndpoint.newInstanceWithCommonProperties()
shallow-copies the features field from the source Spring bean; when that
bean's features list is fixed-size (e.g. Arrays.asList()), the copy aliases
the same immutable list, and a later addAll() call in
setupCommonFactoryProperties() throws UnsupportedOperationException. Wraps
the copied list in a new ArrayList so the copy is mutable, without touching
the original Spring bean's list.

Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>

Closes #25969
davsclaus added a commit that referenced this pull request Aug 31, 2026
…tory bean, backport to camel-4.18.x

Backports #25967 to camel-4.18.x. Same fix as #25969: wraps the shallow-copied
features field in a new ArrayList in CxfRsSpringEndpoint.newInstanceWithCommonProperties()
so a fixed-size source list (e.g. Arrays.asList()) doesn't cause
UnsupportedOperationException on a later append.

Not a straight cherry-pick: this branch's camel-cxf-spring-rest test module
depends on camel-test-spring-junit5, not junit6, so the test's import was
kept as-is. More substantially, CxfRsSpringEndpoint.setupJAXRSClientFactoryBean()
on this branch never calls setupCommonFactoryProperties() (that call was
added later by CAMEL-24183, not present here), so the endpoint's own features
are never auto-appended to the copied list. The ported test is adapted to
assert the copied feature is preserved and that the list is mutable by
appending directly, rather than relying on auto-append behaviour this branch
doesn't have.

Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Closes #25970
@mcarlett
mcarlett deleted the CAMEL-24573 branch September 1, 2026 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants