From 10e09630a60e17a95e759fd4c34b68dd4adbce1b Mon Sep 17 00:00:00 2001 From: Houmgaor Date: Mon, 16 Feb 2026 18:35:44 +0100 Subject: [PATCH] fix: send failure ack for missing quest/scenario files instead of crashing client When a quest or scenario file was missing, handleMsgSysGetFile sent nil data via doAckBufSucceed, which crashed the game client. Now sends doAckBufFail so the client can handle the missing file gracefully. Closes #109 --- CHANGELOG.md | 1 + server/channelserver/handlers_quest.go | 6 +- server/channelserver/handlers_quest_test.go | 95 +++++++++++++++++++++ 3 files changed, 98 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37b8f92d7..3981da685 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed double-save bug in logout flow that caused unnecessary database operations - Fixed save operation ordering - now saves data before session cleanup instead of after - Fixed stale transmog/armor appearance shown to other players - user binary cache now invalidated when plate data is saved +- Fixed client crash when quest or scenario files are missing - now sends failure ack instead of nil data - Fixed server crash when Discord relay receives messages with unsupported Shift-JIS characters (emoji, Lenny faces, cuneiform, etc.) - Fixed data race in token.RNG global used concurrently across goroutines diff --git a/server/channelserver/handlers_quest.go b/server/channelserver/handlers_quest.go index a4188dde7..b1770be4b 100644 --- a/server/channelserver/handlers_quest.go +++ b/server/channelserver/handlers_quest.go @@ -110,8 +110,7 @@ func handleMsgSysGetFile(s *Session, p mhfpacket.MHFPacket) { data, err := os.ReadFile(filepath.Join(s.server.erupeConfig.BinPath, fmt.Sprintf("scenarios/%s.bin", filename))) if err != nil { s.logger.Error(fmt.Sprintf("Failed to open file: %s/scenarios/%s.bin", s.server.erupeConfig.BinPath, filename)) - // This will crash the game. - doAckBufSucceed(s, pkt.AckHandle, data) + doAckBufFail(s, pkt.AckHandle, nil) return } doAckBufSucceed(s, pkt.AckHandle, data) @@ -130,8 +129,7 @@ func handleMsgSysGetFile(s *Session, p mhfpacket.MHFPacket) { data, err := os.ReadFile(filepath.Join(s.server.erupeConfig.BinPath, fmt.Sprintf("quests/%s.bin", pkt.Filename))) if err != nil { s.logger.Error(fmt.Sprintf("Failed to open file: %s/quests/%s.bin", s.server.erupeConfig.BinPath, pkt.Filename)) - // This will crash the game. - doAckBufSucceed(s, pkt.AckHandle, data) + doAckBufFail(s, pkt.AckHandle, nil) return } if _config.ErupeConfig.RealClientMode <= _config.Z1 && s.server.erupeConfig.DebugOptions.AutoQuestBackport { diff --git a/server/channelserver/handlers_quest_test.go b/server/channelserver/handlers_quest_test.go index 8aff59872..e4db7981b 100644 --- a/server/channelserver/handlers_quest_test.go +++ b/server/channelserver/handlers_quest_test.go @@ -5,6 +5,8 @@ import ( "encoding/binary" "erupe-ce/common/byteframe" "erupe-ce/network/mhfpacket" + "os" + "path/filepath" "testing" "time" ) @@ -686,3 +688,96 @@ func BenchmarkBackportQuest(b *testing.B) { _ = BackportQuest(data) } } + +// parseAckFromChannel reads a queued packet from the session's sendPackets channel +// and parses the ErrorCode from the MsgSysAck wire format. +func parseAckFromChannel(t *testing.T, s *Session) (errorCode uint8) { + t.Helper() + select { + case pkt := <-s.sendPackets: + // Wire format: 2 bytes opcode + 4 bytes AckHandle + 1 byte IsBufferResponse + 1 byte ErrorCode + ... + data := pkt.data + if len(data) < 8 { + t.Fatalf("ack packet too short: %d bytes", len(data)) + } + return data[7] // ErrorCode is at offset 7 + case <-time.After(time.Second): + t.Fatal("timed out waiting for ack packet") + return + } +} + +// TestHandleMsgSysGetFile_MissingQuestFile tests that a missing quest file +// sends a failure ack instead of crashing the client with nil data. +func TestHandleMsgSysGetFile_MissingQuestFile(t *testing.T) { + mockConn := &MockCryptConn{sentPackets: make([][]byte, 0)} + s := createTestSession(mockConn) + s.server.erupeConfig.BinPath = t.TempDir() + + pkt := &mhfpacket.MsgSysGetFile{ + AckHandle: 42, + IsScenario: false, + Filename: "d00100d0", + } + + handleMsgSysGetFile(s, pkt) + + errorCode := parseAckFromChannel(t, s) + if errorCode != 1 { + t.Errorf("expected failure ack (ErrorCode=1) for missing quest file, got ErrorCode=%d", errorCode) + } +} + +// TestHandleMsgSysGetFile_MissingScenarioFile tests that a missing scenario file +// sends a failure ack instead of crashing the client with nil data. +func TestHandleMsgSysGetFile_MissingScenarioFile(t *testing.T) { + mockConn := &MockCryptConn{sentPackets: make([][]byte, 0)} + s := createTestSession(mockConn) + s.server.erupeConfig.BinPath = t.TempDir() + + pkt := &mhfpacket.MsgSysGetFile{ + AckHandle: 42, + IsScenario: true, + // ScenarioIdentifer fields default to zero values, producing filename "0_0_0_0_S0_T0_C0" + } + + handleMsgSysGetFile(s, pkt) + + errorCode := parseAckFromChannel(t, s) + if errorCode != 1 { + t.Errorf("expected failure ack (ErrorCode=1) for missing scenario file, got ErrorCode=%d", errorCode) + } +} + +// TestHandleMsgSysGetFile_ExistingQuestFile tests that an existing quest file +// sends a success ack with the file data. +func TestHandleMsgSysGetFile_ExistingQuestFile(t *testing.T) { + mockConn := &MockCryptConn{sentPackets: make([][]byte, 0)} + s := createTestSession(mockConn) + + tmpDir := t.TempDir() + s.server.erupeConfig.BinPath = tmpDir + + // Create the quests directory and a test quest file + questDir := filepath.Join(tmpDir, "quests") + if err := os.MkdirAll(questDir, 0o755); err != nil { + t.Fatalf("failed to create quest dir: %v", err) + } + questData := []byte{0xDE, 0xAD, 0xBE, 0xEF} + if err := os.WriteFile(filepath.Join(questDir, "d00100d0.bin"), questData, 0o644); err != nil { + t.Fatalf("failed to write quest file: %v", err) + } + + pkt := &mhfpacket.MsgSysGetFile{ + AckHandle: 42, + IsScenario: false, + Filename: "d00100d0", + } + + handleMsgSysGetFile(s, pkt) + + errorCode := parseAckFromChannel(t, s) + if errorCode != 0 { + t.Errorf("expected success ack (ErrorCode=0) for existing quest file, got ErrorCode=%d", errorCode) + } +}