allthingssecurity opened a new pull request, #28643: URL: https://github.com/apache/camel/pull/28643
# Description [CAMEL-26290](https://issues.apache.org/jira/browse/CAMEL-26290) Three small camel-util defects, found by a formal check of the specifications of these classes: 1. `CaseInsensitiveMap` compares keys with `String.equalsIgnoreCase`, which folds whole code points (JDK 16+), but `caseInsensitiveHash` folded each UTF-16 char on its own, so the two chars of a surrogate pair were folded separately, which changes nothing. Keys that differ only in the case of a supplementary letter, such as U+10400 / U+10428 (Deseret), were equal but hashed differently: `get` and `containsKey` in the other case missed, and `put` of both stored two keys. The `TreeMap(String.CASE_INSENSITIVE_ORDER)` based map before 4.21 treated them as one key. Header names are ASCII in practice, so this is minor. 2. `FilterIterator.hasNext()` looks ahead in the underlying iterator, and `remove()` delegated to it, so `next(); hasNext(); remove()` removed the element after the one `next()` returned (on `[a, b, c]`: `[a, c]` instead of `[b, c]`, and the next `next()` returned the removed `b`), and `hasNext(); remove()` before any `next()` removed an element instead of throwing. 3. `TimeUtils.isPositive(Duration)` was `getSeconds() > 0 || getNano() != 0`; the nanos part of a negative `Duration` is not negative (`Duration.ofNanos(-1)` is -1 s + 999999999 ns), so it returned true for negative durations with a nanos part. Changes: - `CaseInsensitiveMap.caseInsensitiveHash` folds a surrogate pair as one code point, with the same `toLowerCase(toUpperCase(..))` fold. The ASCII fast path and the hash of BMP keys are unchanged, with no allocation, so lookups, known-key deduplication and iteration order are the same for those keys. - `FilterIterator.remove()` throws `IllegalStateException` unless it directly follows `next()`: before `next()`, twice in a row, or after a `hasNext()` that looked ahead (the `Iterator` contract allows this; removing the right element after a look-ahead is not possible through the underlying iterator). Directly after `next()` it removes the returned element as before. Javadoc added. - `TimeUtils.isPositive` returns `!dur.isNegative() && !dur.isZero()`. - Camel's own code calls neither `FilterIterator` nor `TimeUtils.isPositive` (git grep on main), so the changes 2 and 3 only affect direct users of these public utilities. - The 4.24 upgrade guide gets an entry for the `FilterIterator.remove` change (a new `IllegalStateException`). No option or generated file change. - Backport: item 1 is in camel-4.22.x (since 4.21); items 2 and 3 are the same in camel-4.22.x, 4.18.x and 4.14.x (read, not run). Tests (camel-util, JUnit 5): - New `CaseInsensitiveMapSupplementaryCaseTest`: `supplementaryLetterInTheOtherCaseIsTheSameKey` (get, containsKey, put and remove in the other case) and `hashIsTheSameForEveryCodePointAndItsCaseVariants` (every code point against its upper, lower, title and folded forms; all 260 supplementary case pairs differ on main) fail on main. `asciiAndBmpKeysKeepTheirBehaviour` (ASCII, Turkish dotless i, Kelvin sign, Latin-1 and a lone surrogate key, and the key order) passes on main and with the change; it is a control. - New `FilterIteratorRemoveTest`: `removeAfterHasNextFailsAndRemovesNothing` and `removeBeforeNextFails` fail on main; `removeTwiceFails`, `removeDirectlyAfterNextRemovesTheReturnedElement` and `removeAfterNextThatSkippedFilteredElements` are controls. - `TimeUtilsTest`: `isPositiveIsFalseForNegativeDurationsWithANanosPart` fails on main; `isPositiveForZeroPositiveAndWholeNegativeDurations` is a control. - camel-util: 307 tests, 0 failures. The formatter and import-sort plugins ran. - Also checked outside the tests: 2 million random keys built from case pairs, the Turkish i forms, the Kelvin sign and supplementary letters; for every well-formed pair the map agrees with `equalsIgnoreCase` (about 140,000 disagreements on main). For malformed keys with unpaired surrogates `equalsIgnoreCase` is not an equivalence (it lines up the surrogates of the two strings differently), so no hash can follow it there; a few such pairs still disagree, as on main. How it was found: a Rocq formalisation of the specifications of these classes. The `CaseInsensitiveMap` spec "a map keyed by `String.equalsIgnoreCase`" could not be proved: `cih_consistent_bmp` proves that keys without surrogates that are equal ignoring case have the same hash, and `cih_inconsistent_supplementary` proves the U+10400 / U+10428 counterexample. For the two others the compiled classes were run in a JVM model in Rocq: `filter_remove_wrong_element` and `filter_remove_before_next` (`FilterIterator`), `negative_nanosecond_is_reported_positive` and `duration_controls` (`TimeUtils`). All three were then reproduced on HotSpot. Not changed: `FilterIterator` still cannot remove after a look-ahead; that would need a different design (a remove that does not go through the underlying iterator), and nothing in Camel needs it. # 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-26290) filed for the change (usually before you start working on it): [CAMEL-26290](https://issues.apache.org/jira/browse/CAMEL-26290). # 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 camel-util and ran its tests, including the formatter and import-sort plugins. No generated file changes. 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]
