fix(handlers): harden gacha_items and boost time limit ACK paths

Both bugs surfaced when running protbot against a live Erupe instance.

LoadColumnWithDefault now treats an empty bytea ('\x', len 0) the same
as NULL. The postgres driver returns a non-nil empty slice for empty
bytea, so the prior `data == nil` check let a zero-byte slice reach
handleMsgMhfReceiveGachaItem, which forwarded it to the client as a
malformed gacha_items response. The MHF client interprets a zero-byte
count field as a protocol error and crashes the gacha menu, matching
the #175 symptom class. The handler also gains a defensive fallback
for len(data) == 0 in case another caller hits the same edge.

handleMsgMhfGetBoostTimeLimit was sending two ACKs for a single
request: doAckBufSucceed with the real payload, then an unconditional
doAckSimpleSucceed on the same ack handle. The second ACK was dead on
arrival (the handle was already consumed) but is a latent protocol
bug. Drop it and update the regression test that was asserting the
buggy 2-packet behavior.
This commit is contained in:
Houmgaor
2026-04-06 18:04:10 +02:00
parent 3e9f3d1b62
commit 8b2667f7a0
5 changed files with 34 additions and 6 deletions

View File

@@ -227,7 +227,6 @@ func handleMsgMhfGetBoostTimeLimit(s *Session, p mhfpacket.MHFPacket) {
bf.WriteUint32(uint32(boostLimit.Unix())) bf.WriteUint32(uint32(boostLimit.Unix()))
} }
doAckBufSucceed(s, pkt.AckHandle, bf.Data()) doAckBufSucceed(s, pkt.AckHandle, bf.Data())
doAckSimpleSucceed(s, pkt.AckHandle, make([]byte, 4))
} }
func handleMsgMhfGetBoostRight(s *Session, p mhfpacket.MHFPacket) { func handleMsgMhfGetBoostRight(s *Session, p mhfpacket.MHFPacket) {

View File

@@ -231,7 +231,9 @@ func TestHandleMsgMhfGetBoostTimeLimit(t *testing.T) {
handleMsgMhfGetBoostTimeLimit(session, pkt) handleMsgMhfGetBoostTimeLimit(session, pkt)
// This handler sends two responses (doAckBufSucceed + doAckSimpleSucceed) // One response: the stray second ACK (doAckSimpleSucceed) was removed
// because it fired with the same ack handle as the real buf ACK, which
// is a protocol bug.
count := 0 count := 0
for { for {
select { select {
@@ -242,8 +244,8 @@ func TestHandleMsgMhfGetBoostTimeLimit(t *testing.T) {
} }
} }
done: done:
if count != 2 { if count != 1 {
t.Errorf("Expected 2 response packets, got %d", count) t.Errorf("Expected 1 response packet, got %d", count)
} }
} }

View File

@@ -84,7 +84,9 @@ func handleMsgMhfUseGachaPoint(s *Session, p mhfpacket.MHFPacket) {
func handleMsgMhfReceiveGachaItem(s *Session, p mhfpacket.MHFPacket) { func handleMsgMhfReceiveGachaItem(s *Session, p mhfpacket.MHFPacket) {
pkt := p.(*mhfpacket.MsgMhfReceiveGachaItem) pkt := p.(*mhfpacket.MsgMhfReceiveGachaItem)
data, err := s.server.charRepo.LoadColumnWithDefault(s.charID, "gacha_items", []byte{0x00}) data, err := s.server.charRepo.LoadColumnWithDefault(s.charID, "gacha_items", []byte{0x00})
if err != nil { if err != nil || len(data) == 0 {
// The client requires at least one byte (the item count) or it
// treats the response as malformed and crashes (see #175).
data = []byte{0x00} data = []byte{0x00}
} }

View File

@@ -167,7 +167,11 @@ func (r *CharacterRepository) LoadColumnWithDefault(charID uint32, column string
if err != nil { if err != nil {
return defaultVal, err return defaultVal, err
} }
if data == nil { // Treat empty bytea ('\x', len 0) the same as NULL. The postgres driver
// returns a non-nil empty slice for empty bytea, so a bare `data == nil`
// check would send zero bytes to the client — which the MHF client
// interprets as a malformed response and crashes on (see #175).
if len(data) == 0 {
return defaultVal, nil return defaultVal, nil
} }
return data, nil return data, nil

View File

@@ -392,6 +392,27 @@ func TestLoadColumnWithDefaultExistingData(t *testing.T) {
} }
} }
// TestLoadColumnWithDefaultEmptyBytea verifies that an empty bytea ('\x', len 0)
// is treated the same as NULL and returns the default value. Without this, the
// postgres driver returns a non-nil empty slice that would reach the client
// as a zero-byte ACK payload and crash the MHF gacha menu (see #175).
func TestLoadColumnWithDefaultEmptyBytea(t *testing.T) {
repo, db, charID := setupCharRepo(t)
if _, err := db.Exec("UPDATE characters SET skin_hist=''::bytea WHERE id=$1", charID); err != nil {
t.Fatalf("Setup failed: %v", err)
}
defaultVal := []byte{0x00, 0x01, 0x02}
got, err := repo.LoadColumnWithDefault(charID, "skin_hist", defaultVal)
if err != nil {
t.Fatalf("LoadColumnWithDefault failed: %v", err)
}
if len(got) != 3 || got[0] != 0x00 || got[2] != 0x02 {
t.Errorf("Expected default value for empty bytea, got: %x", got)
}
}
func TestSetDeleted(t *testing.T) { func TestSetDeleted(t *testing.T) {
repo, db, charID := setupCharRepo(t) repo, db, charID := setupCharRepo(t)