Skip to content

RedissonReactiveStringCommands.getEx() ignores PERSIST/KEEPTTL/EXAT/PXAT and throws on PERSIST/KEEPTTL #7308

Description

@stlahxm

Redis version

redis:8.10 (Docker), also reproduced against a plain local Redis 8.x instance

Redisson version

Reproduced on both master (4.7.1-SNAPSHOT) and the latest published release, redisson-4.7.0

Redisson configuration

singleServerConfig:
  address: "redis://127.0.0.1:6379"

Plain single-server config. No special options are needed to trigger this; it only depends on which Expiration variant is passed to getEx().

What is the Expected behavior?

Calling the reactive getEx() with a GetExCommand built from Expiration.persistent(), Expiration.keepTtl(), or Expiration.unixTimestamp(...) should apply the matching GETEX option (PERSIST, no extra args, or PXAT), the same way the sync RedissonConnection.getEx() already does since commit 6c393a51f.

What is the Actual behavior?

ReactiveStringCommands.getEx() accepts a GetExCommand carrying an Expiration that can express five different things (relative TTL, PERSIST, KEEPTTL, and absolute EXAT/PXAT), but the implementation always sends the raw Expiration.getExpirationTimeInMilliseconds() value as a PX argument, regardless of what was actually requested. For PERSIST/KEEPTTL this isn't just wrong. It throws, because those modes are internally encoded as negative sentinel values.

Root cause

Expiration encodes several distinct meanings into one internal number:

  • a plain relative duration (e.g. 60000 = expire in 60s)
  • PERSIST, as a sentinel value that happens to serialize to -1000 when read via getExpirationTimeInMilliseconds()
  • KEEPTTL, similarly sentinel-encoded (-2000)
  • EXAT/PXAT, an absolute epoch timestamp: a completely different kind of number from a relative delay

The correct approach (used by the sibling method setGet() in the same file, and by the sync counterpart RedissonConnection.getEx(), fixed in commit 6c393a51f) is to check expiration.isPersistent() / isKeepTtl() / isUnixTimestamp() first and translate to the matching Redis option accordingly. getEx() in this reactive file skips that check entirely and always does:

Mono<byte[]> m = write(keyBuf, ByteArrayCodec.INSTANCE, GETEX, keyBuf,
                        "PX", command.getExpiration().getExpirationTimeInMilliseconds());

So PERSIST's sentinel value gets sent as PX -1000 and KEEPTTL's as PX -2000, both rejected by Redis, while an EXAT/PXAT epoch timestamp gets sent as if it were a millisecond delay from now, landing decades in the future instead of at the intended time.

Three concrete scenarios this breaks

  1. A cache value is read and, based on some condition, should become permanent (its TTL removed) in the same round-trip: getEx(GetExCommand.key(...).withExpiration(Expiration.persistent())). This throws ERR invalid expire time in 'getex' command instead of removing the TTL.
  2. A value is refreshed (read again) but its existing TTL should be left untouched: Expiration.keepTtl(). Same error.
  3. A promo code or token should expire at an exact wall-clock time (e.g. midnight UTC) rather than "N seconds from now": Expiration.unixTimestamp(...). This doesn't throw, but silently sets a wildly wrong TTL (in our reproduction, ~56 years instead of ~120 seconds), so the value effectively never expires when the caller expected it to.

Steps to reproduce

ReactiveStringCommands.GetExCommand cmd = ReactiveStringCommands.GetExCommand
        .key(key).withExpiration(Expiration.persistent());
stringCommands.getEx(Mono.just(cmd)).blockFirst();

Actual results against a real Redis instance, one call per Expiration variant:

Expiration.milliseconds(60_000)                -> OK (this is the only variant that happens to work, since it's the one hardcoded case)
Expiration.persistent()                        -> RedisSystem ERR invalid expire time in 'getex' command. params: [key, PX, -1000]
Expiration.keepTtl()                           -> RedisSystem ERR invalid expire time in 'getex' command. params: [key, PX, -2000]
Expiration.unixTimestamp(now+120s, SECONDS)    -> PTTL after call: 1787146328998 ms (~56.6 years) instead of ~120000 ms

Additional information

Scope

Checked all 10 Spring Data Redis module variants under redisson-spring/redisson-spring-data/ (versions 2.6 through 4.1). Every one of them has the identical getEx() implementation, since each new module version was created by copying the previous one.

Confirmed the bug reproduces the same way against both the current master branch and the latest published release, redisson-4.7.0.

History

Traced via git log -S: this exact code was first written in commit 3e4c12cfe (2021-11-22, adding Spring Data Redis 2.6.0 support) and has been copied unchanged into every subsequent module version since. The sync equivalent, RedissonConnection.getEx(), had the identical bug and was fixed 5 days ago in 6c393a51f, across all 10 sync module files, but that fix didn't touch any of the reactive files, so the reactive side is still broken.

Why this hasn't surfaced before

Filing this as a low-discoverability bug rather than a "how did nobody hit this" one. A few conditions have to line up before a caller reaches it:

  • ReactiveValueOperations, the API most callers actually use, only exposes getAndExpire(key, Duration) (relative TTL, the one case that already works) and getAndPersist(key). There's no convenience method for KEEPTTL or EXAT/PXAT at all. Reaching those requires constructing a ReactiveStringCommands.GetExCommand directly with Expiration.keepTtl()/Expiration.unixTimestamp(...), which is a deliberate, low-level call most users never make.
  • The sync-side counterpart bug (RedissonConnection.getEx(), fixed in 6c393a51f) also has no linked issue in its commit message, and a GitHub search for that commit hash turns up no referencing issue/PR, suggesting even that side was found via code review rather than a user report.
  • The module this lives in for the 4.1 variant, redisson-spring-data-41 (Spring Data Redis 4.1 support), was only added on 2026-08-04 (55e8d6609), so real-world adoption of that specific module is still minimal.

Suggested fix

Mirror the sync fix's logic in getEx() for all 10 RedissonReactiveStringCommands.java files:

if (expiration.isPersistent()) {
    m = write(keyBuf, ByteArrayCodec.INSTANCE, GETEX, keyBuf, "PERSIST");
} else if (expiration.isKeepTtl()) {
    m = write(keyBuf, ByteArrayCodec.INSTANCE, GETEX, keyBuf);
} else if (expiration.isUnixTimestamp()) {
    m = write(keyBuf, ByteArrayCodec.INSTANCE, GETEX, keyBuf,
            "PXAT", expiration.getExpirationTimeInMilliseconds());
} else {
    m = write(keyBuf, ByteArrayCodec.INSTANCE, GETEX, keyBuf,
            "PX", expiration.getExpirationTimeInMilliseconds());
}

I've already applied this to all 10 module variants and verified it with a test covering all four Expiration variants (relative/PERSIST/KEEPTTL/EXAT), run against real Redis in every module. Results below. The fix is up in #7309.

Before the fix:

testGetExRelativeMillis                      -> PASS (already worked)
testGetExPersistRemovesTtl                   -> ERROR: RedisSystem ERR invalid expire time in 'getex' command. params: [key, PX, -1000]
testGetExKeepTtlPreservesOriginalTtl         -> ERROR: RedisSystem ERR invalid expire time in 'getex' command. params: [key, PX, -2000]
testGetExAbsoluteSecondsUsesExatSemantics    -> FAIL: expected PTTL ~120000ms, got 1787146328998ms (~56.6 years)

After the fix, all four pass in all 10 modules (Tests run: 4, Failures: 0, Errors: 0, Skipped: 0 each).

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions