From d27da5ec86381059da6360bd5d88e42540854a5e Mon Sep 17 00:00:00 2001 From: Houmgaor Date: Thu, 19 Mar 2026 14:35:38 +0100 Subject: [PATCH] fix(items): stop G-rank Workshop/Cog softlock on missing ACK MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MSG_MHF_GET_EXTRA_INFO (0xA6) and MSG_MHF_GET_COG_INFO (0xC3) had Parse() returning NOT IMPLEMENTED. The dispatch loop treats any Parse error as a hard drop — no ACK is ever sent, so the client waits indefinitely and effectively soft-locks when entering the G-rank Workshop or Master Felyne (Cog) screens. Fix: parse AckHandle (the only field we can confirm from the protocol) and respond with doAckBufFail so the client receives a well-formed buf-type ACK with error code 1. The client's fail branch for these requests exits cleanly without reading response fields, avoiding the read-past-EOF crash that an empty success ACK would cause. The full response format for both packets is still unknown; a complete implementation requires further RE. The TODO comments mark the gap. Fixes #180. --- docs/unimplemented.md | 4 +-- network/mhfpacket/msg_batch_parse_test.go | 1 - network/mhfpacket/msg_comprehensive_test.go | 2 ++ network/mhfpacket/msg_mhf_get_cog_info.go | 7 ++-- network/mhfpacket/msg_mhf_get_extra_info.go | 7 ++-- network/mhfpacket/msg_parse_small_test.go | 33 ++++++++++++++++++- server/channelserver/handlers_items.go | 12 +++++-- server/channelserver/handlers_items_test.go | 9 ++--- server/channelserver/handlers_misc_test.go | 11 ++----- server/channelserver/handlers_session_test.go | 2 -- server/channelserver/handlers_simple_test.go | 2 -- 11 files changed, 60 insertions(+), 30 deletions(-) diff --git a/docs/unimplemented.md b/docs/unimplemented.md index 1eb828887..130fdf8ad 100644 --- a/docs/unimplemented.md +++ b/docs/unimplemented.md @@ -15,7 +15,7 @@ All empty handlers carry an inline comment — `// stub: unimplemented` for real --- -## Unimplemented (70 handlers) +## Unimplemented (68 handlers) Grouped by handler file / game subsystem. @@ -77,8 +77,6 @@ Grouped by handler file / game subsystem. | Handler | Notes | |---------|-------| -| `handleMsgMhfGetExtraInfo` | Fetch supplemental item/character info | -| `handleMsgMhfGetCogInfo` | Fetch Cog (partner Felyne) information | | `handleMsgMhfStampcardPrize` | Claim a stamp card prize | ### Misc (`handlers_misc.go`) diff --git a/network/mhfpacket/msg_batch_parse_test.go b/network/mhfpacket/msg_batch_parse_test.go index c7715fa23..ea5225900 100644 --- a/network/mhfpacket/msg_batch_parse_test.go +++ b/network/mhfpacket/msg_batch_parse_test.go @@ -1594,7 +1594,6 @@ func TestBatchParseNotImplemented(t *testing.T) { &MsgSysDispObject{}, &MsgSysHideObject{}, &MsgMhfServerCommand{}, &MsgMhfSetLoginwindow{}, &MsgMhfShutClient{}, &MsgMhfUpdateGuildcard{}, - &MsgMhfGetCogInfo{}, &MsgCaExchangeItem{}, } diff --git a/network/mhfpacket/msg_comprehensive_test.go b/network/mhfpacket/msg_comprehensive_test.go index 67701e1c3..db9f51f75 100644 --- a/network/mhfpacket/msg_comprehensive_test.go +++ b/network/mhfpacket/msg_comprehensive_test.go @@ -495,6 +495,8 @@ func TestAckHandlePacketsParse(t *testing.T) { {"MsgMhfGetUdSchedule", network.MSG_MHF_GET_UD_SCHEDULE}, {"MsgMhfGetUdInfo", network.MSG_MHF_GET_UD_INFO}, {"MsgMhfGetKijuInfo", network.MSG_MHF_GET_KIJU_INFO}, + {"MsgMhfGetExtraInfo", network.MSG_MHF_GET_EXTRA_INFO}, + {"MsgMhfGetCogInfo", network.MSG_MHF_GET_COG_INFO}, } ctx := &clientctx.ClientContext{RealClientMode: cfg.ZZ} diff --git a/network/mhfpacket/msg_mhf_get_cog_info.go b/network/mhfpacket/msg_mhf_get_cog_info.go index ad5703542..3840d9101 100644 --- a/network/mhfpacket/msg_mhf_get_cog_info.go +++ b/network/mhfpacket/msg_mhf_get_cog_info.go @@ -9,7 +9,9 @@ import ( ) // MsgMhfGetCogInfo represents the MSG_MHF_GET_COG_INFO -type MsgMhfGetCogInfo struct{} +type MsgMhfGetCogInfo struct { + AckHandle uint32 +} // Opcode returns the ID associated with this packet type. func (m *MsgMhfGetCogInfo) Opcode() network.PacketID { @@ -18,7 +20,8 @@ func (m *MsgMhfGetCogInfo) Opcode() network.PacketID { // Parse parses the packet from binary func (m *MsgMhfGetCogInfo) Parse(bf *byteframe.ByteFrame, ctx *clientctx.ClientContext) error { - return errors.New("NOT IMPLEMENTED") + m.AckHandle = bf.ReadUint32() + return nil } // Build builds a binary packet from the current data. diff --git a/network/mhfpacket/msg_mhf_get_extra_info.go b/network/mhfpacket/msg_mhf_get_extra_info.go index a8c05776a..272865f0c 100644 --- a/network/mhfpacket/msg_mhf_get_extra_info.go +++ b/network/mhfpacket/msg_mhf_get_extra_info.go @@ -9,7 +9,9 @@ import ( ) // MsgMhfGetExtraInfo represents the MSG_MHF_GET_EXTRA_INFO -type MsgMhfGetExtraInfo struct{} +type MsgMhfGetExtraInfo struct { + AckHandle uint32 +} // Opcode returns the ID associated with this packet type. func (m *MsgMhfGetExtraInfo) Opcode() network.PacketID { @@ -18,7 +20,8 @@ func (m *MsgMhfGetExtraInfo) Opcode() network.PacketID { // Parse parses the packet from binary func (m *MsgMhfGetExtraInfo) Parse(bf *byteframe.ByteFrame, ctx *clientctx.ClientContext) error { - return errors.New("NOT IMPLEMENTED") + m.AckHandle = bf.ReadUint32() + return nil } // Build builds a binary packet from the current data. diff --git a/network/mhfpacket/msg_parse_small_test.go b/network/mhfpacket/msg_parse_small_test.go index df723524f..b27524d16 100644 --- a/network/mhfpacket/msg_parse_small_test.go +++ b/network/mhfpacket/msg_parse_small_test.go @@ -25,7 +25,6 @@ func TestParseSmallNotImplemented(t *testing.T) { {"MsgMhfGetCaUniqueID", &MsgMhfGetCaUniqueID{}}, {"MsgMhfGetDailyMissionMaster", &MsgMhfGetDailyMissionMaster{}}, {"MsgMhfGetDailyMissionPersonal", &MsgMhfGetDailyMissionPersonal{}}, - {"MsgMhfGetExtraInfo", &MsgMhfGetExtraInfo{}}, {"MsgMhfGetRestrictionEvent", &MsgMhfGetRestrictionEvent{}}, {"MsgMhfKickExportForce", &MsgMhfKickExportForce{}}, {"MsgMhfPaymentAchievement", &MsgMhfPaymentAchievement{}}, @@ -196,6 +195,38 @@ func TestParseSmallEnumerateHouse(t *testing.T) { }) } +// TestParseSmallGetExtraInfoAndCogInfo tests that MsgMhfGetExtraInfo and +// MsgMhfGetCogInfo correctly parse their AckHandle field. +func TestParseSmallGetExtraInfoAndCogInfo(t *testing.T) { + ctx := &clientctx.ClientContext{RealClientMode: cfg.ZZ} + + t.Run("GetExtraInfo", func(t *testing.T) { + bf := byteframe.NewByteFrame() + bf.WriteUint32(0xDEADBEEF) + _, _ = bf.Seek(0, io.SeekStart) + pkt := &MsgMhfGetExtraInfo{} + if err := pkt.Parse(bf, ctx); err != nil { + t.Fatalf("Parse() error = %v", err) + } + if pkt.AckHandle != 0xDEADBEEF { + t.Errorf("AckHandle = 0x%X, want 0xDEADBEEF", pkt.AckHandle) + } + }) + + t.Run("GetCogInfo", func(t *testing.T) { + bf := byteframe.NewByteFrame() + bf.WriteUint32(0xCAFEBABE) + _, _ = bf.Seek(0, io.SeekStart) + pkt := &MsgMhfGetCogInfo{} + if err := pkt.Parse(bf, ctx); err != nil { + t.Fatalf("Parse() error = %v", err) + } + if pkt.AckHandle != 0xCAFEBABE { + t.Errorf("AckHandle = 0x%X, want 0xCAFEBABE", pkt.AckHandle) + } + }) +} + // TestParseSmallNotImplementedDoesNotPanic ensures that calling Parse on NOT IMPLEMENTED // packets returns an error and does not panic. func TestParseSmallNotImplementedDoesNotPanic(t *testing.T) { diff --git a/server/channelserver/handlers_items.go b/server/channelserver/handlers_items.go index 23c3eb6c4..83ab57b5b 100644 --- a/server/channelserver/handlers_items.go +++ b/server/channelserver/handlers_items.go @@ -55,7 +55,11 @@ func handleMsgMhfEnumerateOrder(s *Session, p mhfpacket.MHFPacket) { stubEnumerateNoResults(s, pkt.AckHandle) } -func handleMsgMhfGetExtraInfo(s *Session, p mhfpacket.MHFPacket) {} // stub: unimplemented +func handleMsgMhfGetExtraInfo(s *Session, p mhfpacket.MHFPacket) { + pkt := p.(*mhfpacket.MsgMhfGetExtraInfo) + // TODO: response structure unknown; fail ACK prevents softlock without misleading client + doAckBufFail(s, pkt.AckHandle, nil) +} func userGetItems(s *Session) []mhfitem.MHFItemStack { var items []mhfitem.MHFItemStack @@ -91,7 +95,11 @@ func handleMsgMhfUpdateUnionItem(s *Session, p mhfpacket.MHFPacket) { doAckSimpleSucceed(s, pkt.AckHandle, make([]byte, 4)) } -func handleMsgMhfGetCogInfo(s *Session, p mhfpacket.MHFPacket) {} // stub: unimplemented +func handleMsgMhfGetCogInfo(s *Session, p mhfpacket.MHFPacket) { + pkt := p.(*mhfpacket.MsgMhfGetCogInfo) + // TODO: response structure unknown; fail ACK prevents softlock without misleading client + doAckBufFail(s, pkt.AckHandle, nil) +} func handleMsgMhfCheckWeeklyStamp(s *Session, p mhfpacket.MHFPacket) { pkt := p.(*mhfpacket.MsgMhfCheckWeeklyStamp) diff --git a/server/channelserver/handlers_items_test.go b/server/channelserver/handlers_items_test.go index 48d68a40e..e4bde8e71 100644 --- a/server/channelserver/handlers_items_test.go +++ b/server/channelserver/handlers_items_test.go @@ -494,13 +494,8 @@ func TestHandleMsgMhfGetExtraInfo(t *testing.T) { server := createMockServer() session := createMockSession(1, server) - defer func() { - if r := recover(); r != nil { - t.Errorf("handleMsgMhfGetExtraInfo panicked: %v", r) - } - }() - - handleMsgMhfGetExtraInfo(session, nil) + pkt := &mhfpacket.MsgMhfGetExtraInfo{AckHandle: 1} + handleMsgMhfGetExtraInfo(session, pkt) } func TestHandleMsgMhfTransferItem(t *testing.T) { diff --git a/server/channelserver/handlers_misc_test.go b/server/channelserver/handlers_misc_test.go index 71baac561..4a4631b18 100644 --- a/server/channelserver/handlers_misc_test.go +++ b/server/channelserver/handlers_misc_test.go @@ -336,13 +336,8 @@ func TestHandleMsgMhfGetCogInfo(t *testing.T) { server := createMockServer() session := createMockSession(1, server) - defer func() { - if r := recover(); r != nil { - t.Errorf("handleMsgMhfGetCogInfo panicked: %v", r) - } - }() - - handleMsgMhfGetCogInfo(session, nil) + pkt := &mhfpacket.MsgMhfGetCogInfo{AckHandle: 1} + handleMsgMhfGetCogInfo(session, pkt) } // Additional handler tests for coverage @@ -1032,7 +1027,7 @@ func TestEmptyHandlers_MiscFiles_Misc(t *testing.T) { name string fn func() }{ - {"handleMsgMhfGetCogInfo", func() { handleMsgMhfGetCogInfo(session, nil) }}, + {"handleMsgMhfUseUdShopCoin", func() { handleMsgMhfUseUdShopCoin(session, nil) }}, {"handleMsgMhfGetDailyMissionMaster", func() { handleMsgMhfGetDailyMissionMaster(session, nil) }}, {"handleMsgMhfGetDailyMissionPersonal", func() { handleMsgMhfGetDailyMissionPersonal(session, nil) }}, diff --git a/server/channelserver/handlers_session_test.go b/server/channelserver/handlers_session_test.go index 161339c71..ebe5cf743 100644 --- a/server/channelserver/handlers_session_test.go +++ b/server/channelserver/handlers_session_test.go @@ -1079,7 +1079,6 @@ func TestEmptyHandlers_HandlersGo(t *testing.T) { {"handleMsgSysEnumuser", func() { handleMsgSysEnumuser(session, nil) }}, {"handleMsgSysInfokyserver", func() { handleMsgSysInfokyserver(session, nil) }}, {"handleMsgMhfGetCaUniqueID", func() { handleMsgMhfGetCaUniqueID(session, nil) }}, - {"handleMsgMhfGetExtraInfo", func() { handleMsgMhfGetExtraInfo(session, nil) }}, {"handleMsgSysSetStatus", func() { handleMsgSysSetStatus(session, nil) }}, {"handleMsgMhfStampcardPrize", func() { handleMsgMhfStampcardPrize(session, nil) }}, {"handleMsgMhfKickExportForce", func() { handleMsgMhfKickExportForce(session, nil) }}, @@ -1118,7 +1117,6 @@ func TestEmptyHandlers_Concurrent(t *testing.T) { handleMsgSysEnumuser, handleMsgSysInfokyserver, handleMsgMhfGetCaUniqueID, - handleMsgMhfGetExtraInfo, handleMsgSysSetStatus, handleMsgSysDeleteObject, handleMsgSysRotateObject, diff --git a/server/channelserver/handlers_simple_test.go b/server/channelserver/handlers_simple_test.go index 64a284569..4aef45aa2 100644 --- a/server/channelserver/handlers_simple_test.go +++ b/server/channelserver/handlers_simple_test.go @@ -290,8 +290,6 @@ func TestEmptyHandlers_NoDb(t *testing.T) { {"handleMsgSysEnumuser", handleMsgSysEnumuser}, {"handleMsgSysInfokyserver", handleMsgSysInfokyserver}, {"handleMsgMhfGetCaUniqueID", handleMsgMhfGetCaUniqueID}, - {"handleMsgMhfGetExtraInfo", handleMsgMhfGetExtraInfo}, - {"handleMsgMhfGetCogInfo", handleMsgMhfGetCogInfo}, {"handleMsgMhfStampcardPrize", handleMsgMhfStampcardPrize}, {"handleMsgMhfKickExportForce", handleMsgMhfKickExportForce}, {"handleMsgSysSetStatus", handleMsgSysSetStatus},