Skip to content

Commit bba5736

Browse files
committed
Fix review issues: trace PII, commitRaw shutdown, TTL checks, shared prefixes
- Redact value arguments in Redis trace logs (show only command + key) - commitRaw: reject new items after Close() to prevent caller deadlock - loadZSetState: check TTL expiration before loading stale zset data - readValueAt: derive user key from internal keys for correct TTL lookup - Extract redisInternalPrefixes into shared variable in redis_compat_types
1 parent 7fcdede commit bba5736

3 files changed

Lines changed: 39 additions & 8 deletions

File tree

adapter/redis.go

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package adapter
33
import (
44
"bytes"
55
"context"
6+
"fmt"
67
"log"
78
"maps"
89
"math"
@@ -118,6 +119,7 @@ const (
118119
redisTraceArgMaxLen = 96
119120
redisTraceArgEllipsis = "..."
120121
redisTraceArgTrimLen = redisTraceArgMaxLen - len(redisTraceArgEllipsis)
122+
redisTraceRedactAfter = 2 // redact arguments after command name + key
121123
)
122124

123125
var redisTxnKeyPrefix = []byte("!txn|")
@@ -504,6 +506,10 @@ func formatTraceArgs(args [][]byte) string {
504506
parts = append(parts, redisTraceArgEllipsis)
505507
break
506508
}
509+
if i >= redisTraceRedactAfter {
510+
parts = append(parts, fmt.Sprintf("<%d bytes>", len(arg)))
511+
continue
512+
}
507513
s := strconv.QuoteToASCII(string(arg))
508514
if len(s) > redisTraceArgMaxLen {
509515
s = s[:redisTraceArgTrimLen] + redisTraceArgEllipsis
@@ -956,13 +962,7 @@ func (r *RedisServer) localKeysPattern(pattern []byte) ([][]byte, error) {
956962
return nil, err
957963
}
958964

959-
for _, prefix := range []string{
960-
redisHashPrefix,
961-
redisSetPrefix,
962-
redisHLLPrefix,
963-
redisZSetPrefix,
964-
redisStreamPrefix,
965-
} {
965+
for _, prefix := range redisInternalPrefixes {
966966
internalStart, internalEnd := listPatternScanBounds(prefix, pattern)
967967
if err := mergeScannedKeys(internalStart, internalEnd); err != nil {
968968
return nil, err
@@ -1225,6 +1225,19 @@ func (t *txnContext) loadZSetState(key []byte) (*zsetTxnState, error) {
12251225
if st, ok := t.zsetStates[k]; ok {
12261226
return st, nil
12271227
}
1228+
// Check TTL: treat expired keys as non-existent.
1229+
ttlSt, err := t.loadTTLState(key)
1230+
if err != nil {
1231+
return nil, err
1232+
}
1233+
if ttlSt.value != nil && !ttlSt.value.After(time.Now()) {
1234+
st := &zsetTxnState{
1235+
members: map[string]float64{},
1236+
exists: false,
1237+
}
1238+
t.zsetStates[k] = st
1239+
return st, nil
1240+
}
12281241
value, exists, err := t.server.loadZSetAt(context.Background(), key, t.startTS)
12291242
if err != nil {
12301243
return nil, err
@@ -2063,7 +2076,11 @@ func (r *RedisServer) tryLeaderGetAt(key []byte, ts uint64) ([]byte, error) {
20632076
}
20642077

20652078
func (r *RedisServer) readValueAt(key []byte, readTS uint64) ([]byte, error) {
2066-
expired, err := r.hasExpiredTTLAt(context.Background(), key, readTS)
2079+
ttlKey := key
2080+
if userKey := extractRedisInternalUserKey(key); userKey != nil {
2081+
ttlKey = userKey
2082+
}
2083+
expired, err := r.hasExpiredTTLAt(context.Background(), ttlKey, readTS)
20672084
if err != nil {
20682085
return nil, err
20692086
}

adapter/redis_compat_types.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,14 @@ const (
2424
redisTTLPrefix = "!redis|ttl|"
2525
)
2626

27+
var redisInternalPrefixes = []string{
28+
redisHashPrefix,
29+
redisSetPrefix,
30+
redisHLLPrefix,
31+
redisZSetPrefix,
32+
redisStreamPrefix,
33+
}
34+
2735
const (
2836
redisUint64Bytes = 8
2937
redisStreamIDParts = 2

kv/transaction.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,12 @@ func (t *TransactionManager) commitRaw(reqs []*pb.Request) (*TransactionResponse
176176

177177
shouldFlush := false
178178
t.mu.Lock()
179+
select {
180+
case <-t.closeCh:
181+
t.mu.Unlock()
182+
return nil, errShuttingDown
183+
default:
184+
}
179185
t.rawPending = append(t.rawPending, item)
180186
if !t.rawFlushing {
181187
t.rawFlushing = true

0 commit comments

Comments
 (0)