[
https://issues.apache.org/jira/browse/CAMEL-26290?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Work on CAMEL-26290 started by shashank.
----------------------------------------
> camel-util - CaseInsensitiveMap treats keys that differ in the case of a
> supplementary letter as different keys, FilterIterator.remove after hasNext
> removes the wrong element, TimeUtils.isPositive is true for negative
> durations with a nanos part
> -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: CAMEL-26290
> URL: https://issues.apache.org/jira/browse/CAMEL-26290
> Project: Camel
> Issue Type: Bug
> Components: camel-core
> Reporter: shashank
> Assignee: shashank
> Priority: Minor
>
> Three small defects in camel-util, found by a formal check of the
> specifications of these classes. One PR fixes all three.
> *1. CaseInsensitiveMap: keys that differ only in the case of a supplementary
> letter are different keys*
> {{CaseInsensitiveMap}} (the message headers map since 4.21, CAMEL-23691)
> compares keys with {{String.equalsIgnoreCase}}, which folds whole code points
> (JDK 16+), but {{caseInsensitiveHash}} folds each UTF-16 char on its own
> ({{Character.toLowerCase(Character.toUpperCase(c))}}). For a letter outside
> the BMP the two chars of the surrogate pair are folded separately, which
> changes nothing. So two keys that are equal ignoring case get different
> hashes: U+10400 (DESERET CAPITAL LETTER LONG I) and U+10428 (DESERET SMALL
> LETTER LONG I) are {{equalsIgnoreCase}}, but {{get}} and {{containsKey}} with
> the other case miss, and {{put}} of both stores two keys. The earlier
> {{TreeMap(String.CASE_INSENSITIVE_ORDER)}} based map treated them as one key.
> Scripts with such case pairs include Deseret, Osage, Old Hungarian, Warang
> Citi, Medefaidrin and Adlam (260 case pairs of code points on JDK 21). Header
> names are ASCII in practice, hence Minor.
> *Fix*: the hash folds a surrogate pair as one code point, with the same
> two-step fold. ASCII keys keep the fast path, and BMP keys hash as before, so
> nothing else changes (no allocation, same iteration order). For well-formed
> strings the hash is now consistent with {{equalsIgnoreCase}}: a test checks
> every code point against its case variants, and 2 million random keys from
> case pairs, the Turkish i forms, the Kelvin sign and supplementary letters
> agree with {{equalsIgnoreCase}}. Only for malformed keys with unpaired
> surrogates, where {{equalsIgnoreCase}} itself is not an equivalence (it lines
> up the surrogates of the two strings differently), can a lookup still differ,
> as on main.
> *2. FilterIterator.remove after hasNext removes the wrong element*
> {{FilterIterator.hasNext()}} looks ahead: it takes the next matching element
> from the underlying iterator. {{remove()}} delegated to the underlying
> iterator, so after a look-ahead it removed the looked-at element instead of
> the one {{next()}} returned. On an {{ArrayList}} [a, b, c]: {{next()}}
> returns a, {{hasNext()}}, {{remove()}} removes b (leaves [a, c]), and the
> next {{next()}} returns the removed b. {{hasNext(); remove()}} before any
> {{next()}} removed a instead of throwing {{IllegalStateException}} as the
> {{Iterator}} contract requires.
> *Fix*: {{remove()}} throws {{IllegalStateException}} unless it directly
> follows {{next()}} (not before {{next()}}, not twice, not after a
> {{hasNext()}} that looked ahead); otherwise it removes the returned element
> as before. Camel's own code does not use {{FilterIterator}}.
> *3. TimeUtils.isPositive is true for negative durations with a nanos part*
> {{TimeUtils.isPositive(Duration)}} returns {{dur.getSeconds() > 0 ||
> dur.getNano() != 0}}. A {{Duration}} stores normalized seconds and a nanos
> part that is never negative, so {{Duration.ofNanos(-1)}} is -1 seconds +
> 999999999 nanos, and {{isPositive}} returns true (also for -999999999 ns,
> -1000000001 ns or -1500 ms).
> *Fix*: {{!dur.isNegative() && !dur.isZero()}}. Camel's own code does not call
> {{TimeUtils.isPositive}}.
> *Tests*: new {{CaseInsensitiveMapSupplementaryCaseTest}} and
> {{FilterIteratorRemoveTest}}, two new methods in {{TimeUtilsTest}}, all in
> camel-util. 5 of them fail on main:
> {{supplementaryLetterInTheOtherCaseIsTheSameKey}},
> {{hashIsTheSameForEveryCodePointAndItsCaseVariants}} (all 260 supplementary
> case pairs), {{removeAfterHasNextFailsAndRemovesNothing}},
> {{removeBeforeNextFails}} and
> {{isPositiveIsFalseForNegativeDurationsWithANanosPart}}. The others (ASCII,
> BMP, Turkish dotless i, Kelvin sign and lone surrogate keys; remove twice,
> remove after next, remove after skipped elements; zero, positive and whole
> negative durations) pass on main and with the fix; they are controls.
> camel-util: 307 tests, 0 failures.
> *How it was found*: a Rocq formalisation of the specifications of these
> classes. For {{CaseInsensitiveMap}} the spec "a map keyed by
> {{String.equalsIgnoreCase}}" could not be proved: {{cih_consistent_bmp}}
> proves that equal-ignoring-case keys without surrogates 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}} for {{FilterIterator}},
> {{negative_nanosecond_is_reported_positive}} and {{duration_controls}} for
> {{TimeUtils}}. All were then reproduced on a HotSpot JVM.
> *Affected*: item 1: main and camel-4.22.x (since 4.21). Items 2 and 3: main,
> camel-4.22.x, camel-4.18.x and camel-4.14.x (same code; read, not run).
> _Filed with Claude Code on behalf of allthingssecurity._
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
