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

Redis version

7.4 (also reproduced on 8.x)

Redisson version

4.7.1-SNAPSHOT (current master)

Redisson configuration

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

Not configuration-specific. It reproduces with any redisson-spring-data-* module whose Spring Data Redis version supports the relevant Expiration factory method (see Scope below).

What is the Expected behavior?

redisTemplate.opsForValue().set(key, "v2".getBytes(), Expiration.keepTtl());

should update the value while leaving the key's existing TTL untouched, per Redis's own SET ... KEEPTTL semantics.

redisTemplate.opsForValue().set(key, "v2".getBytes(), Expiration.unixTimestamp(epochMillis, TimeUnit.MILLISECONDS));

should expire the key at that exact wall-clock time, per Redis's SET ... PXAT semantics.

What is the Actual behavior?

RedissonConnection.set(byte[] key, byte[] value, Expiration expiration, SetOption option) only branches on expiration.isPersistent(). Everything else falls through to a single relative PX <millis> argument. Neither isKeepTtl() nor isUnixTimestamp() is ever checked. That one gap produces two different, independent failure modes, depending on which Expiration was passed.

1. Expiration.keepTtl() throws. Expiration encodes KEEPTTL internally as a reserved negative sentinel from getExpirationTimeInMilliseconds() (-2000). That sentinel gets sent to Redis as a literal PX value, and Redis rejects it:

org.redisson.client.RedisException: ERR invalid expire time in 'set' command.
command: (SET), params: [key, v2, PX, -2000]

I confirmed this is exactly what's happening by sending the same malformed command directly via redis-cli, with no redisson or test framework involved:

$ redis-cli SET mykey value2 PX -2000
ERR invalid expire time in 'set' command

2. Expiration.unixTimestamp(...) does not throw, but silently sets the wrong TTL. Its absolute epoch-millis value gets sent as if it were a relative PX delay, so instead of expiring at the intended wall-clock time, the key gets a TTL landing decades in the future:

long targetEpochMillis = System.currentTimeMillis() + 120_000; // 2 minutes from now
connection.set(key, "v2".getBytes(), Expiration.unixTimestamp(targetEpochMillis, TimeUnit.MILLISECONDS), SetOption.upsert());
// pTtl(key) afterward: ~1787476157929 ms (~56.6 years), not ~120000 ms

This is the more dangerous of the two: no exception, no log line, the call reports success, and the key effectively never expires.

Two sibling methods in the same class already handle these correctly: setGet(byte[], byte[], Expiration, SetOption) handles isKeepTtl(), and the already-fixed getEx(byte[], Expiration) handles both isKeepTtl() and isUnixTimestamp(). Neither branch was ever copied into set().

Scope

Expiration gained these factory methods incrementally across Spring Data Redis versions, so the two bugs don't apply uniformly to every module. I checked each module's actual Expiration.class (not just assumed it), and confirmed by compiling every affected module individually:

So the KEEPTTL bug spans 12 modules and the unix-timestamp bug spans 10. I confirmed the affected modules all had identical set(Expiration, SetOption) source before the fix.

Why this matters

KEEPTTL exists precisely for the "update the value, don't touch the expiry" pattern: a session payload refresh, a cache value re-computed in place, a counter snapshot rewritten mid-window. A concrete example:

// A session's data changes, but its 30-minute sliding expiry was already set
// earlier and should be left alone. That's the whole point of using KEEPTTL
// here instead of re-specifying the duration.
redisTemplate.opsForValue().set(sessionKey, updatedSessionBytes, Expiration.keepTtl());

Today this throws on every call, for every caller, on every module where Expiration.keepTtl() exists at all (2.4 through 4.1). There's no workaround inside the Expiration-based API. The caller has to abandon Expiration.keepTtl() entirely. The alternatives are reading the current TTL first and re-supplying it explicitly (an extra round trip, and a race if something else changes the TTL in between), or switching to a different RedisConnection method altogether.

The unixTimestamp case is worse in practice because it fails silently. A caller expiring a token, promo code, or scheduled job marker at an exact wall-clock time gets a key that reports success and simply never expires when they thought it would.

Suggested fix

     public Boolean set(byte[] key, byte[] value, Expiration expiration, SetOption option) {
         if (expiration == null) {
             return set(key, value);
         } else if (expiration.isPersistent()) {
             if (option == null || option == SetOption.UPSERT) {
                 return set(key, value);
             }
             if (option == SetOption.SET_IF_ABSENT) {
                 return write(key, StringCodec.INSTANCE, SET, key, value, "NX");
             }
             if (option == SetOption.SET_IF_PRESENT) {
                 return write(key, StringCodec.INSTANCE, SET, key, value, "XX");
             }
+        } else if (expiration.isKeepTtl()) {
+            if (option == null || option == SetOption.UPSERT) {
+                return write(key, StringCodec.INSTANCE, SET, key, value, "KEEPTTL");
+            }
+            if (option == SetOption.SET_IF_ABSENT) {
+                return write(key, StringCodec.INSTANCE, SET, key, value, "KEEPTTL", "NX");
+            }
+            if (option == SetOption.SET_IF_PRESENT) {
+                return write(key, StringCodec.INSTANCE, SET, key, value, "KEEPTTL", "XX");
+            }
+        } else if (expiration.isUnixTimestamp()) {
+            if (option == null || option == SetOption.UPSERT) {
+                return write(key, StringCodec.INSTANCE, SET, key, value, "PXAT", expiration.getExpirationTimeInMilliseconds());
+            }
+            if (option == SetOption.SET_IF_ABSENT) {
+                return write(key, StringCodec.INSTANCE, SET, key, value, "PXAT", expiration.getExpirationTimeInMilliseconds(), "NX");
+            }
+            if (option == SetOption.SET_IF_PRESENT) {
+                return write(key, StringCodec.INSTANCE, SET, key, value, "PXAT", expiration.getExpirationTimeInMilliseconds(), "XX");
+            }
         } else {
             if (option == null || option == SetOption.UPSERT) {
                 return write(key, StringCodec.INSTANCE, SET, key, value, "PX", expiration.getExpirationTimeInMilliseconds());
             }
             ...

The isKeepTtl() branch mirrors the three-line shape setGet() already uses. The isUnixTimestamp() branch always sends PXAT with the millisecond value, matching the already-merged getEx() fix's style exactly. That method deliberately doesn't branch on seconds vs. milliseconds, since getExpirationTimeInMilliseconds() normalizes regardless of which unit the Expiration was originally built with. I verified this holds for a SECONDS-unit unixTimestamp(...) too. The isUnixTimestamp() branch is applied only to the 10 modules where that method exists on Expiration; modules 24-25 only get the isKeepTtl() branch.

I verified KEEPTTL NX/KEEPTTL XX and PXAT ... NX/PXAT ... XX are valid Redis syntax (checked directly against redis-cli); Redis accepts the option flags in any order.

Test code

The test I used to verify the fix, added to several modules (shown here as run against module 41; the same shape was added to 20, 24, 25, 26, 30, 40, adjusted for which Expiration kinds each module's version actually supports):

@Test
public void testSetExpirationOptionMatrix() {
    List<String> failures = new ArrayList<>();
    List<String> passes = new ArrayList<>();

    Expiration[] expirations = new Expiration[] {
        Expiration.milliseconds(60000),
        Expiration.persistent(),
        Expiration.keepTtl(),
        Expiration.unixTimestamp(System.currentTimeMillis() + 60000, TimeUnit.MILLISECONDS)
    };
    String[] expirationNames = { "relative(60s)", "persistent", "keepTtl", "unixTimestamp" };

    SetOption[] options = { SetOption.upsert(), SetOption.ifAbsent(), SetOption.ifPresent() };
    String[] optionNames = { "UPSERT", "IF_ABSENT", "IF_PRESENT" };

    for (int e = 0; e < expirations.length; e++) {
        for (int o = 0; o < options.length; o++) {
            byte[] key = ("matrix-" + e + "-" + o).getBytes();
            if (optionNames[o].equals("IF_ABSENT")) {
                connection.del(key); // NX only applies when the key is absent
            } else {
                connection.set(key, "seed".getBytes());
                if (optionNames[o].equals("IF_PRESENT") || expirationNames[e].equals("keepTtl")) {
                    connection.expire(key, 100);
                }
            }
            String label = expirationNames[e] + " + " + optionNames[o];
            try {
                connection.set(key, "updated".getBytes(), expirations[e], options[o]);

                // Absence of an exception is not enough on its own. unixTimestamp
                // silently set a ~56-year TTL instead of throwing before this was fixed.
                if (expirationNames[e].equals("unixTimestamp")) {
                    long ttl = connection.pTtl(key);
                    if (ttl <= 0 || ttl > 150_000) {
                        failures.add(label + " -> wrong TTL: " + ttl + "ms (expected ~60000ms)");
                        continue;
                    }
                }
                passes.add(label);
            } catch (Exception ex) {
                failures.add(label + " -> " + ex.getCause());
            }
        }
    }

    System.out.println("PASS (" + passes.size() + "): " + passes);
    System.out.println("FAIL (" + failures.size() + "): " + failures);
    assertThat(failures).isEmpty();
    assertThat(passes).hasSize(12);
}

Test results

Actual console output, real Redis instance, after the fix, across modules spanning the full supported range:

Module 26 (Spring Data Redis 2.6.10, oldest module with both Expiration kinds):
PASS (12): [relative(60s) + UPSERT, relative(60s) + IF_ABSENT, relative(60s) + IF_PRESENT,
            persistent + UPSERT, persistent + IF_ABSENT, persistent + IF_PRESENT,
            keepTtl + UPSERT, keepTtl + IF_ABSENT, keepTtl + IF_PRESENT,
            unixTimestamp + UPSERT, unixTimestamp + IF_ABSENT, unixTimestamp + IF_PRESENT]
FAIL (0): []
Tests run: 16, Failures: 0, Errors: 0, Skipped: 0

Module 30 (Spring Data Redis 3.0.12):
PASS (12): [... same 12 as above ...]
FAIL (0): []
Tests run: 19, Failures: 0, Errors: 0, Skipped: 0

Module 40 (Spring Data Redis 4.0.5):
PASS (12): [... same 12 as above ...]
FAIL (0): []
Tests run: 32, Failures: 0, Errors: 0, Skipped: 0

Module 41 (Spring Data Redis 4.1.0):
PASS (12): [... same 12 as above ...]
FAIL (0): []
Tests run: 32, Failures: 0, Errors: 0, Skipped: 0

Modules 24, 25 (Spring Data Redis 2.4.15 / 2.5.12, KEEPTTL-only capability):
Tests run: 13, Failures: 0, Errors: 0, Skipped: 0   (each)

Module 20 (Spring Data Redis 2.0.14, unaffected: Expiration has neither method):
unchanged, original 6 tests still pass, no fix applied here

I also individually compiled all 16 affected modules one at a time (mvn -DskipTests compile per module, not a reactor-wide build that could mask a per-module failure) to confirm the isUnixTimestamp() branch only lands in modules where Expiration actually exposes that method.

Additional information

Why this went unnoticed for so long, as best I can tell:

• No existing test in any module's RedissonConnectionTest ever exercised Expiration.keepTtl() or Expiration.unixTimestamp(...) through this specific overload.
• RedissonConnectionTest only extends redisson's own BaseConnectionTest, not Spring Data Redis's shared connection-factory compliance suite, so there's no cross-implementation safety net here.
• Most callers reach this method through the convenience overloads (set(K, V, Duration), set(K, V, long, TimeUnit)), which always build a plain relative-time Expiration and never construct Expiration.keepTtl() or Expiration.unixTimestamp(...). Both bugs only surface when a caller explicitly builds one of those two Expiration variants. Those are real, documented, public APIs, just one step removed from the common path.
• My own first pass at testing this only checked for exceptions, which is exactly why the unixTimestamp bug stayed hidden even after I'd already found the keepTtl one: it doesn't throw, so an exception-only check reports it as passing.

I checked for existing reports before writing this up: nothing in this repo's issues or PRs references this method, SetOption combined with KEEPTTL or unixTimestamp, or matches either reproduction. Similarly-worded issues I found (#7089, #6421, #5392) are unrelated (int overflow in retry config, lock-renewal script errors, and an unrelated unlock script issue).

See PR #7316 for the fix.