redis/go-redis
1
0
Fork 0
mirror of https://github.com/redis/go-redis.git synced 2025-07-29 17:41:15 +03:00
Code Activity

fix: ensure push notification processor is never nil in newConn

Browse Source 
- 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.
This commit is contained in:
Nedyalko Dyakov
2025-06-27 01:32:30 +03:00
parent be9b6dd6a0
commit 8006fab753
2 changed files with 18 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
push_notifications_test.go
View File

@ -174,9 +174,10 @@ func TestClientPushNotificationIntegration(t *testing.T) {
} }
func TestClientWithoutPushNotifications(t *testing.T) { func TestClientWithoutPushNotifications(t *testing.T) {
// Test client without push notifications enabled // Test client without push notifications enabled (using RESP2)
client := redis.NewClient(&redis.Options{ client := redis.NewClient(&redis.Options{
Addr: "localhost:6379", Addr: "localhost:6379",
Protocol: 2, // RESP2 doesn't support push notifications
PushNotifications: false, // Disabled PushNotifications: false, // Disabled
}) })
defer client.Close() defer client.Close()
@ -651,9 +652,10 @@ func TestPushNotificationProcessorConvenienceMethods(t *testing.T) {
} }
func TestClientPushNotificationEdgeCases(t *testing.T) { func TestClientPushNotificationEdgeCases(t *testing.T) {
// Test client methods when processor is nil // Test client methods when using void processor (RESP2)
client := redis.NewClient(&redis.Options{ client := redis.NewClient(&redis.Options{
Addr: "localhost:6379", Addr: "localhost:6379",
Protocol: 2, // RESP2 doesn't support push notifications
PushNotifications: false, // Disabled PushNotifications: false, // Disabled
}) })
defer client.Close() defer client.Close()
@ -673,10 +675,14 @@ func TestClientPushNotificationEdgeCases(t *testing.T) {
t.Errorf("Expected nil error when processor is nil, got: %v", err) t.Errorf("Expected nil error when processor is nil, got: %v", err)
} }
// GetPushNotificationProcessor should return nil // GetPushNotificationProcessor should return VoidPushNotificationProcessor
processor := client.GetPushNotificationProcessor() processor := client.GetPushNotificationProcessor()
if processor != nil { if processor == nil {
t.Error("Processor should be nil when push notifications are disabled") t.Error("Processor should never be nil")
}
// VoidPushNotificationProcessor should have nil registry
if processor.GetRegistry() != nil {
t.Error("VoidPushNotificationProcessor should have nil registry when disabled")
} }
} }

7
redis.go
View File

@ -970,8 +970,13 @@ func newConn(opt *Options, connPool pool.Pooler, parentHooks *hooksMixin) *Conn
c.hooksMixin = parentHooks.clone() c.hooksMixin = parentHooks.clone()
} }
// Set push notification processor from options (always available now) // Set push notification processor from options, ensure it's never nil
if opt.PushNotificationProcessor != nil {
c.pushProcessor = opt.PushNotificationProcessor c.pushProcessor = opt.PushNotificationProcessor
} else {
// Create a void processor if none provided to ensure we never have nil
c.pushProcessor = NewVoidPushNotificationProcessor()
}
c.cmdable = c.Process c.cmdable = c.Process
c.statefulCmdable = c.Process c.statefulCmdable = c.Process