Skip to content

Fix set(key, value, Expiration, SetOption) ignoring KEEPTTL and unix-timestamp expirations - #7316

Merged
mrniko merged 4 commits into
redisson:masterfrom
stlahxm:fix/set-expiration-keepttl
Aug 24, 2026
Merged

mrniko merged 4 commits into
redisson:masterfrom
stlahxm:fix/set-expiration-keepttl

Conversation

@stlahxm

@stlahxm stlahxm commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Closes #7315

RedissonConnection.set(byte[], byte[], Expiration, SetOption) never checked expiration.isKeepTtl() or expiration.isUnixTimestamp(), so both fell through to the relative-time branch.

  • Expiration.keepTtl() sent its reserved negative sentinel to Redis as a literal PX value, which Redis rejects with ERR invalid expire time in 'set' command.
  • Expiration.unixTimestamp(...) sent its absolute epoch-millis value as a PX delay instead of a PXAT deadline. This does not throw, so it silently produced a TTL decades in the future instead of the intended expiry time.

Scope

Expiration.isKeepTtl()/isUnixTimestamp() were added to Spring Data Redis incrementally, so the fix isn't uniform across every module:

Modules Spring Data Redis Fix applied
20, 21, 22, 23 2.0-2.3 None. Expiration has neither method there, so neither bug can occur
24, 25 2.4-2.5 isKeepTtl() branch only
26, 27, 30, 31, 32, 33, 34, 35, 40, 41 2.6-4.1 Both branches

I compiled all 16 potentially affected modules individually (mvn -DskipTests compile, not a reactor-wide build) to confirm each module only gets the branch its Expiration version actually supports.

Changes

  • RedissonConnection.set(byte[], byte[], Expiration, SetOption): added the missing isKeepTtl() branch (mirroring the already-correct sibling setGet(byte[], byte[], Expiration, SetOption)) and, where supported, the missing isUnixTimestamp() branch (mirroring the already-merged getEx(byte[], Expiration) fix: always sends PXAT with the millisecond value, no separate seconds/EXAT branch, matching that method's style).
  • Added/extended a regression test in six modules spanning the full supported version range (20 unaffected, 24/25 KEEPTTL-only, 26/30/40/41 both) covering the matrix of every Expiration kind times every SetOption, checking TTL correctness for the unixTimestamp cases rather than only checking for an exception. Added two more focused tests confirming the unixTimestamp fix produces the right TTL both when the Expiration was built with a MILLISECONDS value and with a SECONDS value.

Testing

Ran the full matrix against a real Redis instance across modules spanning the supported version range:

Module 20 (Spring Data Redis 2.0.14, unaffected): Tests run: 6, Failures: 0, Errors: 0 (unchanged)
Module 24 (Spring Data Redis 2.4.15, KEEPTTL only): Tests run: 13, Failures: 0, Errors: 0
Module 25 (Spring Data Redis 2.5.12, KEEPTTL only): Tests run: 13, Failures: 0, Errors: 0
Module 26 (Spring Data Redis 2.6.10, both): Tests run: 16, Failures: 0, Errors: 0
Module 30 (Spring Data Redis 3.0.12, both): Tests run: 19, Failures: 0, Errors: 0
Module 40 (Spring Data Redis 4.0.5, both): Tests run: 32, Failures: 0, Errors: 0
Module 41 (Spring Data Redis 4.1.0, both): Tests run: 32, Failures: 0, Errors: 0

Before this fix, the matrix failed on the keepTtl row under all three SetOption values with the exact error from the issue (ERR invalid expire time in 'set' command, PX -2000), and on the unixTimestamp row (in modules where that Expiration kind exists) under all three SetOption values, reporting a ~56-year TTL instead of the intended ~2-minute one. All other combinations already passed and are unaffected by this change.

Also reproduced and re-verified directly against redis-cli (bypassing redisson entirely) to confirm the malformed KEEPTTL command and the fix's KEEPTTL/KEEPTTL NX/KEEPTTL XX/PXAT ... NX/PXAT ... XX syntax are all exactly what Redis expects.

isUnixTimestamp() was never checked either, so Expiration.unixTimestamp(...)
fell into the relative-time branch and sent its absolute epoch-millis value
as a PX delay, silently producing a multi-decade TTL instead of throwing.
Mirrors the maintainer's own getEx() fix style (always PXAT + millis,
no separate EXAT/seconds branch).

Signed-off-by: 심현민 <[email protected]>
…supports them

Spring Data Redis's Expiration class didn't gain isKeepTtl()/isUnixTimestamp()
until 2.4 and 2.6 respectively. Modules 20-23 (SDR 2.0-2.3) have neither
method at all, so the previous patch didn't compile there - reverted, no fix
needed since the feature literally doesn't exist in that Expiration version.
Modules 24-25 (SDR 2.4-2.5) have isKeepTtl() but not isUnixTimestamp() -
kept only the KEEPTTL branch there. Added a matrix test to module 26 (SDR
2.6, the oldest module with both capabilities) to widen version coverage.

Signed-off-by: 심현민 <[email protected]>

@mrniko mrniko left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please apply same changes to RedissonReactiveStringCommands

@mrniko mrniko added this to the 4.8.0 milestone Aug 24, 2026
@mrniko mrniko added the bug label Aug 24, 2026
…nReactiveStringCommands

The reactive sibling of RedissonConnection.set() had the exact same gap:
only expiration.isPersistent() was checked, so Expiration.keepTtl() sent
its reserved negative sentinel as a literal PX value (Redis rejects it),
and Expiration.unixTimestamp(...) sent the absolute epoch value as a
relative PX delay instead of PXAT, silently producing a multi-decade TTL.

Same scope rules as the blocking fix: modules 20-23 unaffected (Expiration
has neither method there), 24-25 get the KEEPTTL branch only, 26-41 get
both branches. Confirmed by individually compiling all 12 affected modules.

Verified RedissonReactiveClusterStringCommands does not override set(),
so it inherits this fix automatically. Also checked RedissonClusterConnection
(blocking) does not override set() either, so no further gap there.

Added a reactive matrix test (all Expiration kinds x all SetOption values,
checking TTL correctness for unixTimestamp rather than only exceptions) to
modules 24, 25, 26, 30, 40, 41, run against a real Redis instance.

Signed-off-by: 심현민 <[email protected]>
@mrniko
mrniko merged commit 2f3527c into redisson:master Aug 24, 2026
4 checks passed
@mrniko

mrniko commented Aug 24, 2026

Copy link
Copy Markdown
Member

Thanks for contribution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

RedisConnection.set(key, value, Expiration, SetOption) ignores KEEPTTL and unix-timestamp expirations

2 participants