allthingssecurity opened a new pull request, #29534: URL: https://github.com/apache/camel/pull/29534
# Description [CAMEL-26753](https://issues.apache.org/jira/browse/CAMEL-26753) Found while fixing CAMEL-26737. `MemoryKeyValueRepository`, `JdbcKeyValueRepository`, `JpaKeyValueRepository`, `EhcacheKeyValueRepository` and `JCacheKeyValueRepository` computed the expiry time of an entry as `System.currentTimeMillis() + ttl.toMillis()` without an overflow check. For a TTL near `Long.MAX_VALUE` milliseconds the sum wrapped around to a negative time: - the in-memory, Ehcache and JCache repositories treated the entry as expired at once: `get` returned `null` right after `put`, and `putIfAbsent` overwrote it; - the JDBC and JPA repositories, where an expiry time of 0 means no expiry, returned the value from `get`, but left the entry out of `keys()` and `size()`. A TTL above `Long.MAX_VALUE` milliseconds, such as `ChronoUnit.FOREVER.getDuration()`, made `Duration.toMillis()` throw an `ArithmeticException` from `put`, `putIfAbsent` and `replace`. `CaffeineKeyValueRepository` calls `ttl.toNanos()`, which throws already for a TTL of about 292 years. Users get there by writing `Long.MAX_VALUE` to mean "forever": the state store `ttl` option and `CamelStateStoreTtl` header, and the Cache EIP `ttl`, are milliseconds passed as `Duration.ofMillis(ttl)`. With the in-memory repository such an entry is gone at once. Change: - New `KeyValueRepositoryHelper.expiresAt(Duration)` adds a positive TTL to the current time with `Math.addExact`, and returns `Long.MAX_VALUE` when the sum or `toMillis()` overflows, so the entry does not expire. The five repositories call it in place of the unchecked sum. - `CaffeineKeyValueRepository` uses `Long.MAX_VALUE` nanoseconds, its value for no expiry, when `toNanos()` overflows. - A `null`, zero or negative TTL still means no expiry, as the `KeyValueRepository` Javadoc says. Only a TTL that overflowed before behaves differently, so there is no upgrade guide entry. Not in this PR: - `KafkaKeyValueRepository.toExpiresAt` has the same sum, but there the wrap is not visible: its `isExpired` treats a negative time as no expiry, in `get` and in `keys()` alike. Only a TTL above `Long.MAX_VALUE` milliseconds throws. A pending change of mine to that class (#29434) adds its first unit test class, so I will make the same one-line change there, with a test, once that one is in. - The Hazelcast, Infinispan and Redis repositories pass the TTL to the store as it is. I have not checked what each store does with a TTL near `Long.MAX_VALUE` milliseconds; I will file a follow-up JIRA for that. Tests (no Docker; JDBC and JPA on embedded H2, JCache on the module's embedded Hazelcast member): - In the test classes of the six repositories, `testTtlTooLargeToAddToTheCurrentTimeDoesNotExpire` stores entries with `Duration.ofMillis(Long.MAX_VALUE)` through `put`, `putIfAbsent` and `replace` (of an entry stored with that TTL), and checks `get`, `contains`, a second `putIfAbsent`, `keys()` and `size()`. `testTtlLongerThanLongMaxValueMillisDoesNotExpire` does `put` with `ChronoUnit.FOREVER.getDuration()`. - `KeyValueRepositoryHelperTest`: `expiresAt` adds a normal TTL to the current time, and returns `Long.MAX_VALUE` for both overflows. - No TTL: `testNoTtlWithZero` of the Caffeine, Ehcache and JCache tests passed `null`; it now passes `Duration.ZERO`. `JpaKeyValueRepositoryTest` gets `testNoTtlWithZero` and `testNoTtlWithNegative`. The existing tests for a short TTL that expires are unchanged. On main (the six repository classes reverted, helper kept) all 12 new repository tests fail: the `FOREVER` tests with `ArithmeticException: long overflow` (JPA wraps it in a `PersistenceException`); `testTtlTooLargeToAddToTheCurrentTimeDoesNotExpire` at `replace` returning `false` for Memory, Ehcache and JCache, at `keys()` returning an empty set for JDBC and JPA, and with `ArithmeticException` for Caffeine. With the fix all pass. The seven changed test classes passed 20 of 20 runs each (one JVM per class). Module tests, offline: camel-sql 344 (`SqlFunctionDataSourceTest` left out: it installs an embedded MariaDB and cannot run offline here), camel-jpa 137 (1 skipped: `JpaRouteSkipLockedEntityTest`, which is `@Disabled`), camel-ehcache 91, camel-jcache 103, camel-caffeine 88, and 106 key-value tests of camel-support and camel-core, 0 failures. # 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): [CAMEL-26753](https://issues.apache.org/jira/browse/CAMEL-26753). # 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 the core modules and the five components with the formatter and import-sort plugins and ran their tests. 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]
