rueidiscompat: fix HSetEXWithArgs panic when Condition is unset#1009
Open
jtlang wants to merge 1 commit into
Open
rueidiscompat: fix HSetEXWithArgs panic when Condition is unset#1009jtlang wants to merge 1 commit into
jtlang wants to merge 1 commit into
Conversation
HSetEXWithArgs only built the underlying command inside the Condition == HSetEXFNX / HSetEXFXX branches. Since Condition is optional, leaving it unset (to apply only ExpirationType) left the command builder as its zero value, causing a nil pointer dereference in FieldValue. Add the missing no-condition path, building directly off Hsetex().Key(key) with no .Fnx()/.Fxx().
Collaborator
|
Hi @jtlang, is the new behavior aligned with go-redis? |
Author
|
@rueian Yes, this will bring it in line with go-redis behavior |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
HSetEXWithArgsonly built the underlyingHSETEXcommand inside theCondition == HSetEXFNX/Condition == HSetEXFXXbranches. SinceConditionis optional (perHSetEXOptions), leaving it unset to apply onlyExpirationType(e.g.EX/PX/KEEPTTL) left the command builder at its zero value, causing a nil pointer dereference inHsetexFieldValue.FieldValue.Repro
panics with
invalid memory address or nil pointer dereferenceingen_hash.go:HsetexFieldValue.FieldValue.Fix
Add the missing no-condition path in the
Conditionswitch, building directly offHsetex().Key(key)(no.Fnx()/.Fxx()). Also added adefaultcase to eachExpirationTypeswitch so an unset expiration falls through to.Fields()instead of leaving the command unset.Test plan
should HSetEXWithArgs apply ExpirationType without a Condition) in the existingRedis 8.6+ commandssuite, covering set + get + TTL round-trip.main(before the fix) and passes after.go build/go vetclean; existing hash-command specs still pass.Note
Low Risk
Localized fix to Redis hash command adapter command-building; behavior change only affects previously broken call paths.
Overview
Fixes a nil pointer panic in
HSetEXWithArgswhenHSetEXOptions.Conditionis left unset butExpirationType(e.g.EX) is set—previously onlyFNX/FXXbranches built theHSETEXcommand.The adapter now handles the no-condition path by building
Hsetex().Key(key)with the appropriate expiration modifiers, and addsdefaultfallbacks on eachExpirationTypeswitch (includingFNX/FXX) so the field builder is always initialized beforeFieldValueruns.A regression test covers set/get/TTL when only expiration options are provided.
Reviewed by Cursor Bugbot for commit ff969bc. Bugbot is set up for automated code reviews on this repo. Configure here.