Skip to content

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

Open
saddamr3e wants to merge 1 commit into
redis:masterfrom
saddamr3e:script-load-digest
Open

fix(script): keep client-computed SHA-1 in Script.Load#3943
saddamr3e wants to merge 1 commit into
redis:masterfrom
saddamr3e:script-load-digest

Conversation

@saddamr3e

@saddamr3e saddamr3e commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Reproducer with a Scripter whose SCRIPT LOAD answers with a digest of its own choosing:

--- FAIL: TestScriptLoad_KeepsClientComputedDigest_RO
    script_load_digest_test.go:46: EVALSHA_RO sent "0123456789abcdef0123456789abcdef01234567", want "7f923f79fe76194c868d7e1d0820de36700eb649"

NewScript computes hex(sha1(src)) so EVALSHA stays pinned to that script body, but Load overwrote it with the server's string. One reply repoints every later Run/RunRO on that *Script at whatever Lua the peer has cached under the returned digest, running with the caller's keys and ARGV, and EVALSHA still reports success.

Overwriting is only correct for NewScriptServerSHA, where the assignment came from in #3700; kept it on that path and made the client-side-SHA path report the mismatch instead.


Note

Medium Risk
Touches Redis script execution identity (EVALSHA digests), which can affect correctness if callers relied on the old overwrite behavior; scope is limited to Script.Load and well-covered by tests.

Overview
Script.Load no longer replaces the client-side digest for scripts created with NewScript. It keeps the locally computed SHA-1 of the script source and errors if SCRIPT LOAD returns a different digest, so later Run / RunRO / EVALSHA stay tied to that script body instead of whatever Lua the server has under a mismatched hash.

NewScriptServerSHA behavior is unchanged: Load still adopts the digest returned by Redis. Tests cover mismatch handling, matching digests, server-SHA adoption, and EVALSHA / EVALSHA_RO digest selection via an extended fakeScripter.

Reviewed by Cursor Bugbot for commit b476f8a. Bugbot is set up for automated code reviews on this repo. Configure here.

@ndyakov

ndyakov commented Aug 3, 2026

Copy link
Copy Markdown
Member

Thank you for the contribution @saddamr3e, To set expectations - I review it once we release the GA version so it won't make it in 9.22.

@saddamr3e

Copy link
Copy Markdown
Contributor Author

Sounds good, no rush. I'll keep the branch up to date in the meantime.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants