Skip to content

fix(conn): guard against nil MaintNotificationsConfig in initConn#3707

Merged
ndyakov merged 2 commits intoredis:masterfrom
veeceey:fix/nil-pointer-maint-notifications-config
Feb 11, 2026
Merged

fix(conn): guard against nil MaintNotificationsConfig in initConn#3707
ndyakov merged 2 commits intoredis:masterfrom
veeceey:fix/nil-pointer-maint-notifications-config

Conversation

@veeceey
Copy link
Copy Markdown
Contributor

@veeceey veeceey commented Feb 10, 2026

Summary

  • Fix nil pointer dereference in initConn when MaintNotificationsConfig is nil
  • Add regression test that verifies the fix against the exact panic scenario

Problem

In baseClient.initConn(), line 540 accessed MaintNotificationsConfig.EndpointType unconditionally, even though line 538 correctly nil-checked MaintNotificationsConfig via short-circuit evaluation:

// Line 538: correctly checks for nil
maintNotifEnabled := c.opt.MaintNotificationsConfig != nil && c.opt.MaintNotificationsConfig.Mode != maintnotifications.ModeDisabled
protocol := c.opt.Protocol
// Line 540: BYPASSES the nil check — panics when MaintNotificationsConfig is nil
endpointType := c.opt.MaintNotificationsConfig.EndpointType

When MaintNotificationsConfig is nil, this causes:

  1. A nil pointer dereference panic
  2. optLock.RLock() is left held (never reaches RUnlock()), causing a deadlock on the next call

Fix

Move the EndpointType access inside a nil guard, consistent with the existing nil check on the Mode field:

var endpointType maintnotifications.EndpointType
if c.opt.MaintNotificationsConfig != nil {
    endpointType = c.opt.MaintNotificationsConfig.EndpointType
}

Test plan

  • Added TestInitConnNilMaintNotificationsConfig regression test
  • Verified test fails (catches panic) without the fix
  • Verified test passes with the fix
  • go vet ./... passes
  • go build ./... passes
  • All existing unit tests pass (only TestGinkgoSuite fails, which requires a running Redis server)

Fixes #3675

🤖 Generated with Claude Code

In initConn, line 540 accessed MaintNotificationsConfig.EndpointType
unconditionally, even though line 538 correctly nil-checked
MaintNotificationsConfig. When MaintNotificationsConfig is nil, this
caused a nil pointer dereference panic and left optLock.RLock held,
leading to a deadlock on the next call.

Move the EndpointType access inside a nil guard to match the existing
nil check for the Mode field.

Fixes redis#3675

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ndyakov
Copy link
Copy Markdown
Member

ndyakov commented Feb 10, 2026

Thank you @veeceey, I think we can directly use if maintNotifEnabled since it reads better, don't you think?

ndyakov
ndyakov previously approved these changes Feb 10, 2026
@veeceey
Copy link
Copy Markdown
Contributor Author

veeceey commented Feb 11, 2026

Good point, that does read better. Updated to use if maintNotifEnabled -- since it already implies the config is non-nil, the guard is redundant. And endpointType is only consumed inside the if maintNotifEnabled block below anyway, so the semantics stay the same.

Copy link
Copy Markdown
Member

@ndyakov ndyakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @veeceey!

@ndyakov ndyakov added the bug label Feb 11, 2026
@ndyakov ndyakov merged commit 34f4568 into redis:master Feb 11, 2026
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

invalid memory address or nil pointer dereference in *baseClient.initConn

2 participants