Fix four nil pointer dereferences that crash the server#460
Merged
Volte6 merged 1 commit intoGoMudEngine:masterfrom Apr 6, 2026
Merged
Fix four nil pointer dereferences that crash the server#460Volte6 merged 1 commit intoGoMudEngine:masterfrom
Volte6 merged 1 commit intoGoMudEngine:masterfrom
Conversation
Add nil checks before dereference in: - MoveToRoom: user lookup may return nil for stale/invalid userId - LogOutUserByConnectionId: user record may be nil when connection map is inconsistent with user map - TryRoomCommand: room may be nil for destroyed ephemeral rooms - SaveRoomTemplate: room may not be in memory cache for new rooms Adds 7 tests across 3 packages to verify each nil path is handled gracefully. Adds test helpers in users package for cross-package test setup. Fixes #6 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Volte6
approved these changes
Apr 5, 2026
Member
Volte6
left a comment
Volte6
left a comment
There was a problem hiding this comment.
Thanks for your contributions!
arinbb
pushed a commit
to arinbb/GoMud
that referenced
this pull request
Apr 17, 2026
) ## Summary Fixes four nil pointer dereference bugs that can panic and crash the server during normal operation. ## Changes 1. **MoveToRoom** (rooms/roommanager.go): users.GetByUserId can return nil for stale/invalid userId. Added nil check before accessing user.Character.RoomId. 2. **LogOutUserByConnectionId** (users/users.go): When connection map contains an entry but user map does not, u is nil but dereferenced unconditionally. Moved map delete calls inside the u != nil guard and added cleanup of orphaned connection entries. 3. **TryRoomCommand** (scripting/room.go): rooms.LoadRoom can return nil for destroyed ephemeral rooms. room.FindExitByName was called before the existing if room != nil check. Added early return. 4. **SaveRoomTemplate** (rooms/save_and_load.go): roomManager.rooms lookup returns nil for new rooms not yet in memory. Added nil guard before ranging over .Containers. ## Tests - 7 new tests across 3 packages verify each nil path is handled gracefully - Tests use require.NotPanics from testify to assert no panic on nil paths - Adds small test helper functions in users package for cross-package test setup ## Test plan - [x] All 7 new tests pass - [x] Full test suite passes (pre-existing flaky test in procedural/ is unrelated) - [x] Tests were written first to demonstrate the panics, then fixes applied
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
2 participants
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
Fixes four nil pointer dereference bugs that can panic and crash the server during normal operation.
Changes
Tests
Test plan