davsclaus opened a new pull request, #26852:
URL: https://github.com/apache/camel/pull/26852

   _Claude Code on behalf of Claus Ibsen (davsclaus)_
   
   JIRA: https://issues.apache.org/jira/browse/CAMEL-24996
   
   A deep review of the Exchange and Message core found the bug below. Each 
test here fails without the fix.
   
   **Changing a header value through the entry set of a copied message changed 
the original message.** Since CAMEL-23681 (Camel 4.21), a copied message shares 
its headers with the original through `CopyOnWriteHeadersMap` until one of them 
writes to them. Writes through the map, its key set and its values all copy the 
map first, but the entry set of a shared map returned the entries of the shared 
map itself, from both its iterator and `toArray()`. So `entry.setValue(...)` 
changed the shared map without copying it first.
   
   For example, in a split each sub-message starts out sharing the parent 
message's headers. A sub-message that changed a header value through 
`getHeaders().entrySet()` changed it for the parent and for the other 
sub-messages. After the split, the parent had the value set by the last 
sub-message (`SplitterHeaderEntrySetValueTest` shows this). The same applies to 
every copied exchange: multicast, recipient list, wire tap, and the error 
handler's defensive copy.
   
   The entries of a shared map are now wrapped, so `setValue` copies the map 
first, the same way the other write operations already do. Only an iterator 
created while the map is still shared wraps its entries. The map an iterator 
walks never changes, so once a message has its own map, its entries are 
returned as before, with no extra allocation. This matters because header 
filter strategies iterate the entry set on every send.
   
   **Affected versions:** 4.21.0 and 4.22.0 (CAMEL-23681). The committers can 
decide on `port/*` labels.
   
   ### Reviewed without changes
   - **Removing entries while iterating a copied map** (`keySet()`, `values()` 
and `entrySet()` iterators, including `removeIf` with several matches): checked 
with tests, and it works both on a shared map and after it has been copied. I 
did not add those tests, to keep this PR focused.
   - **`AbstractExchange`'s copy constructor** does not copy `failureHandled`, 
`transacted`, the redelivery counters, `interrupted` or the message history 
node fields. The EIPs that need `transacted` set it on their copies explicitly, 
and `ExchangeHelper.copyResults` copies the redelivery counters back. I found 
no path where the missing fields cause a problem.
   - **Dead code:** `ExtendedExchangeExtension` looks unused since its state 
was inlined into `AbstractExchange` (CAMEL-20015). It is a public class in 
camel-support, so removing it would need a deprecation first.
   
   ### Tests
   - New tests in `DefaultMessageHeaderTest` (`testCopyOnWriteEntrySetValue`, 
`testCopyOnWriteEntrySetToArraySetValue`) and the new route-level 
`SplitterHeaderEntrySetValueTest`. All three fail without the fix.
   - The full `core/camel-support` suite passes (125 tests), and the full 
`core/camel-core` suite passes: 7570 tests, 0 failures, 44 skipped. 
`StopRouteAbortAfterTimeoutTest` was flaky and passed on rerun; it is 
timing-based.
   
   🤖 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]

Reply via email to