diff --git a/internal/db/queries.go b/internal/db/queries.go index d1abc25..3463f24 100644 --- a/internal/db/queries.go +++ b/internal/db/queries.go @@ -2415,6 +2415,71 @@ func (database *Database) SetChannelUserLimit( return nil } +// SetSessionUserModes applies a set of user-mode flag +// changes to a session inside a single transaction, so a +// multi-mode change such as "+w-o" is all-or-nothing. A nil +// pointer means the caller did not mention that mode and +// the stored value must be left untouched. +func (database *Database) SetSessionUserModes( + ctx context.Context, + sessionID int64, + wallops *bool, + oper *bool, +) error { + if wallops == nil && oper == nil { + return nil + } + + transaction, err := database.conn.BeginTx(ctx, nil) + if err != nil { + return fmt.Errorf("begin tx: %w", err) + } + + if wallops != nil { + if _, err := transaction.ExecContext( + ctx, + `UPDATE sessions SET is_wallops = ? WHERE id = ?`, + boolToInt(*wallops), sessionID, + ); err != nil { + _ = transaction.Rollback() + + return fmt.Errorf( + "set session wallops: %w", err, + ) + } + } + + if oper != nil { + if _, err := transaction.ExecContext( + ctx, + `UPDATE sessions SET is_oper = ? WHERE id = ?`, + boolToInt(*oper), sessionID, + ); err != nil { + _ = transaction.Rollback() + + return fmt.Errorf("set session oper: %w", err) + } + } + + if err := transaction.Commit(); err != nil { + _ = transaction.Rollback() + + return fmt.Errorf("commit user modes: %w", err) + } + + return nil +} + +// boolToInt renders a Go bool as the 0/1 integer used for +// boolean columns in the SQLite schema. +func boolToInt(value bool) int { + if value { + return 1 + } + + return 0 +} + // SetSessionWallops sets the wallops (+w) flag on a // session. func (database *Database) SetSessionWallops( diff --git a/internal/service/service.go b/internal/service/service.go index c147651..b4c3a49 100644 --- a/internal/service/service.go +++ b/internal/service/service.go @@ -956,8 +956,11 @@ type userModeOp struct { // rejects malformed input (empty string, no leading sign, // bare sign with no mode letters, unknown mode letters, // +o which must be set via OPER) with an IRCError. On -// failure, no persistent change is made. On success, the -// resulting mode string is returned. +// failure, no persistent change is made: parsing happens +// before any write, and the writes themselves run inside a +// single database transaction that is rolled back whole if +// any statement fails. On success, the resulting mode +// string is returned. func (s *Service) ApplyUserMode( ctx context.Context, sessionID int64, @@ -968,12 +971,19 @@ func (s *Service) ApplyUserMode( return "", err } - for _, op := range ops { - if err := s.applySingleUserMode( - ctx, sessionID, op.char, op.adding, - ); err != nil { - return "", err - } + wallops, oper, err := collapseUserModeOps(ops) + if err != nil { + return "", err + } + + if err := s.db.SetSessionUserModes( + ctx, sessionID, wallops, oper, + ); err != nil { + s.log.Error( + "apply user modes failed", "error", err, + ) + + return "", fmt.Errorf("apply user modes: %w", err) } return s.QueryUserMode(ctx, sessionID) @@ -1048,56 +1058,43 @@ func isKnownUserModeChar(modeChar rune) bool { } } -// applySingleUserMode applies one already-validated user -// mode character to the session. parseUserModeString must -// have validated the character and sign before this runs; -// the default branch here is defence-in-depth only. -func (s *Service) applySingleUserMode( - ctx context.Context, - sessionID int64, - modeChar rune, - adding bool, -) error { - switch modeChar { - case 'w': - err := s.db.SetSessionWallops( - ctx, sessionID, adding, - ) - if err != nil { - s.log.Error( - "set wallops mode failed", "error", err, - ) +// collapseUserModeOps reduces an already-parsed operation +// list to the final desired value of each user mode flag. +// A nil result for a flag means the mode string never +// mentioned it, so it must be left untouched. Later +// operations win over earlier ones for the same letter +// (e.g. "+w-w" ends with wallops off), which matches the +// left-to-right semantics of applying each op in turn. +// parseUserModeString must have validated every character +// and sign before this runs; the default branch here is +// defence-in-depth only. +func collapseUserModeOps( + ops []userModeOp, +) (wallops, oper *bool, err error) { + unknownFlag := &IRCError{ + Code: irc.ErrUmodeUnknownFlag, + Params: nil, + Message: "Unknown MODE flag", + } - return fmt.Errorf("set wallops: %w", err) - } - case 'o': - if adding { - return &IRCError{ - Code: irc.ErrUmodeUnknownFlag, - Params: nil, - Message: "Unknown MODE flag", + for _, op := range ops { + switch op.char { + case 'w': + val := op.adding + wallops = &val + case 'o': + if op.adding { + return nil, nil, unknownFlag } - } - err := s.db.SetSessionOper( - ctx, sessionID, false, - ) - if err != nil { - s.log.Error( - "clear oper mode failed", "error", err, - ) - - return fmt.Errorf("clear oper: %w", err) - } - default: - return &IRCError{ - Code: irc.ErrUmodeUnknownFlag, - Params: nil, - Message: "Unknown MODE flag", + val := false + oper = &val + default: + return nil, nil, unknownFlag } } - return nil + return wallops, oper, nil } // broadcastNickChange notifies channel peers of a nick