fix(script): keep client-computed SHA-1 in Script.Load (#3943)

* fix(script): keep client-computed SHA-1 in Script.Load

* fix(script): export ErrScriptDigestMismatch

Wrap the Script.Load mismatch error so errors.Is works, skip the
comparison while the SCRIPT LOAD reply is still queued in a pipeline
(Val is empty until Exec), and fold the tests into
script_server_sha_unit_test.go.

* fix(script): compare Load digest case-insensitively

script.go

@@ -5,7 +5,9 @@ import (
 	"crypto/sha1"
 	"encoding/hex"
 	"errors"
+	"fmt"
 	"io"
+	"strings"
 	"sync"
 )
 
@@ -24,6 +26,11 @@ var (
 	_ Scripter = (*ClusterClient)(nil)
 )
 
+// ErrScriptDigestMismatch is returned by Script.Load when SCRIPT LOAD answers
+// with a digest that differs from the SHA-1 computed by NewScript. The local
+// digest is kept so EVALSHA stays pinned to this script's source.
+var ErrScriptDigestMismatch = errors.New("redis: SCRIPT LOAD returned a digest that does not match the client-computed SHA-1")
+
 type Script struct {
 	src       string
 	mu        sync.RWMutex
@@ -60,11 +67,27 @@ func (s *Script) Hash() string {
 
 func (s *Script) Load(ctx context.Context, c Scripter) *StringCmd {
 	cmd := c.ScriptLoad(ctx, s.src)
-	if err := cmd.Err(); err == nil {
-		s.mu.Lock()
-		s.hash = cmd.Val()
-		s.mu.Unlock()
+	if err := cmd.Err(); err != nil {
+		return cmd
 	}
+
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	if !s.serverSHA {
+		// s.hash is SHA-1(s.src) computed in NewScript, and SCRIPT LOAD returns
+		// that same digest. A different value is a protocol violation; keeping
+		// the local one means EVALSHA stays pinned to this script instead of
+		// running whatever the server holds under the returned digest.
+		// Val is empty while the command is still queued in a pipeline.
+		// Redis treats digests case-insensitively, so compare that way too.
+		if v := cmd.Val(); v != "" && !strings.EqualFold(v, s.hash) {
+			cmd.SetErr(fmt.Errorf("%w: got %q, want %q", ErrScriptDigestMismatch, v, s.hash))
+		}
+		return cmd
+	}
+
+	s.hash = cmd.Val()
 	return cmd
 }
 

script_server_sha_unit_test.go

@@ -2,6 +2,7 @@ package redis
 
 import (
 	"context"
+	"errors"
 	"strings"
 	"sync"
 	"testing"
@@ -24,6 +25,10 @@ type fakeScripter struct {
 	// behavior controls
 	hashToReturn string
 
+	// digests the script helper actually sent with EVALSHA/EVALSHA_RO
+	lastEvalShaSHA   string
+	lastEvalShaROSHA string
+
 	// If set, the first EvalSha/EvalShaRO returns a NOSCRIPT error.
 	failFirstEvalShaWithNoScr   bool
 	failFirstEvalShaROWithNoScr bool
@@ -62,6 +67,7 @@ func (f *fakeScripter) Eval(ctx context.Context, script string, keys []string, a
 func (f *fakeScripter) EvalSha(ctx context.Context, sha1 string, keys []string, args ...interface{}) *Cmd {
 	f.mu.Lock()
 	f.evalShaCalls++
+	f.lastEvalShaSHA = sha1
 	callNum := f.evalShaCalls
 	fail := f.failFirstEvalShaWithNoScr && callNum == 1
 	f.mu.Unlock()
@@ -89,6 +95,7 @@ func (f *fakeScripter) EvalRO(ctx context.Context, script string, keys []string,
 func (f *fakeScripter) EvalShaRO(ctx context.Context, sha1 string, keys []string, args ...interface{}) *Cmd {
 	f.mu.Lock()
 	f.evalShaROCalls++
+	f.lastEvalShaROSHA = sha1
 	callNum := f.evalShaROCalls
 	fail := f.failFirstEvalShaROWithNoScr && callNum == 1
 	f.mu.Unlock()
@@ -191,3 +198,103 @@ func TestNewScriptServerSHA_RunRO_RetriesOnNoScript_NoEvalROFallback(t *testing.
 		t.Fatalf("expected EvalRO 0 calls, got %d", c.evalROCalls)
 	}
 }
+
+// forgedSHA is a well-formed digest that is not the SHA-1 of any script used here.
+const forgedSHA = "0123456789abcdef0123456789abcdef01234567"
+
+func TestScriptLoad_KeepsClientComputedDigest(t *testing.T) {
+	ctx := context.Background()
+	c := &fakeScripter{hashToReturn: forgedSHA}
+
+	s := NewScript("return 1")
+	want := s.Hash()
+
+	err := s.Load(ctx, c).Err()
+	if !errors.Is(err, ErrScriptDigestMismatch) {
+		t.Fatalf("Load() err = %v, want ErrScriptDigestMismatch", err)
+	}
+	if got := s.Hash(); got != want {
+		t.Fatalf("Hash() = %q after Load, want the locally computed %q", got, want)
+	}
+
+	if err := s.Run(ctx, c, []string{"k"}).Err(); err != nil {
+		t.Fatalf("Run() err: %v", err)
+	}
+	if c.lastEvalShaSHA != want {
+		t.Fatalf("EVALSHA sent %q, want %q", c.lastEvalShaSHA, want)
+	}
+}
+
+func TestScriptLoad_KeepsClientComputedDigest_RO(t *testing.T) {
+	ctx := context.Background()
+	c := &fakeScripter{hashToReturn: forgedSHA}
+
+	s := NewScript("return 2")
+	want := s.Hash()
+	_ = s.Load(ctx, c)
+
+	if err := s.RunRO(ctx, c, []string{"k"}).Err(); err != nil {
+		t.Fatalf("RunRO() err: %v", err)
+	}
+	if c.lastEvalShaROSHA != want {
+		t.Fatalf("EVALSHA_RO sent %q, want %q", c.lastEvalShaROSHA, want)
+	}
+}
+
+func TestScriptLoad_MatchingDigestIsNotAnError(t *testing.T) {
+	ctx := context.Background()
+	s := NewScript("return 3")
+	c := &fakeScripter{hashToReturn: s.Hash()}
+
+	cmd := s.Load(ctx, c)
+	if err := cmd.Err(); err != nil {
+		t.Fatalf("Load() err: %v", err)
+	}
+	if cmd.Val() != s.Hash() {
+		t.Fatalf("Load() val = %q, want %q", cmd.Val(), s.Hash())
+	}
+}
+
+func TestScriptLoad_UppercaseDigestIsNotAnError(t *testing.T) {
+	ctx := context.Background()
+	s := NewScript("return 5")
+	want := s.Hash()
+	c := &fakeScripter{hashToReturn: strings.ToUpper(want)}
+
+	if err := s.Load(ctx, c).Err(); err != nil {
+		t.Fatalf("Load() err: %v", err)
+	}
+	if got := s.Hash(); got != want {
+		t.Fatalf("Hash() = %q after Load, want %q", got, want)
+	}
+}
+
+func TestScriptLoad_EmptyDigestIsNotAnError(t *testing.T) {
+	// A pipelined SCRIPT LOAD has no value until Exec; Load must not flag it.
+	ctx := context.Background()
+	c := &fakeScripter{hashToReturn: ""}
+
+	s := NewScript("return 4")
+	want := s.Hash()
+
+	if err := s.Load(ctx, c).Err(); err != nil {
+		t.Fatalf("Load() err: %v", err)
+	}
+	if got := s.Hash(); got != want {
+		t.Fatalf("Hash() = %q after Load, want %q", got, want)
+	}
+}
+
+func TestScriptLoad_ServerSHAAdoptsServerDigest(t *testing.T) {
+	ctx := context.Background()
+	hash := strings.Repeat("e", 40)
+	c := &fakeScripter{hashToReturn: hash}
+
+	s := NewScriptServerSHA("return 5")
+	if err := s.Load(ctx, c).Err(); err != nil {
+		t.Fatalf("Load() err: %v", err)
+	}
+	if s.Hash() != hash {
+		t.Fatalf("Hash() = %q, want %q", s.Hash(), hash)
+	}
+}