1
0
mirror of https://github.com/redis/go-redis.git synced 2026-10-24 07:59:20 +03:00
Files
go-redis/command_entry_array_desync_test.go
sadd amr3e dbedca714c fix(command): reconcile declared entry-array length in reply parsers (#3907)
* reconcile declared entry-array length in reply parsers

* drain extra GEOSEARCH entry elements instead of erroring

* fix(command): repair SLOWLOG GET merge resolution

The conflict resolution in b895673c kept master's nn > 7 upper-bound
error alongside the drain loop (making the drain dead code, against the
PR's intent) and broke the brace structure, so the package no longer
compiled. Keep the Redis 8.10+ CommandArgc field from master, drop the
upper-bound error, and drain any elements past the 7 known fields.

* test(command): exercise SlowLog drain loop

Bugbot flagged that a 7-element entry is fully consumed by the parser
now that CommandArgc is the 7th known field, so the drain loop never
ran. Declare 8 elements — the 7 known fields plus one extra — and
assert CommandArgc is parsed rather than drained.

---------

Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com>
Co-authored-by: Nedyalko Dyakov <nedyalko.dyakov@gmail.com>
2026-07-29 13:36:18 +03:00

122 lines
4.3 KiB
Go

package redis
import (
"context"
"strings"
"testing"
"github.com/redis/go-redis/v9/internal/proto"
)
// These parsers read a fixed set of fields from each entry array but did not
// reconcile that with the element count the server declares. A server that
// declares a longer array leaves the trailing frames on the wire; readReply
// returns nil and the desynced connection goes back to the pool, so the next
// command reads the leftover frames as its own reply. Each case appends a
// sentinel reply and checks it is still readable after readReply.
func TestEntryArrayDrainsExtraElements(t *testing.T) {
t.Run("LatencyCmd", func(t *testing.T) {
// entry declares 5 elements: the 4 fields plus one extra
rd := proto.NewReader(strings.NewReader(
"*1\r\n*5\r\n$5\r\nevent\r\n:1000\r\n:5\r\n:9\r\n:7\r\n+PONG\r\n"))
cmd := NewLatencyCmd(context.Background())
if err := cmd.readReply(rd); err != nil {
t.Fatalf("readReply: %v", err)
}
assertSentinel(t, rd)
})
t.Run("SlowLogCmd", func(t *testing.T) {
// entry declares 8 elements: the 7 known fields (through CommandArgc)
// plus one extra that only the drain loop can consume
rd := proto.NewReader(strings.NewReader(
"*1\r\n*8\r\n:1\r\n:1000\r\n:50\r\n*1\r\n$3\r\nGET\r\n$5\r\n1.2.3\r\n$4\r\nname\r\n:99\r\n$5\r\nextra\r\n+PONG\r\n"))
cmd := NewSlowLogCmd(context.Background())
if err := cmd.readReply(rd); err != nil {
t.Fatalf("readReply: %v", err)
}
if len(cmd.val) != 1 || cmd.val[0].CommandArgc != 99 {
t.Fatalf("CommandArgc not parsed before drain: %+v", cmd.val)
}
assertSentinel(t, rd)
})
t.Run("HotKeysCmd", func(t *testing.T) {
// outer array wraps two elements; only the first is parsed
rd := proto.NewReader(strings.NewReader(
"*2\r\n%1\r\n$15\r\ntracking-active\r\n:1\r\n%0\r\n+PONG\r\n"))
cmd := NewHotKeysCmd(context.Background())
if err := cmd.readReply(rd); err != nil {
t.Fatalf("readReply: %v", err)
}
assertSentinel(t, rd)
})
}
// LATENCY GET entries hold 4 fields; a 3-element entry must be rejected rather
// than over-read into the next reply.
func TestLatencyCmdShortEntryRejected(t *testing.T) {
// The trailing :9 belongs to the next reply; the unpatched parser reads it
// as the 4th field and returns nil, desyncing the connection.
rd := proto.NewReader(strings.NewReader(
"*1\r\n*3\r\n$5\r\nevent\r\n:1000\r\n:5\r\n:9\r\n"))
cmd := NewLatencyCmd(context.Background())
if err := cmd.readReply(rd); err == nil {
t.Fatalf("short latency entry accepted; readReply returned nil (val=%+v)", cmd.val)
}
}
// GeoSearchLocationCmd must reconcile each entry array against the requested
// WITH flags: extra elements are drained (forwards compatible with servers
// that append fields), while entries shorter than the flags require are
// rejected rather than over-read into the next reply.
func TestGeoSearchLocationEntryLengthReconciled(t *testing.T) {
opt := &GeoSearchLocationQuery{WithDist: true} // withLen == 2
t.Run("oversized drained", func(t *testing.T) {
rd := proto.NewReader(strings.NewReader(
"*1\r\n*3\r\n$1\r\na\r\n$3\r\n1.5\r\n:9\r\n+PONG\r\n"))
cmd := NewGeoSearchLocationCmd(context.Background(), opt)
if err := cmd.readReply(rd); err != nil {
t.Fatalf("readReply: %v", err)
}
if len(cmd.val) != 1 || cmd.val[0].Name != "a" || cmd.val[0].Dist != 1.5 {
t.Fatalf("unexpected val: %+v", cmd.val)
}
assertSentinel(t, rd)
})
t.Run("short rejected", func(t *testing.T) {
rd := proto.NewReader(strings.NewReader(
"*1\r\n*1\r\n$1\r\na\r\n"))
cmd := NewGeoSearchLocationCmd(context.Background(), opt)
if err := cmd.readReply(rd); err == nil {
t.Fatalf("short geo entry accepted; readReply returned nil (val=%+v)", cmd.val)
}
})
t.Run("valid parses", func(t *testing.T) {
rd := proto.NewReader(strings.NewReader(
"*1\r\n*2\r\n$1\r\na\r\n$3\r\n1.5\r\n"))
cmd := NewGeoSearchLocationCmd(context.Background(), opt)
if err := cmd.readReply(rd); err != nil {
t.Fatalf("valid geo entry: %v", err)
}
if len(cmd.val) != 1 || cmd.val[0].Name != "a" || cmd.val[0].Dist != 1.5 {
t.Fatalf("unexpected val: %+v", cmd.val)
}
})
}
func assertSentinel(t *testing.T, rd *proto.Reader) {
t.Helper()
s, err := rd.ReadString()
if err != nil {
t.Fatalf("stream desynced, sentinel unreadable: %v", err)
}
if s != "PONG" {
t.Fatalf("stream desynced, got %q want PONG", s)
}
}