refactor(channelserver): replace init() handler registration with explicit construction

The handler table was a package-level global populated by init(), making
registration implicit and untestable. Move it to buildHandlerTable()
which returns the map, store it as a Server struct field initialized in
NewServer(), and add a missing-handler guard in handlePacketGroup to log
a warning instead of panicking on unknown opcodes.
This commit is contained in:
Houmgaor
2026-02-20 18:58:32 +01:00
parent 45c29837a5
commit e5133e5dcf
6 changed files with 66 additions and 35 deletions

View File

@@ -168,7 +168,8 @@ func TestHandlerTableRegistered(t *testing.T) {
} }
// Verify handler table is populated // Verify handler table is populated
if len(handlerTable) == 0 { table := buildHandlerTable()
if len(table) == 0 {
t.Error("handlers table should not be empty") t.Error("handlers table should not be empty")
} }
@@ -181,8 +182,8 @@ func TestHandlerTableRegistered(t *testing.T) {
_ = criticalHandlers // We just verify the table is non-empty since handler function names aren't directly accessible _ = criticalHandlers // We just verify the table is non-empty since handler function names aren't directly accessible
// Verify minimum handler count // Verify minimum handler count
if len(handlerTable) < 50 { if len(table) < 50 {
t.Errorf("handlers count = %d, expected at least 50", len(handlerTable)) t.Errorf("handlers count = %d, expected at least 50", len(table))
} }
} }
@@ -191,8 +192,9 @@ func TestHandlerTableNilSession(t *testing.T) {
// but doesn't call handlers (which would require a real session) // but doesn't call handlers (which would require a real session)
_ = createMockServer() _ = createMockServer()
table := buildHandlerTable()
count := 0 count := 0
for range handlerTable { for range table {
count++ count++
} }

View File

@@ -7,10 +7,10 @@ import (
type handlerFunc func(s *Session, p mhfpacket.MHFPacket) type handlerFunc func(s *Session, p mhfpacket.MHFPacket)
var handlerTable map[network.PacketID]handlerFunc // buildHandlerTable constructs and returns the handler table mapping packet IDs
// to their handler functions. Called once during server construction.
func init() { func buildHandlerTable() map[network.PacketID]handlerFunc {
handlerTable = make(map[network.PacketID]handlerFunc) handlerTable := make(map[network.PacketID]handlerFunc)
handlerTable[network.MSG_HEAD] = handleMsgHead handlerTable[network.MSG_HEAD] = handleMsgHead
handlerTable[network.MSG_SYS_reserve01] = handleMsgSysReserve01 handlerTable[network.MSG_SYS_reserve01] = handleMsgSysReserve01
handlerTable[network.MSG_SYS_reserve02] = handleMsgSysReserve02 handlerTable[network.MSG_SYS_reserve02] = handleMsgSysReserve02
@@ -443,4 +443,5 @@ func init() {
handlerTable[network.MSG_SYS_reserve1AD] = handleMsgSysReserve1AD handlerTable[network.MSG_SYS_reserve1AD] = handleMsgSysReserve1AD
handlerTable[network.MSG_SYS_reserve1AE] = handleMsgSysReserve1AE handlerTable[network.MSG_SYS_reserve1AE] = handleMsgSysReserve1AE
handlerTable[network.MSG_SYS_reserve1AF] = handleMsgSysReserve1AF handlerTable[network.MSG_SYS_reserve1AF] = handleMsgSysReserve1AF
return handlerTable
} }

View File

@@ -7,23 +7,26 @@ import (
) )
func TestHandlerTableInitialized(t *testing.T) { func TestHandlerTableInitialized(t *testing.T) {
if handlerTable == nil { table := buildHandlerTable()
t.Fatal("handlerTable should be initialized by init()") if table == nil {
t.Fatal("buildHandlerTable() should return a non-nil map")
} }
} }
func TestHandlerTableHasEntries(t *testing.T) { func TestHandlerTableHasEntries(t *testing.T) {
if len(handlerTable) == 0 { table := buildHandlerTable()
if len(table) == 0 {
t.Error("handlerTable should have entries") t.Error("handlerTable should have entries")
} }
// Should have many handlers // Should have many handlers
if len(handlerTable) < 100 { if len(table) < 100 {
t.Errorf("handlerTable has %d entries, expected 100+", len(handlerTable)) t.Errorf("handlerTable has %d entries, expected 100+", len(table))
} }
} }
func TestHandlerTableSystemPackets(t *testing.T) { func TestHandlerTableSystemPackets(t *testing.T) {
table := buildHandlerTable()
// Test that key system packets have handlers // Test that key system packets have handlers
systemPackets := []network.PacketID{ systemPackets := []network.PacketID{
network.MSG_HEAD, network.MSG_HEAD,
@@ -38,7 +41,7 @@ func TestHandlerTableSystemPackets(t *testing.T) {
for _, opcode := range systemPackets { for _, opcode := range systemPackets {
t.Run(opcode.String(), func(t *testing.T) { t.Run(opcode.String(), func(t *testing.T) {
if _, ok := handlerTable[opcode]; !ok { if _, ok := table[opcode]; !ok {
t.Errorf("handler missing for %s", opcode) t.Errorf("handler missing for %s", opcode)
} }
}) })
@@ -46,6 +49,7 @@ func TestHandlerTableSystemPackets(t *testing.T) {
} }
func TestHandlerTableStagePackets(t *testing.T) { func TestHandlerTableStagePackets(t *testing.T) {
table := buildHandlerTable()
// Test stage-related packet handlers // Test stage-related packet handlers
stagePackets := []network.PacketID{ stagePackets := []network.PacketID{
network.MSG_SYS_CREATE_STAGE, network.MSG_SYS_CREATE_STAGE,
@@ -60,7 +64,7 @@ func TestHandlerTableStagePackets(t *testing.T) {
for _, opcode := range stagePackets { for _, opcode := range stagePackets {
t.Run(opcode.String(), func(t *testing.T) { t.Run(opcode.String(), func(t *testing.T) {
if _, ok := handlerTable[opcode]; !ok { if _, ok := table[opcode]; !ok {
t.Errorf("handler missing for stage packet %s", opcode) t.Errorf("handler missing for stage packet %s", opcode)
} }
}) })
@@ -68,6 +72,7 @@ func TestHandlerTableStagePackets(t *testing.T) {
} }
func TestHandlerTableBinaryPackets(t *testing.T) { func TestHandlerTableBinaryPackets(t *testing.T) {
table := buildHandlerTable()
// Test binary message handlers // Test binary message handlers
binaryPackets := []network.PacketID{ binaryPackets := []network.PacketID{
network.MSG_SYS_CAST_BINARY, network.MSG_SYS_CAST_BINARY,
@@ -78,7 +83,7 @@ func TestHandlerTableBinaryPackets(t *testing.T) {
for _, opcode := range binaryPackets { for _, opcode := range binaryPackets {
t.Run(opcode.String(), func(t *testing.T) { t.Run(opcode.String(), func(t *testing.T) {
if _, ok := handlerTable[opcode]; !ok { if _, ok := table[opcode]; !ok {
t.Errorf("handler missing for binary packet %s", opcode) t.Errorf("handler missing for binary packet %s", opcode)
} }
}) })
@@ -86,6 +91,7 @@ func TestHandlerTableBinaryPackets(t *testing.T) {
} }
func TestHandlerTableReservedPackets(t *testing.T) { func TestHandlerTableReservedPackets(t *testing.T) {
table := buildHandlerTable()
// Reserved packets should still have handlers (usually no-ops) // Reserved packets should still have handlers (usually no-ops)
reservedPackets := []network.PacketID{ reservedPackets := []network.PacketID{
network.MSG_SYS_reserve01, network.MSG_SYS_reserve01,
@@ -99,7 +105,7 @@ func TestHandlerTableReservedPackets(t *testing.T) {
for _, opcode := range reservedPackets { for _, opcode := range reservedPackets {
t.Run(opcode.String(), func(t *testing.T) { t.Run(opcode.String(), func(t *testing.T) {
if _, ok := handlerTable[opcode]; !ok { if _, ok := table[opcode]; !ok {
t.Errorf("handler missing for reserved packet %s", opcode) t.Errorf("handler missing for reserved packet %s", opcode)
} }
}) })
@@ -107,8 +113,9 @@ func TestHandlerTableReservedPackets(t *testing.T) {
} }
func TestHandlerFuncType(t *testing.T) { func TestHandlerFuncType(t *testing.T) {
table := buildHandlerTable()
// Verify all handlers are valid functions // Verify all handlers are valid functions
for opcode, handler := range handlerTable { for opcode, handler := range table {
if handler == nil { if handler == nil {
t.Errorf("handler for %s is nil", opcode) t.Errorf("handler for %s is nil", opcode)
} }
@@ -116,6 +123,7 @@ func TestHandlerFuncType(t *testing.T) {
} }
func TestHandlerTableObjectPackets(t *testing.T) { func TestHandlerTableObjectPackets(t *testing.T) {
table := buildHandlerTable()
objectPackets := []network.PacketID{ objectPackets := []network.PacketID{
network.MSG_SYS_ADD_OBJECT, network.MSG_SYS_ADD_OBJECT,
network.MSG_SYS_DEL_OBJECT, network.MSG_SYS_DEL_OBJECT,
@@ -125,7 +133,7 @@ func TestHandlerTableObjectPackets(t *testing.T) {
for _, opcode := range objectPackets { for _, opcode := range objectPackets {
t.Run(opcode.String(), func(t *testing.T) { t.Run(opcode.String(), func(t *testing.T) {
if _, ok := handlerTable[opcode]; !ok { if _, ok := table[opcode]; !ok {
t.Errorf("handler missing for object packet %s", opcode) t.Errorf("handler missing for object packet %s", opcode)
} }
}) })
@@ -133,6 +141,7 @@ func TestHandlerTableObjectPackets(t *testing.T) {
} }
func TestHandlerTableClientPackets(t *testing.T) { func TestHandlerTableClientPackets(t *testing.T) {
table := buildHandlerTable()
clientPackets := []network.PacketID{ clientPackets := []network.PacketID{
network.MSG_SYS_SET_STATUS, network.MSG_SYS_SET_STATUS,
network.MSG_SYS_HIDE_CLIENT, network.MSG_SYS_HIDE_CLIENT,
@@ -141,7 +150,7 @@ func TestHandlerTableClientPackets(t *testing.T) {
for _, opcode := range clientPackets { for _, opcode := range clientPackets {
t.Run(opcode.String(), func(t *testing.T) { t.Run(opcode.String(), func(t *testing.T) {
if _, ok := handlerTable[opcode]; !ok { if _, ok := table[opcode]; !ok {
t.Errorf("handler missing for client packet %s", opcode) t.Errorf("handler missing for client packet %s", opcode)
} }
}) })
@@ -149,6 +158,7 @@ func TestHandlerTableClientPackets(t *testing.T) {
} }
func TestHandlerTableSemaphorePackets(t *testing.T) { func TestHandlerTableSemaphorePackets(t *testing.T) {
table := buildHandlerTable()
semaphorePackets := []network.PacketID{ semaphorePackets := []network.PacketID{
network.MSG_SYS_CREATE_ACQUIRE_SEMAPHORE, network.MSG_SYS_CREATE_ACQUIRE_SEMAPHORE,
network.MSG_SYS_ACQUIRE_SEMAPHORE, network.MSG_SYS_ACQUIRE_SEMAPHORE,
@@ -157,7 +167,7 @@ func TestHandlerTableSemaphorePackets(t *testing.T) {
for _, opcode := range semaphorePackets { for _, opcode := range semaphorePackets {
t.Run(opcode.String(), func(t *testing.T) { t.Run(opcode.String(), func(t *testing.T) {
if _, ok := handlerTable[opcode]; !ok { if _, ok := table[opcode]; !ok {
t.Errorf("handler missing for semaphore packet %s", opcode) t.Errorf("handler missing for semaphore packet %s", opcode)
} }
}) })
@@ -165,6 +175,7 @@ func TestHandlerTableSemaphorePackets(t *testing.T) {
} }
func TestHandlerTableMHFPackets(t *testing.T) { func TestHandlerTableMHFPackets(t *testing.T) {
table := buildHandlerTable()
// Test some core MHF packets have handlers // Test some core MHF packets have handlers
mhfPackets := []network.PacketID{ mhfPackets := []network.PacketID{
network.MSG_MHF_SAVEDATA, network.MSG_MHF_SAVEDATA,
@@ -173,7 +184,7 @@ func TestHandlerTableMHFPackets(t *testing.T) {
for _, opcode := range mhfPackets { for _, opcode := range mhfPackets {
t.Run(opcode.String(), func(t *testing.T) { t.Run(opcode.String(), func(t *testing.T) {
if _, ok := handlerTable[opcode]; !ok { if _, ok := table[opcode]; !ok {
t.Errorf("handler missing for MHF packet %s", opcode) t.Errorf("handler missing for MHF packet %s", opcode)
} }
}) })
@@ -181,6 +192,7 @@ func TestHandlerTableMHFPackets(t *testing.T) {
} }
func TestHandlerTableEnumeratePackets(t *testing.T) { func TestHandlerTableEnumeratePackets(t *testing.T) {
table := buildHandlerTable()
enumPackets := []network.PacketID{ enumPackets := []network.PacketID{
network.MSG_SYS_ENUMERATE_CLIENT, network.MSG_SYS_ENUMERATE_CLIENT,
network.MSG_SYS_ENUMERATE_STAGE, network.MSG_SYS_ENUMERATE_STAGE,
@@ -188,7 +200,7 @@ func TestHandlerTableEnumeratePackets(t *testing.T) {
for _, opcode := range enumPackets { for _, opcode := range enumPackets {
t.Run(opcode.String(), func(t *testing.T) { t.Run(opcode.String(), func(t *testing.T) {
if _, ok := handlerTable[opcode]; !ok { if _, ok := table[opcode]; !ok {
t.Errorf("handler missing for enumerate packet %s", opcode) t.Errorf("handler missing for enumerate packet %s", opcode)
} }
}) })
@@ -196,6 +208,7 @@ func TestHandlerTableEnumeratePackets(t *testing.T) {
} }
func TestHandlerTableLogPackets(t *testing.T) { func TestHandlerTableLogPackets(t *testing.T) {
table := buildHandlerTable()
logPackets := []network.PacketID{ logPackets := []network.PacketID{
network.MSG_SYS_TERMINAL_LOG, network.MSG_SYS_TERMINAL_LOG,
network.MSG_SYS_ISSUE_LOGKEY, network.MSG_SYS_ISSUE_LOGKEY,
@@ -204,7 +217,7 @@ func TestHandlerTableLogPackets(t *testing.T) {
for _, opcode := range logPackets { for _, opcode := range logPackets {
t.Run(opcode.String(), func(t *testing.T) { t.Run(opcode.String(), func(t *testing.T) {
if _, ok := handlerTable[opcode]; !ok { if _, ok := table[opcode]; !ok {
t.Errorf("handler missing for log packet %s", opcode) t.Errorf("handler missing for log packet %s", opcode)
} }
}) })
@@ -212,13 +225,14 @@ func TestHandlerTableLogPackets(t *testing.T) {
} }
func TestHandlerTableFilePackets(t *testing.T) { func TestHandlerTableFilePackets(t *testing.T) {
table := buildHandlerTable()
filePackets := []network.PacketID{ filePackets := []network.PacketID{
network.MSG_SYS_GET_FILE, network.MSG_SYS_GET_FILE,
} }
for _, opcode := range filePackets { for _, opcode := range filePackets {
t.Run(opcode.String(), func(t *testing.T) { t.Run(opcode.String(), func(t *testing.T) {
if _, ok := handlerTable[opcode]; !ok { if _, ok := table[opcode]; !ok {
t.Errorf("handler missing for file packet %s", opcode) t.Errorf("handler missing for file packet %s", opcode)
} }
}) })
@@ -226,12 +240,14 @@ func TestHandlerTableFilePackets(t *testing.T) {
} }
func TestHandlerTableEchoPacket(t *testing.T) { func TestHandlerTableEchoPacket(t *testing.T) {
if _, ok := handlerTable[network.MSG_SYS_ECHO]; !ok { table := buildHandlerTable()
if _, ok := table[network.MSG_SYS_ECHO]; !ok {
t.Error("handler missing for MSG_SYS_ECHO") t.Error("handler missing for MSG_SYS_ECHO")
} }
} }
func TestHandlerTableReserveStagePackets(t *testing.T) { func TestHandlerTableReserveStagePackets(t *testing.T) {
table := buildHandlerTable()
reservePackets := []network.PacketID{ reservePackets := []network.PacketID{
network.MSG_SYS_RESERVE_STAGE, network.MSG_SYS_RESERVE_STAGE,
network.MSG_SYS_UNRESERVE_STAGE, network.MSG_SYS_UNRESERVE_STAGE,
@@ -241,7 +257,7 @@ func TestHandlerTableReserveStagePackets(t *testing.T) {
for _, opcode := range reservePackets { for _, opcode := range reservePackets {
t.Run(opcode.String(), func(t *testing.T) { t.Run(opcode.String(), func(t *testing.T) {
if _, ok := handlerTable[opcode]; !ok { if _, ok := table[opcode]; !ok {
t.Errorf("handler missing for reserve stage packet %s", opcode) t.Errorf("handler missing for reserve stage packet %s", opcode)
} }
}) })
@@ -249,14 +265,16 @@ func TestHandlerTableReserveStagePackets(t *testing.T) {
} }
func TestHandlerTableThresholdPacket(t *testing.T) { func TestHandlerTableThresholdPacket(t *testing.T) {
if _, ok := handlerTable[network.MSG_SYS_EXTEND_THRESHOLD]; !ok { table := buildHandlerTable()
if _, ok := table[network.MSG_SYS_EXTEND_THRESHOLD]; !ok {
t.Error("handler missing for MSG_SYS_EXTEND_THRESHOLD") t.Error("handler missing for MSG_SYS_EXTEND_THRESHOLD")
} }
} }
func TestHandlerTableNoNilValues(t *testing.T) { func TestHandlerTableNoNilValues(t *testing.T) {
table := buildHandlerTable()
nilCount := 0 nilCount := 0
for opcode, handler := range handlerTable { for opcode, handler := range table {
if handler == nil { if handler == nil {
nilCount++ nilCount++
t.Errorf("nil handler for opcode %s", opcode) t.Errorf("nil handler for opcode %s", opcode)

View File

@@ -8,6 +8,7 @@ import (
"erupe-ce/common/byteframe" "erupe-ce/common/byteframe"
_config "erupe-ce/config" _config "erupe-ce/config"
"erupe-ce/network"
"erupe-ce/network/binpacket" "erupe-ce/network/binpacket"
"erupe-ce/network/mhfpacket" "erupe-ce/network/mhfpacket"
"erupe-ce/server/discordbot" "erupe-ce/server/discordbot"
@@ -81,6 +82,8 @@ type Server struct {
questCacheLock sync.RWMutex questCacheLock sync.RWMutex
questCacheData map[int][]byte questCacheData map[int][]byte
questCacheTime map[int]time.Time questCacheTime map[int]time.Time
handlerTable map[network.PacketID]handlerFunc
} }
// NewServer creates a new Server type. // NewServer creates a new Server type.
@@ -109,6 +112,7 @@ func NewServer(config *Config) *Server {
}, },
questCacheData: make(map[int][]byte), questCacheData: make(map[int][]byte),
questCacheTime: make(map[int]time.Time), questCacheTime: make(map[int]time.Time),
handlerTable: buildHandlerTable(),
} }
// Mezeporta // Mezeporta

View File

@@ -81,7 +81,7 @@ func NewSession(server *Server, conn net.Conn) *Session {
logger: server.logger.Named(conn.RemoteAddr().String()), logger: server.logger.Named(conn.RemoteAddr().String()),
server: server, server: server,
rawConn: conn, rawConn: conn,
cryptConn: network.NewCryptConn(conn, server.erupeConfig.RealClientMode), cryptConn: network.NewCryptConn(conn, server.erupeConfig.RealClientMode, server.logger.Named(conn.RemoteAddr().String())),
sendPackets: make(chan packet, 20), sendPackets: make(chan packet, 20),
clientContext: &clientctx.ClientContext{RealClientMode: server.erupeConfig.RealClientMode}, clientContext: &clientctx.ClientContext{RealClientMode: server.erupeConfig.RealClientMode},
lastPacket: time.Now(), lastPacket: time.Now(),
@@ -257,7 +257,12 @@ func (s *Session) handlePacketGroup(pktGroup []byte) {
return return
} }
// Handle the packet. // Handle the packet.
handlerTable[opcode](s, mhfPkt) handler, ok := s.server.handlerTable[opcode]
if !ok {
s.logger.Warn("No handler for opcode", zap.Stringer("opcode", opcode))
return
}
handler(s, mhfPkt)
// If there is more data on the stream that the .Parse method didn't read, then read another packet off it. // If there is more data on the stream that the .Parse method didn't read, then read another packet off it.
remainingData := bf.DataFromCurrent() remainingData := bf.DataFromCurrent()
if len(remainingData) >= 2 { if len(remainingData) >= 2 {

View File

@@ -38,10 +38,11 @@ func (m *mockPacket) Parse(bf *byteframe.ByteFrame, ctx *clientctx.ClientContext
func createMockServer() *Server { func createMockServer() *Server {
logger, _ := zap.NewDevelopment() logger, _ := zap.NewDevelopment()
s := &Server{ s := &Server{
logger: logger, logger: logger,
erupeConfig: &_config.Config{}, erupeConfig: &_config.Config{},
stages: make(map[string]*Stage), stages: make(map[string]*Stage),
sessions: make(map[net.Conn]*Session), sessions: make(map[net.Conn]*Session),
handlerTable: buildHandlerTable(),
raviente: &Raviente{ raviente: &Raviente{
register: make([]uint32, 30), register: make([]uint32, 30),
state: make([]uint32, 30), state: make([]uint32, 30),