allthingssecurity opened a new pull request, #26942: URL: https://github.com/apache/camel/pull/26942
# Description [CAMEL-25059](https://issues.apache.org/jira/browse/CAMEL-25059) Since 4.21 (CAMEL-23691, #23766), `CaseInsensitiveMap` extends `AbstractMap` and overrides `entrySet()` but not `keySet()`. The inherited key set sends `contains` to `containsKey`, which is case-insensitive, but its `remove(o)` is `AbstractCollection.remove`, which compares the keys with `equals`. `AbstractSet.removeAll(c)` calls that `remove` when the set is larger than `c`. The message headers expose this view: the key set of `CopyOnWriteHeadersMap` delegates `remove`, `removeAll` and `retainAll` to it. So ``` headers X-Trace, X-Tenant, Accept exchange.getMessage().getHeaders().keySet().remove("x-trace") -> false, X-Trace stays exchange.getMessage().getHeaders().keySet().removeAll(List.of("x-trace", "x-tenant")) -> removes nothing ``` while `Map.remove("x-trace")`, `removeHeader("x-trace")` and `keySet().contains("x-trace")` work. Up to 4.20 the map was a `TreeMap(String.CASE_INSENSITIVE_ORDER)`, whose key set removes through `TreeMap.remove`, so `keySet().remove("x-trace")` removed `X-Trace`. The camel-4.18.x and 4.14.x branches still use the `TreeMap` and are not affected. This change overrides `keySet()` with a view that uses the lookup of the map: - `contains` goes to `containsKey`, and `remove(o)` finds the key with `findIndex` and removes it with `removeByIndex`, as `EntrySet.remove` already does. - `removeAll(c)` removes each element of `c`, so the result no longer depends on the sizes. - `retainAll(c)` keeps a key when `c` holds it ignoring case (it builds a `CaseInsensitiveMap` of the elements of `c`, so it stays linear). - `clear`, and a key iterator over the entry iterator, whose `remove` works, so `removeIf` works too. The iterator returns the key without creating a `MapEntry`. Compatibility: on 4.20 `removeAll(c)` was size-dependent as well. When `c` has at least as many elements as the map, `AbstractSet.removeAll` asks `c.contains(key)`, which is case-sensitive for a `List`, so `removeAll(List.of("x-trace", "x-tenant", "x-other"))` did not remove `X-Trace` on 4.20 either. `retainAll(c)` always used `c.contains(key)` and was case-sensitive on 4.20 and on main. With this change both are case-insensitive whatever the sizes. That is consistent with the rest of the map rather than identical to 4.20: `retainAll(List.of("accept"))` now keeps `Accept`, where 4.20 removed it. I did not add an upgrade-guide note, because the change restores the 4.20 behaviour of `remove(o)` and only makes the other two consistent with it; I can add one if you prefer. `CopyOnWriteHeadersMap` needs no change. `entrySet().remove(entry)` already finds the key case-insensitively, and `values()` compares values only. Camel's own code does not remove headers through the key set (`git grep -E "keySet\(\)\.(remove|removeAll|retainAll|removeIf)\("` over the non-test code finds, on message headers, only the `CopyOnWriteHeadersMap` delegation), so the effect is on user code, such as a processor that strips some headers this way before calling another system. Tests: - `CaseInsensitiveMapTest.testKeySetRemoveWithDifferentCase`: `keySet().remove("x-trace")` removes `X-Trace` and returns `true`, and returns `false` when there is nothing to remove. - `CaseInsensitiveMapTest.testKeySetRemoveAllWithDifferentCase`: `removeAll` with 2 elements and with 4 elements (more than the map) both leave `[Accept]`, which pins the size independence. - `CaseInsensitiveMapTest.testKeySetRetainAllWithDifferentCase`: `retainAll(List.of("accept", "x-tenant", "x-other"))` keeps `X-TENANT` and `Accept`, a second call returns `false`, and an empty collection clears the map. - `CaseInsensitiveMapTest.testKeySetIteratorRemove`: the key iterator's `remove`, and `removeIf`. - `DefaultMessageHeaderTest.testCopyOnWriteKeySetRemoveWithDifferentCase`: `remove`, `removeAll` and `retainAll` through `getHeaders().keySet()` of a copied message, and the original is unaffected by the copy. Without the change in `CaseInsensitiveMap`, RESULT_NEGATIVE. With it, `SUITE_PATTERN` in camel-util, camel-support, camel-core and camel-console pass: RESULT_SUITE. This touches the same class as #26931 (CAMEL-25051) in a different part (`deduplicateKey` there, the key set here), and the two branches merge without conflicts in either order. Found with a Lean model of the key set, which shows that `keySet().remove(o)` changes nothing whenever no key is exactly `o`, whatever case-insensitive matches exist. I then reproduced it against the real classes and a route, with a `TreeMap(CASE_INSENSITIVE_ORDER)` as the 4.20 control. # 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]
