Skip to content

Commit 8bb0e59

Browse files
committed
fix: ensure push notification processor is never nil in newConn
- Add nil check in newConn to create VoidPushNotificationProcessor when needed - Fix tests to use Protocol 2 for disabled push notification scenarios - Prevent nil pointer dereference in transaction and connection contexts - Ensure consistent behavior across all connection creation paths The panic was occurring because newConn could create connections with nil pushProcessor when options didn't have a processor set. Now we always ensure a processor exists (real or void) to maintain the 'never nil' guarantee.
1 parent 3ff465d commit 8bb0e59

File tree

2 files changed

+18
-7
lines changed

2 files changed

+18
-7
lines changed

push_notifications_test.go

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -174,9 +174,10 @@ func TestClientPushNotificationIntegration(t *testing.T) {
174174
}
175175

176176
func TestClientWithoutPushNotifications(t *testing.T) {
177-
// Test client without push notifications enabled
177+
// Test client without push notifications enabled (using RESP2)
178178
client := redis.NewClient(&redis.Options{
179179
Addr: "localhost:6379",
180+
Protocol: 2, // RESP2 doesn't support push notifications
180181
PushNotifications: false, // Disabled
181182
})
182183
defer client.Close()
@@ -651,9 +652,10 @@ func TestPushNotificationProcessorConvenienceMethods(t *testing.T) {
651652
}
652653

653654
func TestClientPushNotificationEdgeCases(t *testing.T) {
654-
// Test client methods when processor is nil
655+
// Test client methods when using void processor (RESP2)
655656
client := redis.NewClient(&redis.Options{
656657
Addr: "localhost:6379",
658+
Protocol: 2, // RESP2 doesn't support push notifications
657659
PushNotifications: false, // Disabled
658660
})
659661
defer client.Close()
@@ -673,10 +675,14 @@ func TestClientPushNotificationEdgeCases(t *testing.T) {
673675
t.Errorf("Expected nil error when processor is nil, got: %v", err)
674676
}
675677

676-
// GetPushNotificationProcessor should return nil
678+
// GetPushNotificationProcessor should return VoidPushNotificationProcessor
677679
processor := client.GetPushNotificationProcessor()
678-
if processor != nil {
679-
t.Error("Processor should be nil when push notifications are disabled")
680+
if processor == nil {
681+
t.Error("Processor should never be nil")
682+
}
683+
// VoidPushNotificationProcessor should have nil registry
684+
if processor.GetRegistry() != nil {
685+
t.Error("VoidPushNotificationProcessor should have nil registry when disabled")
680686
}
681687
}
682688

redis.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -970,8 +970,13 @@ func newConn(opt *Options, connPool pool.Pooler, parentHooks *hooksMixin) *Conn
970970
c.hooksMixin = parentHooks.clone()
971971
}
972972

973-
// Set push notification processor from options (always available now)
974-
c.pushProcessor = opt.PushNotificationProcessor
973+
// Set push notification processor from options, ensure it's never nil
974+
if opt.PushNotificationProcessor != nil {
975+
c.pushProcessor = opt.PushNotificationProcessor
976+
} else {
977+
// Create a void processor if none provided to ensure we never have nil
978+
c.pushProcessor = NewVoidPushNotificationProcessor()
979+
}
975980

976981
c.cmdable = c.Process
977982
c.statefulCmdable = c.Process

0 commit comments

Comments
 (0)