allthingssecurity opened a new pull request, #26882: URL: https://github.com/apache/camel/pull/26882
# Description [CAMEL-25017](https://issues.apache.org/jira/browse/CAMEL-25017) `ClaimCheckProcessor` keeps its repository in the internal exchange property `CamelClaimCheckRepository`. The repository is a `DefaultClaimCheckRepository`, a plain `HashMap` plus an `ArrayDeque`. The class javadoc says it is "not shared among Exchanges, but a private instance is created per Exchange". Copying an exchange copied the internal properties by reference, though (`new EnumMap<>(parent.internalProperties)`). So once an exchange had used the Claim Check, every copy made by Split, Multicast, Recipient List, Wire Tap, Enrich, ... used the same repository instance as the parent and as each other: - with parallel processing, parts that `Set`/`Get` the same key overwrote each other's claim checks, and `Push`/`Pop` popped another part's message. A part silently continued with another part's body and headers; - the unsynchronized `HashMap`/`ArrayDeque` were modified concurrently, by parallel parts, or by a Wire Tap copy running next to the original; - sequentially, a part that left a message on the stack between `Push` and `Pop` (it failed, or left the split) changed what the parent popped later. A typical affected route: `claimCheck(Set, "original")`, then `split(...).parallelProcessing()` with `claimCheck(Set, "item")`, a service call and `claimCheck(Get, "item")` in each part. The one `Set` in the parent before the split is enough to make all the parts share a repository. This change gives each copy its own repository, which starts with the same claim checks as the exchange it was copied from: - `DefaultClaimCheckRepository` implements `SafeCopyProperty`. `safeCopy()` copies its map and its stack. The stored exchanges are only read, so they are not copied; - `AbstractExchange.copy()` applies it to the repository, next to where it already copies the message history. `ExchangeHelper.copyExchangeWithProperties`, which the Disruptor consumer uses, does the same next to its message history copy; - `PooledProcessorExchangeFactory` copies exchanges without `Exchange.copy()`, so it does the same. The pooled exchange factory is deprecated in 4.23, but it is still shipped and can still be enabled, so it is included here; - the exchange that `Set`/`Push` stores does not carry a repository. Otherwise, with the change above, an aggregation strategy that returns the stored exchange as the result would replace the exchange's repository with an old copy. `ClaimCheckProcessor` detaches the repository from the exchange while it makes the copy to store, and puts it back in a `finally` block. Copying the exchange with the repository and then removing it from the copy would copy the whole repository on every `Set` or `Push`, so building a large repository would take quadratic time. Why copy the entries instead of giving a copy an empty repository: sub-exchanges use the Claim Check to get data the parent stored. `MulticastMixOriginalMessageBodyAndEnrichedHeadersClaimCheckTest` does this from the `onException` of a multicast part, which restores the parent's original body. With an empty repository per copy, that test fails with `mock://b Body of message: 0. Expected: <Hello World> but was: <Changed body>`. Behaviour changes to be aware of (also in the 4.23 upgrade guide): - Exchange copies (Split, Multicast, Recipient List, Wire Tap, SEDA, Disruptor) get their own copy of the repository. Sharing only happened when the parent had used the Claim Check before the EIP, because the repository is created on first use. - What a part stores, removes, pushes or pops is no longer visible to the parent or to the other parts. Before, that was only visible through the shared (and racy) instance. - After a Split/Multicast with an aggregation strategy, the result exchange's properties are copied back to the parent (`ExchangeHelper.copyResults`), as before. The parent then has the result exchange's repository, the same as for its other properties. - A custom `ClaimCheckRepository` set as the property that does not implement `SafeCopyProperty` is still shared, as before. Tests: new `ClaimCheckEipSplitParallelTest`. The parent does `claimCheck(Set, "original")`, then a parallel split of "A,B" in which each part saves its message (`Set "item"`, or `Push`), replaces the body, and restores it (`Get "item"`, or `Pop`). Latches force the order: part 0 saves, part 1 saves, part 0 restores, part 1 restores. The test also checks that a part can get the parent's claim check. `PooledExchangeClaimCheckEipSplitParallelTest` runs the same tests with pooled exchanges. Without the fix: ``` testSetGetInParallelSplit: java.lang.AssertionError: mock://part Message with body 0:A was expected but not found in [0:B, 1:B] testPushPopInParallelSplit: java.lang.AssertionError: mock://part Message with body 0:A was expected but not found in [0:B, 1:A] ``` With only the `Exchange.copy()` part of the fix, the pooled variant still fails the same way. `ExchangeHelperTest.testCopyExchangeWithPropertiesDoesNotShareClaimCheckRepository` checks that `copyExchangeWithProperties` gives the copy its own repository with the parent's claim checks. Without that part of the fix: ``` org.opentest4j.AssertionFailedError: The copy should get its own claim check repository ==> expected: not same but was: <org.apache.camel.processor.DefaultClaimCheckRepository@89f597c> ``` `ClaimCheckEipStoreCopyTest` counts the `safeCopy()` calls of the repository while a route does `Set`, `Set`, `Push` and `Pop`. When the stored copy is made with the repository and the repository is removed afterwards, it fails with `Set and Push should not copy the claim check repository ==> expected: <0> but was: <3>`. With the fix all pass. `*ClaimCheck*,*Split*,*Multicast*,*Pooled*,*WireTap*,*RecipientList*,*Exchange*Copy*,DefaultExchangeTest,ExchangeHelperTest` in camel-support, camel-base-engine, camel-core-processor and camel-core: 683 tests, 0 failures (6 skipped). I did not run the camel-disruptor tests: its dependencies (the LMAX Disruptor) were not available in my offline build. Found with a TLA+ model of the Claim Check repository shared by split parts, then reproduced against the real classes. In the reproduction, 20 of 20 parallel splits returned another part's message for both Set/Get and Push/Pop; now none do. # 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](https://issues.apache.org/jira/browse/CAMEL) 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. - [ ] I have run `mvn clean install -DskipTests` locally from root folder and I have committed all auto-generated changes. (I built and tested the affected modules, including the formatter and import-sort plugins. I did not run the full root build.) # 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. This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a `Co-Authored-By` trailer. _Claude Code on behalf of allthingssecurity_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
