mirror of
https://github.com/Mezeporta/Erupe.git
synced 2026-03-22 07:32:32 +01:00
fix(guild): fix nil panics causing clan menu softlock (#171)
The crash was in handleMsgMhfGetGuildManageRight where a variable shadowing bug (guild, err := instead of guild, err =) left the outer guild pointer nil after the inner GetByID lookup succeeded, panicking at guild.MemberCount. This is confirmed by the user's stack trace pointing to handlers_guild.go:234. Also fix 6 other nil-dereference risks across guild handlers: - handleMsgMhfArrangeGuildMember: guild nil after GetByID - handleMsgMhfEnumerateGuildMember: alliance nil when AllianceID > 0 - handleMsgMhfUpdateGuildIcon: guild and characterInfo nil - handleMsgMhfOperateGuild: guild nil, characterGuildInfo nil - handleAvoidLeadershipUpdate: characterGuildData nil Improve panic recovery to log opcode and full stack trace so future panics can be diagnosed from console screenshots.
This commit is contained in:
@@ -41,7 +41,7 @@ func handleMsgMhfArrangeGuildMember(s *Session, p mhfpacket.MHFPacket) {
|
||||
|
||||
guild, err := s.server.guildRepo.GetByID(pkt.GuildID)
|
||||
|
||||
if err != nil {
|
||||
if err != nil || guild == nil {
|
||||
s.logger.Error(
|
||||
"failed to respond to ArrangeGuildMember message",
|
||||
zap.Uint32("charID", s.charID),
|
||||
@@ -131,7 +131,7 @@ func handleMsgMhfEnumerateGuildMember(s *Session, p mhfpacket.MHFPacket) {
|
||||
|
||||
alliance, err := s.server.guildRepo.GetAllianceByID(guild.AllianceID)
|
||||
if err != nil {
|
||||
s.logger.Error("Failed to get alliance data")
|
||||
s.logger.Error("Failed to get alliance data", zap.Error(err))
|
||||
doAckBufFail(s, pkt.AckHandle, make([]byte, 4))
|
||||
return
|
||||
}
|
||||
@@ -170,7 +170,7 @@ func handleMsgMhfEnumerateGuildMember(s *Session, p mhfpacket.MHFPacket) {
|
||||
bf.WriteUint32(member.LastLogin)
|
||||
}
|
||||
|
||||
if guild.AllianceID > 0 {
|
||||
if guild.AllianceID > 0 && alliance != nil {
|
||||
bf.WriteUint16(alliance.TotalMembers - uint16(len(guildMembers)))
|
||||
if guild.ID != alliance.ParentGuildID {
|
||||
mems, err := s.server.guildRepo.GetMembers(alliance.ParentGuildID, false)
|
||||
@@ -222,7 +222,8 @@ func handleMsgMhfGetGuildManageRight(s *Session, p mhfpacket.MHFPacket) {
|
||||
|
||||
guild, _ := s.server.guildRepo.GetByCharID(s.charID)
|
||||
if guild == nil || s.prevGuildID != 0 {
|
||||
guild, err := s.server.guildRepo.GetByID(s.prevGuildID)
|
||||
var err error
|
||||
guild, err = s.server.guildRepo.GetByID(s.prevGuildID)
|
||||
s.prevGuildID = 0
|
||||
if guild == nil || err != nil {
|
||||
doAckBufSucceed(s, pkt.AckHandle, make([]byte, 4))
|
||||
@@ -298,7 +299,7 @@ func handleMsgMhfUpdateGuildIcon(s *Session, p mhfpacket.MHFPacket) {
|
||||
|
||||
guild, err := s.server.guildRepo.GetByID(pkt.GuildID)
|
||||
|
||||
if err != nil {
|
||||
if err != nil || guild == nil {
|
||||
s.logger.Error("Failed to get guild info for icon update", zap.Error(err))
|
||||
doAckSimpleFail(s, pkt.AckHandle, make([]byte, 4))
|
||||
return
|
||||
@@ -306,7 +307,7 @@ func handleMsgMhfUpdateGuildIcon(s *Session, p mhfpacket.MHFPacket) {
|
||||
|
||||
characterInfo, err := s.server.guildRepo.GetCharacterMembership(s.charID)
|
||||
|
||||
if err != nil {
|
||||
if err != nil || characterInfo == nil {
|
||||
s.logger.Error("Failed to get character guild data for icon update", zap.Error(err))
|
||||
doAckSimpleFail(s, pkt.AckHandle, make([]byte, 4))
|
||||
return
|
||||
|
||||
@@ -13,7 +13,7 @@ func handleMsgMhfOperateGuild(s *Session, p mhfpacket.MHFPacket) {
|
||||
pkt := p.(*mhfpacket.MsgMhfOperateGuild)
|
||||
|
||||
guild, err := s.server.guildRepo.GetByID(pkt.GuildID)
|
||||
if err != nil {
|
||||
if err != nil || guild == nil {
|
||||
doAckSimpleFail(s, pkt.AckHandle, make([]byte, 4))
|
||||
return
|
||||
}
|
||||
@@ -49,7 +49,8 @@ func handleMsgMhfOperateGuild(s *Session, p mhfpacket.MHFPacket) {
|
||||
bf.WriteUint32(0)
|
||||
}
|
||||
case mhfpacket.OperateGuildLeave:
|
||||
result, err := s.server.guildService.Leave(s.charID, guild.ID, characterGuildInfo.IsApplicant, guild.Name)
|
||||
isApplicant := characterGuildInfo != nil && characterGuildInfo.IsApplicant
|
||||
result, err := s.server.guildService.Leave(s.charID, guild.ID, isApplicant, guild.Name)
|
||||
if err != nil {
|
||||
s.logger.Error("Failed to leave guild", zap.Error(err))
|
||||
}
|
||||
@@ -73,7 +74,7 @@ func handleMsgMhfOperateGuild(s *Session, p mhfpacket.MHFPacket) {
|
||||
case mhfpacket.OperateGuildSetAvoidLeadershipFalse:
|
||||
handleAvoidLeadershipUpdate(s, pkt, false)
|
||||
case mhfpacket.OperateGuildUpdateComment:
|
||||
if !characterGuildInfo.IsLeader && !characterGuildInfo.IsSubLeader() {
|
||||
if characterGuildInfo == nil || (!characterGuildInfo.IsLeader && !characterGuildInfo.IsSubLeader()) {
|
||||
doAckSimpleFail(s, pkt.AckHandle, make([]byte, 4))
|
||||
return
|
||||
}
|
||||
@@ -82,7 +83,7 @@ func handleMsgMhfOperateGuild(s *Session, p mhfpacket.MHFPacket) {
|
||||
s.logger.Error("Failed to save guild comment", zap.Error(err))
|
||||
}
|
||||
case mhfpacket.OperateGuildUpdateMotto:
|
||||
if !characterGuildInfo.IsLeader && !characterGuildInfo.IsSubLeader() {
|
||||
if characterGuildInfo == nil || (!characterGuildInfo.IsLeader && !characterGuildInfo.IsSubLeader()) {
|
||||
doAckSimpleFail(s, pkt.AckHandle, make([]byte, 4))
|
||||
return
|
||||
}
|
||||
@@ -217,7 +218,7 @@ func handleDonateRP(s *Session, amount uint16, guild *Guild, _type int) []byte {
|
||||
func handleAvoidLeadershipUpdate(s *Session, pkt *mhfpacket.MsgMhfOperateGuild, avoidLeadership bool) {
|
||||
characterGuildData, err := s.server.guildRepo.GetCharacterMembership(s.charID)
|
||||
|
||||
if err != nil {
|
||||
if err != nil || characterGuildData == nil {
|
||||
doAckSimpleFail(s, pkt.AckHandle, make([]byte, 4))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -7,6 +7,7 @@ import (
|
||||
"fmt"
|
||||
"io"
|
||||
"net"
|
||||
"runtime/debug"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"time"
|
||||
@@ -240,7 +241,12 @@ func (s *Session) handlePacketGroup(pktGroup []byte) {
|
||||
// This shouldn't be needed, but it's better to recover and let the connection die than to panic the server.
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
s.logger.Error("Recovered from panic", zap.String("name", s.Name), zap.Any("panic", r))
|
||||
s.logger.Error("Recovered from panic",
|
||||
zap.String("name", s.Name),
|
||||
zap.Stringer("opcode", opcode),
|
||||
zap.Any("panic", r),
|
||||
zap.String("stack", string(debug.Stack())),
|
||||
)
|
||||
}
|
||||
}()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user