diff options
Diffstat (limited to 'internal')
30 files changed, 343 insertions, 54 deletions
diff --git a/internal/admin/api_entity.go b/internal/admin/api_entity.go index 204c59e..04b9dca 100644 --- a/internal/admin/api_entity.go +++ b/internal/admin/api_entity.go @@ -230,6 +230,10 @@ func (s *AdminServer) entityCreate(w http.ResponseWriter, r *http.Request, c ent c.fail(w, http.StatusBadRequest, c.invalidID()) return } + // Editor-only keys never belong in the file (the update path drops them + // too); without this a GET->POST round trip poisons the YAML. + delete(m, "_path") + delete(m, "_raw") if _, _, err := c.find(s, id); err == nil { c.fail(w, http.StatusConflict, c.noun+" already exists") return diff --git a/internal/admin/api_files.go b/internal/admin/api_files.go index a19cb3b..b1b3fff 100644 --- a/internal/admin/api_files.go +++ b/internal/admin/api_files.go @@ -234,8 +234,13 @@ func (s *AdminServer) handleFiles(w http.ResponseWriter, r *http.Request) { return } // A redacted account hash is restored from the file on disk before - // writing, so the raw editor can never disclose or overwrite it. - body = restoreAccountSecret(relPath, fullPath, body) + // writing, so the raw editor can never disclose or overwrite it. A + // non-mapping body is refused rather than blanking the hash. + body, err = restoreAccountSecret(relPath, fullPath, body) + if err != nil { + failJSON(w, err.Error()) + return + } oldContent, existed, err := snapshotBefore(fullPath) if err != nil { @@ -375,31 +380,34 @@ func redactAccountContent(relPath string, data []byte) []byte { // restoreAccountSecret copies the on-disk password hash back into a submitted // account file, so a redacted value (or any client-supplied value) can never -// change or blank an existing hash through the raw file editor. -func restoreAccountSecret(relPath, fullPath string, body []byte) []byte { +// change or blank an existing hash through the raw file editor. It returns an +// error when the existing hash cannot be preserved (for example a non-mapping +// body), so the caller refuses the write instead of blanking the hash. +func restoreAccountSecret(relPath, fullPath string, body []byte) ([]byte, error) { if !isAccountFilePath(relPath) { - return body + return body, nil } existing, err := os.ReadFile(fullPath) if err != nil { - return body + // A missing account file is a create; nothing to preserve. + return body, nil } em, err := yamlToMap(existing) if err != nil { - return body + return nil, fmt.Errorf("existing account file is not a YAML mapping: %w", err) } hash, ok := em["password_hash"] if !ok { - return body + return body, nil } bm, err := yamlToMap(body) if err != nil { - return body + return nil, fmt.Errorf("submitted account file is not a YAML mapping; refusing to blank the password hash") } bm["password_hash"] = hash out, err := marshalMapYAML(bm) if err != nil { - return body + return nil, fmt.Errorf("marshal account file: %w", err) } - return out + return out, nil } diff --git a/internal/admin/api_helpers.go b/internal/admin/api_helpers.go index 37cc4ae..d5e23c3 100644 --- a/internal/admin/api_helpers.go +++ b/internal/admin/api_helpers.go @@ -17,12 +17,17 @@ import ( // JSON bodies, so an unchecked join could otherwise escape the data // directory ("../") or touch dot files. func validEntityID(id string) bool { - if id == "" || id == "." || id == ".." { + if id == "" || id == "." || id == ".." || strings.HasPrefix(id, ".") { return false } if strings.ContainsAny(id, `/\`) || strings.ContainsRune(id, 0) { return false } + for _, r := range id { + if r < 32 || r == 127 { + return false + } + } return true } diff --git a/internal/admin/api_room_courses.go b/internal/admin/api_room_courses.go index db74adf..099ebef 100644 --- a/internal/admin/api_room_courses.go +++ b/internal/admin/api_room_courses.go @@ -420,8 +420,9 @@ func (s *AdminServer) handleRoomAttachCourse(w http.ResponseWriter, r *http.Requ // Detach a room from its course; delete the course file if it becomes empty -// detachRoomFromCourse removes roomID from the course it belongs to, if any. -// Returns the course ID that was modified, or "" if none match. +// detachRoomFromCourse removes roomID from every course it belongs to, if any. +// Returns the first affected course ID (or "" if none match) and whether any +// course was deleted. func (s *AdminServer) detachRoomFromCourse(roomID int) (courseID string, deleted bool) { ids, err := s.listCourseIDs() if err != nil { @@ -448,15 +449,23 @@ func (s *AdminServer) detachRoomFromCourse(roomID int) (courseID string, deleted remaining = append(remaining, obstacles[idx+1:]...) if len(remaining) == 0 { - os.Remove(path) + // A failed unlink must not be reported as a detach: the course + // would keep listing a deleted room. + if err := os.Remove(path); err != nil { + continue + } s.undoStack.Push(ChangeDesc{ Description: fmt.Sprintf("delete course %s (last obstacle removed)", id), FilePath: path, OldContent: oldContent, IsDelete: true, }) + if courseID == "" { + courseID = id + } + deleted = true s.reloadCoursesIfWired() - return id, true + continue } anyList := make([]any, len(remaining)) @@ -466,7 +475,7 @@ func (s *AdminServer) detachRoomFromCourse(roomID int) (courseID string, deleted m["obstacles"] = anyList newContent, err := writeMapAsYAML(path, m) if err != nil { - return "", false + continue } s.undoStack.Push(ChangeDesc{ Description: fmt.Sprintf("detach room %d from course %s", roomID, id), @@ -474,10 +483,12 @@ func (s *AdminServer) detachRoomFromCourse(roomID int) (courseID string, deleted OldContent: oldContent, NewContent: newContent, }) + if courseID == "" { + courseID = id + } s.reloadCoursesIfWired() - return id, false } - return "", false + return courseID, deleted } func (s *AdminServer) handleRoomDetachCourse(w http.ResponseWriter, r *http.Request, roomID int) { diff --git a/internal/admin/correct_audit_fixes_test.go b/internal/admin/correct_audit_fixes_test.go index 381fb5c..60073f7 100644 --- a/internal/admin/correct_audit_fixes_test.go +++ b/internal/admin/correct_audit_fixes_test.go @@ -1,6 +1,11 @@ package admin -import "testing" +import ( + "os" + "path/filepath" + "strings" + "testing" +) // Regression: the raw file API must canonicalize repeated "/" and "data/" // prefixes, or "//players/..." reaches the real file while the classifiers @@ -20,3 +25,27 @@ func TestCanonicalDataRelPathStripsRepeatedPrefixes(t *testing.T) { } } } + +// Regression: a valid-but-non-mapping body used to pass through unchanged and +// blank password_hash. It must now be refused, and a mapping body must keep +// the on-disk hash. +func TestRestoreAccountSecretRefusesNonMappingBody(t *testing.T) { + dir := t.TempDir() + full := filepath.Join(dir, "players", "accounts", "alice.yaml") + if err := os.MkdirAll(filepath.Dir(full), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte("name: alice\npassword_hash: secret\n"), 0600); err != nil { + t.Fatal(err) + } + if _, err := restoreAccountSecret("players/accounts/alice.yaml", full, []byte("# comment only\n")); err == nil { + t.Fatal("non-mapping body accepted; the password hash would be blanked") + } + out, err := restoreAccountSecret("players/accounts/alice.yaml", full, []byte("name: alice\npassword_hash: \"[redacted]\"\n")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(out), "secret") { + t.Fatalf("password hash not preserved: %s", out) + } +} diff --git a/internal/admin/live_owner_test.go b/internal/admin/live_owner_test.go index ae53606..de3ade8 100644 --- a/internal/admin/live_owner_test.go +++ b/internal/admin/live_owner_test.go @@ -178,6 +178,7 @@ func (discardConn) ReadMessage() (string, error) { return "", io.EOF } func (discardConn) Write(p []byte) (int, error) { return len(p), nil } func (discardConn) Close() error { return nil } func (discardConn) SetWriteDeadline(time.Time) error { return nil } +func (discardConn) SetReadDeadline(time.Time) error { return nil } func (discardConn) SetEcho(bool) error { return nil } func newEditorHarness(t *testing.T) (*editorHarness, *gamenet.Session) { diff --git a/internal/behavior/store.go b/internal/behavior/store.go index f2e9350..d31fe84 100644 --- a/internal/behavior/store.go +++ b/internal/behavior/store.go @@ -84,7 +84,11 @@ func WalkYAMLDir(dir string, fn func(path, id string, data []byte) error) error } return err } - if d.IsDir() || !strings.HasSuffix(d.Name(), ".yaml") { + if d.IsDir() { + return nil + } + id, ok := yamlID(d.Name()) + if !ok { return nil } if excludedContentPath(dir, path) { @@ -97,11 +101,23 @@ func WalkYAMLDir(dir string, fn func(path, id string, data []byte) error) error } return readErr } - id := strings.TrimSuffix(d.Name(), ".yaml") return fn(path, id, data) }) } +// yamlID returns the id (filename without extension) for a .yaml or .yml file. +// The runtime trigger loader accepts .yml, so validation discovery must too, +// otherwise such a file loads and fires while every check ignores it. +func yamlID(name string) (string, bool) { + if strings.HasSuffix(name, ".yaml") { + return strings.TrimSuffix(name, ".yaml"), true + } + if strings.HasSuffix(name, ".yml") { + return strings.TrimSuffix(name, ".yml"), true + } + return "", false +} + type Duplicate struct { ID string Paths []string @@ -110,13 +126,16 @@ type Duplicate struct { func CheckDuplicateIDs(dir string) []Duplicate { seen := make(map[string][]string) filepath.WalkDir(dir, func(path string, d os.DirEntry, err error) error { - if err != nil || d.IsDir() || !strings.HasSuffix(d.Name(), ".yaml") { + if err != nil || d.IsDir() { + return nil + } + id, ok := yamlID(d.Name()) + if !ok { return nil } if excludedContentPath(dir, path) { return nil } - id := strings.TrimSuffix(d.Name(), ".yaml") seen[id] = append(seen[id], path) return nil }) diff --git a/internal/behavior/yml_test.go b/internal/behavior/yml_test.go new file mode 100644 index 0000000..9452e9e --- /dev/null +++ b/internal/behavior/yml_test.go @@ -0,0 +1,29 @@ +package behavior + +import ( + "os" + "path/filepath" + "testing" +) + +// Regression: WalkYAMLDir only matched .yaml, so a .yml trigger file loaded +// and fired at runtime while every validation pass ignored it. +func TestWalkYAMLDirIncludesYml(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "a.yaml"), []byte("x: 1"), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "b.yml"), []byte("x: 2"), 0644); err != nil { + t.Fatal(err) + } + ids := map[string]bool{} + if err := WalkYAMLDir(dir, func(path, id string, data []byte) error { + ids[id] = true + return nil + }); err != nil { + t.Fatal(err) + } + if !ids["a"] || !ids["b"] { + t.Fatalf("WalkYAMLDir ids = %v, want both a and b", ids) + } +} diff --git a/internal/casino/correct_audit_fixes_test.go b/internal/casino/correct_audit_fixes_test.go new file mode 100644 index 0000000..e5e3bd3 --- /dev/null +++ b/internal/casino/correct_audit_fixes_test.go @@ -0,0 +1,23 @@ +package casino + +import ( + "math/rand" + "testing" +) + +// Regression: when the shooter leaves and others remain, the next shooter must +// start a fresh come-out rather than inheriting the previous point and +// made/seen bonus progress (which blocked line and bonus bets). +func TestCrapsShooterChangeResetsHand(t *testing.T) { + tb := newCrapsTable(1, TableConfig{ID: "craps", Game: GameCraps, BettingTicks: 2}, rand.New(rand.NewSource(7))) + tb.join("alice") + tb.join("bob") + tb.point = 6 + tb.rolled = true + tb.made = map[int]bool{4: true} + tb.seen = map[int]bool{4: true} + tb.remove("alice") + if tb.point != 0 || tb.rolled || len(tb.made) != 0 || len(tb.seen) != 0 { + t.Fatalf("new shooter inherited hand: point=%d rolled=%v made=%v seen=%v", tb.point, tb.rolled, tb.made, tb.seen) + } +} diff --git a/internal/casino/craps.go b/internal/casino/craps.go index 73c67c4..0abf24b 100644 --- a/internal/casino/craps.go +++ b/internal/casino/craps.go @@ -133,7 +133,8 @@ func (t *crapsTable) join(name string) []Event { return append([]Event{e, t.menu(name)}, timing...) } func (t *crapsTable) remove(name string) { - if len(t.queue) > 0 && t.queue[0] == name { + wasShooter := len(t.queue) > 0 && t.queue[0] == name + if wasShooter { // The queued roll belongs to the shooter; a departing shooter cancels it. t.pendingRollTicks = 0 } @@ -152,6 +153,15 @@ func (t *crapsTable) remove(name string) { t.seen = map[int]bool{} t.phase = phaseWaiting t.window.close() + } else if wasShooter { + // The next shooter starts a fresh come-out. Without this he inherits + // the previous shooter's point (blocking pass/dontpass and bonus + // bets) and made/seen bonus progress. The shared countdown is left + // alone: it belongs to the table, not the shooter. + t.point = 0 + t.rolled = false + t.made = map[int]bool{} + t.seen = map[int]bool{} } } func (t *crapsTable) leave(name string) []Event { @@ -245,7 +255,7 @@ func (t *crapsTable) bet(name, spot string, amount int, w Wallet) []Event { } limit := t.Config.Craps.oddsLimit(b.number, against) if b.odds.Total > 0 { - continue + return reject("Odds already placed on that bet.") } requested := amount amount = roundUpTo(amount, payoutDen) diff --git a/internal/casino/machine.go b/internal/casino/machine.go index 0733384..93ab8aa 100644 --- a/internal/casino/machine.go +++ b/internal/casino/machine.go @@ -357,7 +357,9 @@ func (m *Machine) finishSpin() []Event { m.phase = machineSpinning m.spin = generateSlotSpinWithProfile(m.rng, m.spinBet, m.profile) m.reel, m.clock, m.suspense = 0, 0, false - events = append(events, m.public(fmt.Sprintf("%d free spins remain.", m.freeSpins))...) + if m.freeSpins > 0 { + events = append(events, m.public(fmt.Sprintf("%d free spins remain.", m.freeSpins))...) + } } else { m.inFreeSpins = false if !payFailed && m.player.Wallet != nil { @@ -382,9 +384,34 @@ func (m *Machine) leave(name string) []Event { if m.phase != machineReady { return m.private("You're in the middle of a spin!") } + bank := int(m.featureBank) + wallet := m.player.Wallet + stakeFailed := m.stakeFailed m.resetSeat() - return []Event{{RoomID: m.RoomID, TableID: m.Config.ID, Type: EventLeft, Player: name, + events := []Event{{RoomID: m.RoomID, TableID: m.Config.ID, Type: EventLeft, Player: name, Message: fmt.Sprintf("%s stands up from the slots machine.", name), PrivateMessage: "You stand up from the slot machine.", PrivateTo: name, Color: "casino_action", Public: true}} + // Banked free-spin winnings must not vanish when the player stands up: + // pay them like markDisconnected/refundAll do, or report the forfeiture. + if bank > 0 && wallet != nil { + switch { + case stakeFailed: + events = append(events, Event{RoomID: m.RoomID, TableID: m.Config.ID, Type: EventResult, + PrivateTo: name, Player: name, + PrivateMessage: fmt.Sprintf("Your %d credits of banked free-spin winnings were forfeited because the round could not be persisted.", bank), + Color: "casino_action", Public: false}) + case wallet.PayCredits(bank) != nil: + events = append(events, Event{RoomID: m.RoomID, TableID: m.Config.ID, Type: EventResult, + PrivateTo: name, Player: name, + PrivateMessage: fmt.Sprintf("Your %d credits of free-spin winnings could not be persisted; they were forfeited.", bank), + Color: "casino_action", Public: false}) + default: + events = append(events, Event{RoomID: m.RoomID, TableID: m.Config.ID, Type: EventResult, + PrivateTo: name, Player: name, + PrivateMessage: fmt.Sprintf("The house settles your %d banked free-spin winnings.", bank), + Color: "casino_action", Public: false}) + } + } + return events } // resetSeat frees the machine, discarding any in-flight spin and autobet diff --git a/internal/casino/machine_test.go b/internal/casino/machine_test.go index 1e8cdad..8942936 100644 --- a/internal/casino/machine_test.go +++ b/internal/casino/machine_test.go @@ -14,6 +14,20 @@ func TestBetRangeTextFormatsUnlimitedMax(t *testing.T) { } } +// Regression: standing up while a feature was banked silently dropped the +// credits; leave must settle them like markDisconnected does. +func TestMachineLeavePaysBankedFeature(t *testing.T) { + m := newMachine(1, MachineConfig{ID: "slots", Game: GameSlots}, rand.New(rand.NewSource(7))) + w := &testWallet{} + m.player = &Participant{Name: "alice", Connected: true, Wallet: w} + m.featureBank = 25 + m.phase = machineReady + _ = m.leave("alice") + if w.paid != 25 { + t.Fatalf("banked feature not paid on leave: paid = %d, want 25", w.paid) + } +} + func TestMachineSetBetUnsetDefaultMaxMessage(t *testing.T) { m := newMachine(1, MachineConfig{ID: "slots", Game: GameSlots, MinBet: 1}, rand.New(rand.NewSource(7))) m.player = &Participant{Name: "alice", Connected: true} diff --git a/internal/casino/video_poker.go b/internal/casino/video_poker.go index 4cb9883..98bfda1 100644 --- a/internal/casino/video_poker.go +++ b/internal/casino/video_poker.go @@ -279,7 +279,7 @@ func (t *videoPokerTable) bet(name, spot string, amount int, w Wallet) []Event { if wallet, ok := w.(PokerWallet); ok { banks, err := wallet.LoadPokerBanks(t.RoomID, t.Config.ID) if err != nil { - return []Event{t.private(name, "Machine bank unavailable; try again later: "+err.Error())} + return []Event{t.private(name, "Machine bank unavailable; try again later.")} } if banks != nil { t.banks = banks @@ -308,7 +308,7 @@ func (t *videoPokerTable) bet(name, spot string, amount int, w Wallet) []Event { } stake, err := w.Wager(p.cost()) if err != nil { - return []Event{t.private(name, "The wager could not be placed: "+err.Error())} + return []Event{t.private(name, walletErrorMessage(err, "Not enough chips and credits for that wager."))} } p.wallet, p.wager, p.lastBet = w, stake, p.loaded p.bankKey = t.multiplierKey(p) diff --git a/internal/game/casino_test.go b/internal/game/casino_test.go index 0b82945..a3c3d18 100644 --- a/internal/game/casino_test.go +++ b/internal/game/casino_test.go @@ -22,6 +22,7 @@ func (c *casinoTestConn) Write(b []byte) (int, error) { return len(b), nil } func (c *casinoTestConn) SetWriteDeadline(time.Time) error { return nil } +func (c *casinoTestConn) SetReadDeadline(time.Time) error { return nil } func (c *casinoTestConn) Close() error { return nil } func (c *casinoTestConn) SetEcho(bool) error { return nil } diff --git a/internal/game/cmd_bank.go b/internal/game/cmd_bank.go index bb03869..76fefb7 100644 --- a/internal/game/cmd_bank.go +++ b/internal/game/cmd_bank.go @@ -265,10 +265,15 @@ func (g *Game) doBankWithdraw(sess *net.Session, input string) { return } - for _, e := range entries { - def, _ := g.ItemStore.Load(e.id) - if def != nil && def.MatchesName(input) { - matches = append(matches, e) + // Only fall back to fuzzy MatchesName when the prefix pass found nothing; + // running it unconditionally appended the same entries twice and printed a + // duplicated ambiguity list. + if len(matches) == 0 { + for _, e := range entries { + def, _ := g.ItemStore.Load(e.id) + if def != nil && def.MatchesName(input) { + matches = append(matches, e) + } } } diff --git a/internal/game/cmd_get.go b/internal/game/cmd_get.go index a806ffa..d4742a5 100644 --- a/internal/game/cmd_get.go +++ b/internal/game/cmd_get.go @@ -40,7 +40,7 @@ func (g *Game) doGet(sess *net.Session, input string) { itemID := matches[0].ID if itemID == "credits" { - g.pickupCredits(sess, p, itemID) + g.pickupCredits(sess, p, itemID, qty) return } @@ -162,7 +162,7 @@ func (g *Game) doGetAllNamed(sess *net.Session, input string) { itemID := matches[0].ID if itemID == "credits" { - g.pickupCredits(sess, p, itemID) + g.pickupCredits(sess, p, itemID, 0) return } @@ -391,7 +391,7 @@ func (g *Game) doGetAll(sess *net.Session) { } } -func (g *Game) pickupCredits(sess *net.Session, p *player.Player, itemID string) { +func (g *Game) pickupCredits(sess *net.Session, p *player.Player, itemID string, want int) { // The balance cap is a hard limit: credits the purse cannot absorb must // stay on the ground instead of being removed and then clamped away by // grantCredits. Only the headroom below the cap is taken. @@ -400,7 +400,13 @@ func (g *Game) pickupCredits(sess *net.Session, p *player.Player, itemID string) sess.WriteLine(g.colorize(sess, "currency_pickup", "Your purse can't hold any more credits.")) return } - qty := g.World.RemoveAvailableItem(p.RoomID, itemID, room, p.Name) + // Honor an explicit quantity (get 5 credits) instead of always sweeping + // the whole purse headroom. + take := room + if want > 0 && want < take { + take = want + } + qty := g.World.RemoveAvailableItem(p.RoomID, itemID, take, p.Name) if qty <= 0 { return } diff --git a/internal/game/cmd_room_remove_test.go b/internal/game/cmd_room_remove_test.go index be2d917..dc5a397 100644 --- a/internal/game/cmd_room_remove_test.go +++ b/internal/game/cmd_room_remove_test.go @@ -23,6 +23,7 @@ func (c *removeTestConn) Write(b []byte) (int, error) { return len(b), nil } func (c *removeTestConn) SetWriteDeadline(time.Time) error { return nil } +func (c *removeTestConn) SetReadDeadline(time.Time) error { return nil } func (c *removeTestConn) Close() error { return nil } func (c *removeTestConn) SetEcho(bool) error { return nil } diff --git a/internal/game/correct_audit_fixes_test.go b/internal/game/correct_audit_fixes_test.go index 90fdc53..681843c 100644 --- a/internal/game/correct_audit_fixes_test.go +++ b/internal/game/correct_audit_fixes_test.go @@ -6,6 +6,7 @@ import ( "thehouseoficarus/internal/behavior" "thehouseoficarus/internal/casino" "thehouseoficarus/internal/item" + "thehouseoficarus/internal/net" "thehouseoficarus/internal/prediction" ) @@ -87,6 +88,22 @@ func TestGlobalCommandPreservesArgumentCase(t *testing.T) { } } +// Regression: `get 5 credits` used to sweep the whole purse headroom. +func TestPickupCreditsHonorsRequestedQuantity(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + h.g.RunOnOwner(func() { + p.Credits = 0 + h.g.World.AddGroundItem(p.RoomID, "credits", 10) + sess := &net.Session{Player: p, Conn: newChanConn()} + h.g.pickupCredits(sess, p, "credits", 5) + if p.Credits != 5 { + t.Errorf("credits = %d, want 5 (output: %s)", p.Credits, sess.Conn.(*chanConn).Bytes()) + } + }) +} + // Regression: the player-file classifier must canonicalize repeated "/" and // "data/" prefixes so a raw-editor path cannot dodge the online check. func TestPlayerDataFileNameCanonicalizesPrefixes(t *testing.T) { diff --git a/internal/game/credits_pickup_test.go b/internal/game/credits_pickup_test.go index 721c550..4342ea8 100644 --- a/internal/game/credits_pickup_test.go +++ b/internal/game/credits_pickup_test.go @@ -19,7 +19,7 @@ func TestPickupCreditsSentinelQuantity(t *testing.T) { p.Credits = 100 h.g.World.AddGroundItem(p.RoomID, "credits", 10) sess := &net.Session{Player: p, Conn: newChanConn()} - h.g.pickupCredits(sess, p, "credits") + h.g.pickupCredits(sess, p, "credits", 0) if p.Credits != 110 { t.Errorf("credits after pickup = %d, want 110 (output: %s)", p.Credits, sess.Conn.(*chanConn).Bytes()) } diff --git a/internal/game/hardening_credit_cap_test.go b/internal/game/hardening_credit_cap_test.go index 872c746..05b0aeb 100644 --- a/internal/game/hardening_credit_cap_test.go +++ b/internal/game/hardening_credit_cap_test.go @@ -23,7 +23,7 @@ func TestPickupCreditsAtCapLeavesRemainderOnGround(t *testing.T) { sess := &net.Session{Conn: newChanConn(), Player: p} sess.SetState(net.StateGame) - h.g.pickupCredits(sess, p, "credits") + h.g.pickupCredits(sess, p, "credits", 0) if p.Credits != casino.MaxCredits { t.Errorf("balance = %d, want the cap %d", p.Credits, casino.MaxCredits) @@ -44,7 +44,7 @@ func TestPickupCreditsAtCapDestroysNothing(t *testing.T) { sess := &net.Session{Conn: newChanConn(), Player: p} sess.SetState(net.StateGame) - h.g.pickupCredits(sess, p, "credits") + h.g.pickupCredits(sess, p, "credits", 0) if p.Credits != casino.MaxCredits { t.Errorf("balance = %d, want the cap %d", p.Credits, casino.MaxCredits) diff --git a/internal/game/harness_test.go b/internal/game/harness_test.go index 1d3df10..ff5ac32 100644 --- a/internal/game/harness_test.go +++ b/internal/game/harness_test.go @@ -62,6 +62,7 @@ func (c *chanConn) Close() error { } func (c *chanConn) SetWriteDeadline(time.Time) error { return nil } +func (c *chanConn) SetReadDeadline(time.Time) error { return nil } func (c *chanConn) SetEcho(bool) error { return nil } diff --git a/internal/game/sys_escape_test.go b/internal/game/sys_escape_test.go index 61b50cd..fd4c9ab 100644 --- a/internal/game/sys_escape_test.go +++ b/internal/game/sys_escape_test.go @@ -24,6 +24,7 @@ func (c *escapeTestConn) Write(b []byte) (int, error) { return len(b), nil } func (c *escapeTestConn) SetWriteDeadline(time.Time) error { return nil } +func (c *escapeTestConn) SetReadDeadline(time.Time) error { return nil } func (c *escapeTestConn) Close() error { return nil } func (c *escapeTestConn) SetEcho(bool) error { return nil } diff --git a/internal/net/conn.go b/internal/net/conn.go index ca40354..a4be83a 100644 --- a/internal/net/conn.go +++ b/internal/net/conn.go @@ -23,6 +23,10 @@ type Conn interface { // instead of a hung writer goroutine. Implementations without deadline // support return nil (deadlines are best-effort). SetWriteDeadline(t time.Time) error + // SetReadDeadline is the read-side counterpart: transports implement it so + // a silent peer cannot pin a goroutine and a connection slot forever. + // Implementations without deadline support return nil. + SetReadDeadline(t time.Time) error SetEcho(bool) error } @@ -114,6 +118,10 @@ func (t *tcpConn) SetWriteDeadline(ts time.Time) error { return t.conn.SetWriteDeadline(ts) } +func (t *tcpConn) SetReadDeadline(ts time.Time) error { + return t.conn.SetReadDeadline(ts) +} + func (t *tcpConn) Close() error { return t.conn.Close() } diff --git a/internal/net/remote_host_test.go b/internal/net/remote_host_test.go index 690e758..bfd10f5 100644 --- a/internal/net/remote_host_test.go +++ b/internal/net/remote_host_test.go @@ -13,6 +13,7 @@ func (c remoteAddrConn) ReadMessage() (string, error) { return "", nil } func (c remoteAddrConn) Write(b []byte) (int, error) { return len(b), nil } func (c remoteAddrConn) Close() error { return nil } func (c remoteAddrConn) SetWriteDeadline(time.Time) error { return nil } +func (c remoteAddrConn) SetReadDeadline(time.Time) error { return nil } func (c remoteAddrConn) SetEcho(bool) error { return nil } func (c remoteAddrConn) RemoteAddr() string { return c.addr } diff --git a/internal/net/server.go b/internal/net/server.go index 6367419..71e34c8 100644 --- a/internal/net/server.go +++ b/internal/net/server.go @@ -331,17 +331,11 @@ func sessionIdleTimeout(sess *Session) time.Duration { } } -// armReadDeadline sets an idle read deadline on the session's transport when -// it supports one. tcpConn and wsConn (both in this package) expose their -// underlying connection, so telnet, telnet-TLS, and websocket are all covered. +// armReadDeadline sets an idle read deadline on the session's transport. The +// Conn interface requires SetReadDeadline, so every transport (telnet, +// telnet-TLS, websocket) is covered and a new one cannot silently miss it. func armReadDeadline(sess *Session, d time.Duration) { - deadline := time.Now().Add(d) - switch c := sess.Conn.(type) { - case *tcpConn: - _ = c.conn.SetReadDeadline(deadline) - case *wsConn: - _ = c.conn.SetReadDeadline(deadline) - } + _ = sess.Conn.SetReadDeadline(time.Now().Add(d)) } // StartWriter puts the session into queued mode: writes are enqueued and a @@ -559,8 +553,11 @@ func (h *Hub) LeaveRoom(s *Session) { } func (h *Hub) leaveRoomLocked(s *Session) { - for _, room := range h.rooms { + for id, room := range h.rooms { delete(room, s) + if len(room) == 0 { + delete(h.rooms, id) + } } } @@ -582,7 +579,15 @@ func (h *Hub) RekeyRooms(idMap map[int]int) { } } for newID, set := range moved { - h.rooms[newID] = set + if existing := h.rooms[newID]; existing != nil { + // Merge instead of clobbering a target bucket that exists but is + // not itself in idMap; assigning would drop its sessions. + for s := range set { + existing[s] = true + } + } else { + h.rooms[newID] = set + } } } diff --git a/internal/net/server_test.go b/internal/net/server_test.go index 6a3b376..b540704 100644 --- a/internal/net/server_test.go +++ b/internal/net/server_test.go @@ -47,6 +47,7 @@ type testConn struct{ data []byte } func (c *testConn) ReadMessage() (string, error) { return "", nil } func (c *testConn) Write(b []byte) (int, error) { c.data = append(c.data, b...); return len(b), nil } func (c *testConn) SetWriteDeadline(time.Time) error { return nil } +func (c *testConn) SetReadDeadline(time.Time) error { return nil } func (c *testConn) Close() error { return nil } func (c *testConn) SetEcho(bool) error { return nil } diff --git a/internal/net/wsconn.go b/internal/net/wsconn.go index 95f714c..e7763aa 100644 --- a/internal/net/wsconn.go +++ b/internal/net/wsconn.go @@ -41,6 +41,11 @@ func (w *wsConn) SetWriteDeadline(ts time.Time) error { return w.conn.SetWriteDeadline(ts) } +// SetReadDeadline delegates to the underlying websocket connection. +func (w *wsConn) SetReadDeadline(ts time.Time) error { + return w.conn.SetReadDeadline(ts) +} + func (w *wsConn) Close() error { return w.conn.Close() } diff --git a/internal/persistence/txn.go b/internal/persistence/txn.go index 1049077..a12e227 100644 --- a/internal/persistence/txn.go +++ b/internal/persistence/txn.go @@ -406,7 +406,16 @@ func Recover(journalDir string) error { return err } for _, e := range entries { - if e.IsDir() || !hasSuffix(e.Name(), ".json") { + if e.IsDir() { + continue + } + if hasPrefix(e.Name(), ".save-") { + // A crash during a journal write leaves an atomic-writer temp + // behind; it is never a valid journal, so sweep it. + os.Remove(filepath.Join(journalDir, e.Name())) + continue + } + if !hasSuffix(e.Name(), ".json") { continue } path := filepath.Join(journalDir, e.Name()) @@ -445,6 +454,10 @@ func hasSuffix(s, suffix string) bool { return len(s) >= len(suffix) && s[len(s)-len(suffix):] == suffix } +func hasPrefix(s, prefix string) bool { + return len(s) >= len(prefix) && s[:len(prefix)] == prefix +} + // ListJournalFiles is a debugging helper: returns sorted journal file names. func ListJournalFiles(journalDir string) ([]string, error) { entries, err := os.ReadDir(journalDir) diff --git a/internal/validate/checks.go b/internal/validate/checks.go index 82f746f..d3fa111 100644 --- a/internal/validate/checks.go +++ b/internal/validate/checks.go @@ -1643,6 +1643,17 @@ func validateTalkConfig(prefix string, cfg *behavior.TalkConfig, itemIDs map[str if cfg == nil { return issues } + // A talk config with nodes but no "start" node is dead content: the + // runtime prints "This person has nothing to say." + if len(cfg.Nodes) > 0 { + if _, ok := cfg.Nodes["start"]; !ok { + issues = append(issues, Issue{ + Level: "ERROR", + Type: "reference", + Message: fmt.Sprintf("%s: talk config has nodes but no \"start\" node", prefix), + }) + } + } for nodeID, node := range cfg.Nodes { nodePrefix := fmt.Sprintf("%s: node %q", prefix, nodeID) if node.Action != nil { diff --git a/internal/validate/correct_audit_fixes_test.go b/internal/validate/correct_audit_fixes_test.go new file mode 100644 index 0000000..8ed6f52 --- /dev/null +++ b/internal/validate/correct_audit_fixes_test.go @@ -0,0 +1,33 @@ +package validate + +import ( + "strings" + "testing" + + "thehouseoficarus/internal/behavior" +) + +// Regression: a talk config with nodes but no "start" node is dead content at +// runtime; validation must report it. +func TestValidateTalkConfigRequiresStartNode(t *testing.T) { + cfg := &behavior.TalkConfig{Nodes: map[string]behavior.TalkNode{ + "greet": {Messages: []string{"hi"}}, + }} + issues := validateTalkConfig("test", cfg, nil, nil, nil, nil) + found := false + for _, iss := range issues { + if strings.Contains(iss.Message, "start") { + found = true + } + } + if !found { + t.Fatalf("missing start node not reported: %+v", issues) + } + + clean := &behavior.TalkConfig{Nodes: map[string]behavior.TalkNode{"start": {}}} + for _, iss := range validateTalkConfig("test", clean, nil, nil, nil, nil) { + if strings.Contains(iss.Message, "start") { + t.Fatalf("start node wrongly reported: %+v", iss) + } + } +} |