The ignored() function is called on every packet when debug logging
is enabled. It was rebuilding a slice and map from scratch each time.
Move to a package-level map initialized once at startup.
The userBinary and minidata maps with their locks were spread across
Server as raw fields with manual lock management. Cross-channel session
searches also required acquiring nested locks (server lock + binary
lock). Encapsulating in dedicated types eliminates the nested locking
and reduces Server's field count by 4.
The quest cache fields (lock, data map, expiry map) were spread across
Server with manual lock management. The old read path also missed the
RLock entirely, creating a data race. Encapsulating in a dedicated type
fixes the race and reduces Server's field count by 2.
A failed Begin() returned a nil tx that would panic on the next
tx.Exec() call. Now logs the error, closes rows, and returns an
empty response gracefully.
Move remaining raw s.server.db.* queries from handler files into
dedicated repository structs, completing the repository extraction
effort. Also adds SaveCharacterData and SaveHouseData to
CharacterRepository.
Fixes guild_hunts query to select both cats_used and start columns
to match the existing two-column Scan call. Adds slot index
validation in GoocooRepository to prevent SQL injection via
fmt.Sprintf.
The config package used `package _config` with a leading underscore,
which is unconventional in Go. Rename to `package config` (matching the
directory name) and use `cfg` as the standard import alias across all
93 importing files.
Move 22 raw SQL queries from 4 handler files into dedicated repository
structs, continuing the repository extraction pattern. Achievement
insert uses ON CONFLICT DO NOTHING to eliminate check-then-insert race,
and IncrementScore validates the column index to prevent SQL injection.
RolloverDailyRP now locks the guild row with SELECT FOR UPDATE and
re-checks rp_reset_at inside the transaction, so concurrent callers
cannot double-rollover and zero out freshly donated RP.
Gacha stepup entry-type check is now skipped when the row was already
deleted as stale, avoiding a redundant DELETE on step 0.
Gacha stepup progress now resets when queried after the most recent
noon boundary, using a new created_at column on gacha_stepup.
Guild member rp_today rolls into rp_yesterday lazily when members are
enumerated after noon, using a new rp_reset_at column on guilds.
Both follow the established lazy-reset pattern from the cafe handler.
The handler was a stub that discarded pkt.NumUsers. Now it
looks up the player's guild and atomically accumulates the
count via a new weekly_bonus_users column on the guilds table.
sqlx.Open was called with no pool configuration, risking PostgreSQL
connection exhaustion under load. Set max open/idle conns and lifetimes.
CreatePost INSERT + soft-delete UPDATE were two separate queries with
no transaction, risking inconsistent state on partial failure.
CollectAdventure used SELECT then UPDATE without a lock, allowing
concurrent guild members to double-collect. Now uses SELECT FOR UPDATE
within a transaction.
- testhelpers_db: retry truncateAllTables up to 3 times on deadlock,
which occurs when previous tests' goroutines still hold DB connections
- handlers_rengoku_integration_test: restore rengoku_score table after
TestRengokuData_SaveOnDBError drops it, preventing cascading failures
in all subsequent rengoku tests
- client_connection_simulation_test: fix TestClientConnection_PacketDuringLogout
to accept both race outcomes (save-wins or logout-wins) since both
handlers independently load from DB and last-writer-wins is valid
Eliminate 18 direct s.server.db calls from handlers_items.go,
handlers_distitem.go, and handlers_session.go by moving queries into
dedicated repository types.
New repositories:
- StampRepository (7 methods, stamps table)
- DistributionRepository (4 methods, distribution/distribution_items)
- SessionRepository (4 methods, sign_sessions/servers)
Also adds ClearTreasureHunt and InsertKillLog to GuildRepository,
which already owns those tables for read operations.
- Use users.frontier_points instead of characters.frontier_points
(column moved in 9.2 schema migration) across savedata and session
lifecycle tests
- Use BYTEA column (otomoairou) instead of INTEGER column
(kouryou_point) in repo_character Load/SaveColumn tests
- Build blocked CSV from actual auto-incremented character IDs instead
of hardcoded IDs in ListMember integration test
- Fix nil charRepo panic in CompleteSaveLoadCycle by using SetTestDB()
Cover all 14 handler functions in handlers_house.go with 25 new tests:
- 7 unit tests for guard paths (payload size limits, box index
bounds, no-op handlers) that run without a database
- 18 integration tests against real PostgreSQL covering interior
updates, house state/password, house enumeration by char ID and
name, house loading with access control, mission data CRUD,
title acquisition with dedup, warehouse operations (box names,
usage limits, rename guards), item storage round-trips, and
deco myset defaults
Introduces readAck() helper to parse MsgSysAck wire format from
the sendPackets channel, and setupHouseTest() for DB + session
scaffolding with user_binary row initialization.
Three fixes for the channelserver test suite:
- Add net.ErrClosed check in acceptClients() so a closed listener
breaks the loop immediately instead of spinning and logging warnings.
This is correct production behavior too.
- Remove -c flag from pg_restore that conflicted with CleanTestDB's
prior DROP, causing "gook DROP CONSTRAINT" errors. Clean the schema
with DROP SCHEMA CASCADE instead of per-table drops.
- Use sync.Once to apply the test schema once per binary run, then
TRUNCATE tables for isolation. Reduces ~60 pg_restore + 29-patch
cycles to a single setup pass.
Move all direct DB access from handlers_rengoku.go (11 calls) and
handlers_mail.go (10 calls) into dedicated repository types, continuing
the established extraction pattern.
RengokuRepository provides UpsertScore and GetRanking, replacing a
3-call check/insert/update sequence and an 8-case switch of nearly
identical queries respectively.
MailRepository provides SendMail, SendMailTx, GetListForCharacter,
GetByID, MarkRead, MarkDeleted, SetLocked, and MarkItemReceived.
The old Mail.Send(), Mail.MarkRead(), GetMailListForCharacter, and
GetMailByID free functions are removed. Guild handlers that sent mail
via Mail.Send(s, ...) now call mailRepo directly.
The stages map was protected by two incompatible locks: the embedded
Server.Mutex and Server.stagesLock (RWMutex). Since these are separate
mutexes they don't exclude each other, and many handlers accessed the
map with no lock at all.
Route all stages map access through stagesLock: read-only lookups use
RLock, writes (create/delete) use Lock. Per-stage field mutations
continue to use each stage's own RWMutex. Restructure
handleMsgSysUnlockStage to avoid holding stagesLock nested inside a
stage RLock, preventing potential deadlock with destructEmptyStages.
Move all direct DB calls from handlers_festa.go (23 calls across 8
tables) and handlers_tower.go (16 calls across 4 tables) into
dedicated repository structs following the established pattern.
FestaRepository (14 methods): lifecycle cleanup, event management,
team souls, trial stats/rankings, user state, voting, registration,
soul submission, prize claiming/enumeration.
TowerRepository (12 methods): personal tower data (skills, progress,
gems), guild tenrouirai progress/scores/page advancement, tower RP.
Also fix pre-existing nil pointer panics in integration tests by
adding SetTestDB helper that initializes both the DB connection and
all repositories, and wire the done channel in createTestServerWithDB
to prevent Shutdown panics.
The game client sometimes writes -1 (0xFF bytes) into the house_tier
field during save, which causes the house theme to vanish on next
login. Snapshot the house tier before applying the save delta and
restore it if the incoming value is corrupted.
Add 34 tests covering all previously-untested GuildRepository methods:
invitations/scouts, guild posts, alliances, adventures, treasure
hunts, meals, kill tracking, and edge cases like disband with
alliance cleanup.
Fix test schema setup to apply the 9.2 update schema after init.sql,
which bridges v9.1.0 to v9.2.0 and resolves 9 pre-existing test
failures caused by missing columns (rp_today, created_at, etc.).
Make patch schemas 14 and 19 idempotent so they no longer fail when
applied against a schema that already has the target state (e.g.
festival_color type already renamed, legacy column names).
Centralizes all gacha_shop/gacha_entries/gacha_items/gacha_stepup/gacha_box
table access into GachaRepository (15 methods) and all user_binary house
columns, warehouse, and titles table access into HouseRepository (17 methods).
Eliminates all direct DB calls from handlers_gacha.go and handlers_house.go.
Centralizes all 31 direct users-table SQL queries from 11 handler
files into a single UserRepository, following the same pattern as
CharacterRepository and GuildRepository. The only excluded query is
the sign_sessions JOIN in handleMsgSysLogin which spans multiple
tables.
Move ~39 inline SQL queries from six handler files into repo_guild.go,
consolidating all guild-related DB access (posts, alliances, adventures,
treasure hunts, meals, kill logs, scouts) behind GuildRepository methods.
Handler files now contain only packet serialization, business logic, and
ACK responses with no direct database calls.
Per anti-patterns.md item #9, guild-related SQL was scattered across
~15 handler files with no repository abstraction. Following the same
pattern established by CharacterRepository, this centralizes all
guilds, guild_characters, and guild_applications table access into a
single GuildRepository (~30 methods).
guild_model.go and handlers_guild_member.go are trimmed to types and
pure business logic only. All handler files (guild_*, festa, mail,
house, mercenary, rengoku) now call s.server.guildRepo methods
instead of direct DB queries or methods on domain objects.
Add 18 new typed methods to CharacterRepository (ReadTime, SaveTime,
SaveInt, SaveBool, SaveString, ReadBool, ReadString, LoadColumnWithDefault,
SetDeleted, UpdateDailyCafe, ResetDailyQuests, ReadEtcPoints, ResetCafeTime,
UpdateGuildPostChecked, ReadGuildPostChecked, SaveMercenary, UpdateGCPAndPact,
FindByRastaID) and migrate ~56 inline SQL queries across 13 handler files.
Pure refactor — zero behavior change. Each handler produces identical SQL
with identical parameters. Cross-table JOINs and bulk CharacterSaveData
operations are intentionally left out of scope.
Centralizes all characters table SQL behind a CharacterRepository struct
in repo_character.go. The 4 existing helpers (loadCharacterData,
saveCharacterData, readCharacterInt, adjustCharacterInt) now delegate to
the repository, keeping identical signatures so all ~70 callsites remain
unchanged. Direct queries in handlers_session.go, sys_channel_server.go
(DisconnectUser), and handlers_mail.go are also migrated.
Pure refactor with zero behavior change — first step toward eliminating
the ~130 scattered character queries identified in anti-patterns #9.
golangci-lint's errcheck rule requires explicit handling of error
return values from Close, Write, and Logout calls. Use blank
identifier assignment for cleanup paths where errors are
intentionally discarded.
Add readCharacterInt/adjustCharacterInt helpers for single-column
integer operations on the characters table. Eliminates fmt.Sprintf
SQL construction in handlers_misc.go and replaces inline queries
across cafe, kouryou, and mercenary handlers.
Second round of protocol constant extraction: adds constants_time.go
(secsPerDay, secsPerWeek), constants_raviente.go (register IDs,
semaphore constants), and named constants across 14 handler files
replacing raw hex/numeric literals. Updates anti-patterns doc to
mark #4 (magic numbers) as substantially fixed.
Silently ignored DB errors in handlers could cause data loss (frontier
point transactions completing without DB writes), reward duplication
(stamp exchange granting items on failed UPDATE), and crashes (tower
mission page=0 causing index-out-of-bounds). House access state
defaulting to 0 on DB failure also bypassed all access controls.
HIGH risk fixes:
- frontier point buy/sell now fails with ACK on DB error
- stamp exchange/stampcard abort on failed UPDATE
- guild meal INSERT returns fail ACK instead of orphaned ID 0
- mercenary/airou creation aborts on failed sequence nextval
MEDIUM risk fixes:
- tower mission page clamped to >= 1 preventing array underflow
- tower RP donation returns early on failed guild state read
- house state defaults to 2 (password-protected) on DB failure
- playtime read failure logged instead of silently resetting RP
Also cache userID on Session at login time, eliminating ~25 redundant
subqueries of the form WHERE u.id=(SELECT c.user_id FROM characters
c WHERE c.id=$1) across shop, gacha, command, and distitem handlers.
Binary I/O (#5): all 12 remaining encoding/binary calls are
legitimate (zero-alloc spot-reads, random-access into game blobs).
Copy-paste handlers (#8): loadCharacterData/saveCharacterData helpers
now cover standard blob patterns.
Also upgrades saveCharacterData to send doAckSimpleFail on oversize
payloads and DB errors, and migrates handleMsgMhfSaveScenarioData
to the improved helper.
handleMsgSysOperateRegister returned without sending an ACK when the
payload was empty, causing the client to softlock waiting for a response.
Send doAckBufSucceed with nil data on the early-return path to match
the success-path ACK type.
Also update tests to expect error returns instead of panics from
unimplemented Build/Parse stubs (matching prior panic→error refactor),
and mark resolved anti-patterns in docs.
ByteFrame previously panicked on out-of-bounds reads, which crashed
the server when parsing malformed client packets. Now sets a sticky
error (checked via Err()) and returns zero values, matching the
encoding/binary scanner pattern. The session recv loop checks Err()
after parsing to reject malformed packets gracefully.
Also replaces remaining panic("Not implemented") stubs in network
packet Build/Parse methods with proper error returns.
Extract numeric literals into named constants across quest handling,
save data parsing, rengoku skill layout, diva event timing, guild info,
achievement trophies, RP accrual rates, and semaphore IDs. Adds
constants_quest.go for quest-related constants shared across functions.
Pure rename/extract with zero behavior change.
Handlers that log errors and return without sending a MsgSysAck leave
the client waiting indefinitely. Add doAckSimpleFail/doAckBufFail to
14 error paths across 4 files, matching the pattern already used in
~70 other error paths across the codebase.
Affected handlers:
- handleMsgMhfGetCafeDuration (1 path)
- handleMsgMhfSavedata (1 path)
- handleMsgMhfArrangeGuildMember (3 paths)
- handleMsgMhfEnumerateGuildMember (5 paths)
- handleMsgSysLogin (4 paths)
- handleMsgSysIssueLogkey (1 path)
Replace all fmt.Printf/Println and log.Printf/Fatal with structured
zap.Logger calls to eliminate inconsistent logging (anti-pattern #12).
- network/crypt_conn: inject logger via NewCryptConn, replace 6 fmt calls
- signserver/session: use existing s.logger for debug packet dumps
- entranceserver: use s.logger for inbound/outbound debug logging
- api/utils: accept logger param in verifyPath, replace fmt.Println
- api/endpoints: use s.logger for screenshot path diagnostics
- config: replace log.Fatal with error return in getOutboundIP4
- deltacomp: replace log.Printf with zap.L() global logger
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.
Replace vague "Alpelo object system backport" references in CHANGELOG
and AUTHORS with accurate descriptions of the per-session object ID
allocation rework. Remove stale test comment referencing the deleted
NextObjectID method.
Replace the mutable global `_config.ErupeConfig` with dependency
injection across 79 files. Config is now threaded through existing
paths: `ClientContext.RealClientMode` for packet encoding, `s.server.
erupeConfig` for channel handlers, and explicit parameters for utility
functions. This removes hidden coupling, enables test parallelism
without global save/restore, and prevents low-level packages from
reaching up to the config layer.
Key changes:
- Enrich ClientContext with RealClientMode for packet files
- Add mode parameter to CryptConn, mhfitem, mhfcourse functions
- Convert handlers_commands init() to lazy sync.Once initialization
- Delete global var, init(), and helper functions from config.go
- Update all tests to pass config explicitly
The channel server had several concurrency issues found by the race
detector during isolation testing:
- acceptClients could send on a closed acceptConns channel during
shutdown, causing a panic. Replace close(acceptConns) with a done
channel and select-based shutdown signaling in both acceptClients
and manageSessions.
- invalidateSessions read isShuttingDown and iterated sessions without
holding the lock. Rewrite with ticker + done channel select and
snapshot sessions under lock before processing timeouts.
- sendLoop/recvLoop accessed global _config.ErupeConfig.LoopDelay
which races with tests modifying the global. Use the per-server
erupeConfig instead.
- logoutPlayer panicked on DB errors and crashed on nil DB (no-db
test scenarios). Guard with nil check and log errors instead.
- Shutdown was not idempotent, double-calling caused double-close
panic on done channel.
Add 5 channel isolation tests verifying independent shutdown,
listener failure, session panic recovery, cross-channel registry
after shutdown, and stage isolation.
The protbot sent "DSGN:\x00" as the sign request type, but the server
strips the last 3 characters as a version suffix. Send "DSGN:041"
(ZZ client mode 41) to match the real client format.
The entrance channel entry parser read 14 bytes for remaining fields
but the server writes 18 bytes (9 uint16, not 7), causing a panic
when parsing the server list.
The channel server panicked on disconnect when a session had no
decompressed save data (e.g. protbot or early client disconnect).
Guard Save() against nil decompSave.
Also fix docker-compose volume mount for Postgres 18 which changed
its data directory layout.
Enable multiple Erupe instances to share a single PostgreSQL database
without destroying each other's state, fix existing data races in
cross-channel access, and lay groundwork for future distributed
channel server deployments.
Phase 1 — DB safety:
- Scope DELETE FROM servers/sign_sessions to this instance's server IDs
- Fix ci++ bug where failed channel start shifted subsequent IDs
Phase 2 — Fix data races in cross-channel access:
- Lock sessions map in FindSessionByCharID and DisconnectUser
- Lock stagesLock in handleMsgSysLockGlobalSema
- Snapshot sessions/stages under lock in TransitMessage types 1-4
- Lock channel when finding mail notification targets
Phase 3 — ChannelRegistry interface:
- Define ChannelRegistry interface with 7 cross-channel operations
- Implement LocalChannelRegistry with proper locking
- Add SessionSnapshot/StageSnapshot immutable copy types
- Delegate WorldcastMHF, FindSessionByCharID, DisconnectUser to Registry
- Migrate LockGlobalSema and guild mail handlers to use Registry
- Add comprehensive tests including concurrent access
Phase 4 — Per-channel enable/disable:
- Add Enabled *bool to EntranceChannelInfo (nil defaults to true)
- Skip disabled channels in startup loop, preserving ID stability
- Add IsEnabled() helper with backward-compatible default
- Update config.example.json with Enabled field
A malicious or buggy client could send arbitrarily large payloads
that get written directly to PostgreSQL, wasting disk and memory.
Each save handler now rejects payloads exceeding a generous upper
bound derived from the known data format sizes.
Covers all remaining items from #158: partner, hunternavi,
savemercenary, scenariodata, platedata, platebox, platemyset,
rengokudata, mezfes, savefavoritequest, house_furniture, mission.
Closes#158
Several handlers used packet fields as array indices or SQL column
names without bounds checking, allowing crafted packets to panic the
server or produce malformed SQL.
Panic fixes (high severity):
- handlers_mail: bounds check AccIndex against mailList length
- handlers_misc: validate ArmourID >= 10000 and MogType <= 4
- handlers_mercenary: check RawDataPayload length before slicing
- handlers_house: check RawDataPayload length in SaveDecoMyset
- handlers_register: guard empty RawDataPayload in OperateRegister
SQL column name fixes (medium severity):
- handlers_misc: early return on unknown PointType
- handlers_items: reject unknown StampType in weekly stamp handlers
- handlers_achievement: cap AchievementID at 32
- handlers_goocoo: skip goocoo.Index > 4
- handlers_house: cap BoxIndex for warehouse operations
- handlers_tower: fix MissionIndex=0 bypassing normalization guard
UserBinary type1-5 and EnhancedMinidata are transient session state
resent by the client on every login. Persisting them to the DB on
every set was unnecessary I/O. Both are now served exclusively from
server-scoped in-memory maps (userBinaryParts, minidataParts).
Includes a schema migration to drop the now-unused type2/type3
columns from user_binary and minidata column from characters.
Ref #158
- Reject BinaryType outside 1-5 in SetUserBinary to prevent
dynamic column name with unchecked client input
- Check rengoku payload length before DB write and fixed-offset
reads to prevent panic on short payloads
- Require MercData >= 4 bytes before ReadUint32 to prevent panic
Ref: Mezeporta/Erupe#158
