modes refactor, #255

Makefile

@@ -12,5 +12,7 @@ deps:
 	git submodule update --init
 
 test:
-	cd irc && go test .
-	cd irc && go vet .
+	cd irc && go test . && go vet .
+	cd irc/isupport && go test . && go vet .
+	cd irc/modes && go test . && go vet .
+	cd irc/utils && go test . && go vet .

irc/channel.go

@@ -20,7 +20,7 @@ import (
 
 // Channel represents a channel that clients can join.
 type Channel struct {
-	flags             modes.ModeSet
+	flags             *modes.ModeSet
 	lists             map[modes.Mode]*UserMaskSet
 	key               string
 	members           MemberSet
@@ -51,7 +51,7 @@ func NewChannel(s *Server, name string, regInfo *RegisteredChannel) *Channel {
 
 	channel := &Channel{
 		createdTime: time.Now(), // may be overwritten by applyRegInfo
-		flags:       make(modes.ModeSet),
+		flags:       modes.NewModeSet(),
 		lists: map[modes.Mode]*UserMaskSet{
 			modes.BanMask:    NewUserMaskSet(),
 			modes.ExceptMask: NewUserMaskSet(),
@@ -68,7 +68,7 @@ func NewChannel(s *Server, name string, regInfo *RegisteredChannel) *Channel {
 		channel.applyRegInfo(regInfo)
 	} else {
 		for _, mode := range s.DefaultChannelModes() {
-			channel.flags[mode] = true
+			channel.flags.SetMode(mode, true)
 		}
 	}
 
@@ -87,7 +87,7 @@ func (channel *Channel) applyRegInfo(chanReg *RegisteredChannel) {
 	channel.key = chanReg.Key
 
 	for _, mode := range chanReg.Modes {
-		channel.flags[mode] = true
+		channel.flags.SetMode(mode, true)
 	}
 	for _, mask := range chanReg.Banlist {
 		channel.lists[modes.BanMask].Add(mask)
@@ -120,9 +120,7 @@ func (channel *Channel) ExportRegistration(includeFlags uint) (info RegisteredCh
 
 	if includeFlags&IncludeModes != 0 {
 		info.Key = channel.key
-		for mode := range channel.flags {
-			info.Modes = append(info.Modes, mode)
-		}
+		info.Modes = channel.flags.AllModes()
 	}
 
 	if includeFlags&IncludeLists != 0 {
@@ -225,14 +223,16 @@ func (channel *Channel) ClientIsAtLeast(client *Client, permission modes.Mode) b
 	channel.stateMutex.RLock()
 	defer channel.stateMutex.RUnlock()
 
+	clientModes := channel.members[client]
+
 	// get voice, since it's not a part of ChannelPrivModes
-	if channel.members.HasMode(client, permission) {
+	if clientModes.HasMode(permission) {
 		return true
 	}
 
 	// check regular modes
 	for _, mode := range modes.ChannelPrivModes {
-		if channel.members.HasMode(client, mode) {
+		if clientModes.HasMode(mode) {
 			return true
 		}
 
@@ -263,14 +263,14 @@ func (channel *Channel) ClientHasPrivsOver(client *Client, target *Client) bool
 	targetModes := channel.members[target]
 	result := false
 	for _, mode := range modes.ChannelPrivModes {
-		if clientModes[mode] {
+		if clientModes.HasMode(mode) {
 			result = true
 			// admins cannot kick other admins
-			if mode == modes.ChannelAdmin && targetModes[modes.ChannelAdmin] {
+			if mode == modes.ChannelAdmin && targetModes.HasMode(modes.ChannelAdmin) {
 				result = false
 			}
 			break
-		} else if channel.members[target][mode] {
+		} else if targetModes.HasMode(mode) {
 			break
 		}
 	}
@@ -331,14 +331,11 @@ func (channel *Channel) modeStrings(client *Client) (result []string) {
 		mods += modes.UserLimit.String()
 	}
 
+	mods += channel.flags.String()
+
 	channel.stateMutex.RLock()
 	defer channel.stateMutex.RUnlock()
 
-	// flags
-	for mode := range channel.flags {
-		mods += mode.String()
-	}
-
 	result = []string{mods}
 
 	// args for flags with args: The order must match above to keep
@@ -395,7 +392,7 @@ func (channel *Channel) Join(client *Client, key string, rb *ResponseBuffer) {
 	}
 
 	isInvited := channel.lists[modes.InviteMask].Match(client.nickMaskCasefolded)
-	if channel.flags[modes.InviteOnly] && !isInvited {
+	if channel.flags.HasMode(modes.InviteOnly) && !isInvited {
 		rb.Add(nil, client.server.name, ERR_INVITEONLYCHAN, channel.name, fmt.Sprintf(client.t("Cannot join channel (+%s)"), "i"))
 		return
 	}
@@ -446,7 +443,7 @@ func (channel *Channel) Join(client *Client, key string, rb *ResponseBuffer) {
 		givenMode = &modes.ChannelOperator
 	}
 	if givenMode != nil {
-		channel.members[client][*givenMode] = true
+		channel.members[client].SetMode(*givenMode, true)
 	}
 	channel.stateMutex.Unlock()
 
@@ -515,12 +512,12 @@ func (channel *Channel) SendTopic(client *Client, rb *ResponseBuffer) {
 
 // SetTopic sets the topic of this channel, if the client is allowed to do so.
 func (channel *Channel) SetTopic(client *Client, topic string, rb *ResponseBuffer) {
-	if !(client.flags[modes.Operator] || channel.hasClient(client)) {
+	if !(client.HasMode(modes.Operator) || channel.hasClient(client)) {
 		rb.Add(nil, client.server.name, ERR_NOTONCHANNEL, channel.name, client.t("You're not on that channel"))
 		return
 	}
 
-	if channel.HasMode(modes.OpOnlyTopic) && !channel.ClientIsAtLeast(client, modes.ChannelOperator) {
+	if channel.flags.HasMode(modes.OpOnlyTopic) && !channel.ClientIsAtLeast(client, modes.ChannelOperator) {
 		rb.Add(nil, client.server.name, ERR_CHANOPRIVSNEEDED, channel.name, client.t("You're not a channel operator"))
 		return
 	}
@@ -552,13 +549,13 @@ func (channel *Channel) CanSpeak(client *Client) bool {
 	defer channel.stateMutex.RUnlock()
 
 	_, hasClient := channel.members[client]
-	if channel.flags[modes.NoOutside] && !hasClient {
+	if channel.flags.HasMode(modes.NoOutside) && !hasClient {
 		return false
 	}
-	if channel.flags[modes.Moderated] && !channel.ClientIsAtLeast(client, modes.Voice) {
+	if channel.flags.HasMode(modes.Moderated) && !channel.ClientIsAtLeast(client, modes.Voice) {
 		return false
 	}
-	if channel.flags[modes.RegisteredOnly] && client.Account() == "" {
+	if channel.flags.HasMode(modes.RegisteredOnly) && client.Account() == "" {
 		return false
 	}
 	return true
@@ -682,13 +679,7 @@ func (channel *Channel) sendSplitMessage(msgid, cmd string, minPrefix *modes.Mod
 	}
 }
 
-func (channel *Channel) applyModeMemberNoMutex(client *Client, mode modes.Mode, op modes.ModeOp, nick string, rb *ResponseBuffer) *modes.ModeChange {
-	if nick == "" {
-		//TODO(dan): shouldn't this be handled before it reaches this function?
-		rb.Add(nil, client.server.name, ERR_NEEDMOREPARAMS, "MODE", client.t("Not enough parameters"))
-		return nil
-	}
-
+func (channel *Channel) applyModeToMember(client *Client, mode modes.Mode, op modes.ModeOp, nick string, rb *ResponseBuffer) (result *modes.ModeChange) {
 	casefoldedName, err := CasefoldName(nick)
 	target := channel.server.clients.Get(casefoldedName)
 	if err != nil || target == nil {
@@ -698,26 +689,21 @@ func (channel *Channel) applyModeMemberNoMutex(client *Client, mode modes.Mode,
 
 	channel.stateMutex.Lock()
 	modeset, exists := channel.members[target]
-	var already bool
 	if exists {
-		enable := op == modes.Add
-		already = modeset[mode] == enable
-		modeset[mode] = enable
+		if applied := modeset.SetMode(mode, op == modes.Add); applied {
+			result = &modes.ModeChange{
+				Op:   op,
+				Mode: mode,
+				Arg:  nick,
+			}
+		}
 	}
 	channel.stateMutex.Unlock()
 
 	if !exists {
 		rb.Add(nil, client.server.name, ERR_USERNOTINCHANNEL, client.nick, channel.name, client.t("They aren't on that channel"))
-		return nil
-	} else if already {
-		return nil
-	} else {
-		return &modes.ModeChange{
-			Op:   op,
-			Mode: mode,
-			Arg:  nick,
-		}
 	}
+	return
 }
 
 // ShowMaskList shows the given list to the client.
@@ -790,7 +776,7 @@ func (channel *Channel) Quit(client *Client) {
 }
 
 func (channel *Channel) Kick(client *Client, target *Client, comment string, rb *ResponseBuffer) {
-	if !(client.flags[modes.Operator] || channel.hasClient(client)) {
+	if !(client.HasMode(modes.Operator) || channel.hasClient(client)) {
 		rb.Add(nil, client.server.name, ERR_NOTONCHANNEL, channel.name, client.t("You're not on that channel"))
 		return
 	}
@@ -823,7 +809,7 @@ func (channel *Channel) Kick(client *Client, target *Client, comment string, rb
 
 // Invite invites the given client to the channel, if the inviter can do so.
 func (channel *Channel) Invite(invitee *Client, inviter *Client, rb *ResponseBuffer) {
-	if channel.flags[modes.InviteOnly] && !channel.ClientIsAtLeast(inviter, modes.ChannelOperator) {
+	if channel.flags.HasMode(modes.InviteOnly) && !channel.ClientIsAtLeast(inviter, modes.ChannelOperator) {
 		rb.Add(nil, inviter.server.name, ERR_CHANOPRIVSNEEDED, channel.name, inviter.t("You're not a channel operator"))
 		return
 	}
@@ -834,7 +820,7 @@ func (channel *Channel) Invite(invitee *Client, inviter *Client, rb *ResponseBuf
 	}
 
 	//TODO(dan): handle this more nicely, keep a list of last X invited channels on invitee rather than explicitly modifying the invite list?
-	if channel.flags[modes.InviteOnly] {
+	if channel.flags.HasMode(modes.InviteOnly) {
 		nmc := invitee.NickCasefolded()
 		channel.stateMutex.Lock()
 		channel.lists[modes.InviteMask].Add(nmc)
@@ -850,7 +836,7 @@ func (channel *Channel) Invite(invitee *Client, inviter *Client, rb *ResponseBuf
 	//TODO(dan): should inviter.server.name here be inviter.nickMaskString ?
 	rb.Add(nil, inviter.server.name, RPL_INVITING, invitee.nick, channel.name)
 	invitee.Send(nil, inviter.nickMaskString, "INVITE", invitee.nick, channel.name)
-	if invitee.flags[modes.Away] {
+	if invitee.HasMode(modes.Away) {
 		rb.Add(nil, inviter.server.name, RPL_AWAY, invitee.nick, invitee.awayMessage)
 	}
 }

irc/chanserv.go

@@ -199,7 +199,7 @@ func csOpHandler(server *Server, client *Client, command, params string, rb *Res
 	if client == target {
 		givenMode = modes.ChannelFounder
 	}
-	change := channelInfo.applyModeMemberNoMutex(target, givenMode, modes.Add, client.NickCasefolded(), rb)
+	change := channelInfo.applyModeToMember(target, givenMode, modes.Add, client.NickCasefolded(), rb)
 	if change != nil {
 		//TODO(dan): we should change the name of String and make it return a slice here
 		//TODO(dan): unify this code with code in modes.go
@@ -260,7 +260,7 @@ func csRegisterHandler(server *Server, client *Client, command, params string, r
 	server.snomasks.Send(sno.LocalChannels, fmt.Sprintf(ircfmt.Unescape("Channel registered $c[grey][$r%s$c[grey]] by $c[grey][$r%s$c[grey]]"), channelName, client.nickMaskString))
 
 	// give them founder privs
-	change := channelInfo.applyModeMemberNoMutex(client, modes.ChannelFounder, modes.Add, client.NickCasefolded(), rb)
+	change := channelInfo.applyModeToMember(client, modes.ChannelFounder, modes.Add, client.NickCasefolded(), rb)
 	if change != nil {
 		//TODO(dan): we should change the name of String and make it return a slice here
 		//TODO(dan): unify this code with code in modes.go

irc/client.go

@@ -50,7 +50,7 @@ type Client struct {
 	ctime              time.Time
 	exitedSnomaskSent  bool
 	fakelag            *Fakelag
-	flags              map[modes.Mode]bool
+	flags              *modes.ModeSet
 	hasQuit            bool
 	hops               int
 	hostname           string
@@ -98,7 +98,7 @@ func NewClient(server *Server, conn net.Conn, isTLS bool) *Client {
 		capVersion:     caps.Cap301,
 		channels:       make(ChannelSet),
 		ctime:          now,
-		flags:          make(map[modes.Mode]bool),
+		flags:          modes.NewModeSet(),
 		server:         server,
 		socket:         socket,
 		nick:           "*", // * is used until actual nick is given
@@ -109,7 +109,7 @@ func NewClient(server *Server, conn net.Conn, isTLS bool) *Client {
 
 	client.recomputeMaxlens()
 	if isTLS {
-		client.flags[modes.TLS] = true
+		client.SetMode(modes.TLS, true)
 
 		// error is not useful to us here anyways so we can ignore it
 		client.certfp, _ = client.socket.CertFP()
@@ -504,13 +504,7 @@ func (client *Client) HasRoleCapabs(capabs ...string) bool {
 
 // ModeString returns the mode string for this client.
 func (client *Client) ModeString() (str string) {
-	str = "+"
-
-	for flag := range client.flags {
-		str += flag.String()
-	}
-
-	return
+	return "+" + client.flags.String()
 }
 
 // Friends refers to clients that share a channel with this client.

irc/config.go

@@ -21,6 +21,7 @@ import (
 	"github.com/oragono/oragono/irc/custime"
 	"github.com/oragono/oragono/irc/languages"
 	"github.com/oragono/oragono/irc/logger"
+	"github.com/oragono/oragono/irc/modes"
 	"github.com/oragono/oragono/irc/passwd"
 	"github.com/oragono/oragono/irc/utils"
 	"gopkg.in/yaml.v2"
@@ -352,7 +353,7 @@ type Oper struct {
 	WhoisLine string
 	Vhost     string
 	Pass      []byte
-	Modes     string
+	Modes     []modes.ModeChange
 }
 
 // Operators returns a map of operator configs from the given OperClass and config.
@@ -379,7 +380,12 @@ func (conf *Config) Operators(oc *map[string]OperClass) (map[string]Oper, error)
 		} else {
 			oper.WhoisLine = class.WhoisLine
 		}
-		oper.Modes = strings.TrimSpace(opConf.Modes)
+		modeStr := strings.TrimSpace(opConf.Modes)
+		modeChanges, unknownChanges := modes.ParseUserModeChanges(strings.Split(modeStr, " ")...)
+		if len(unknownChanges) > 0 {
+			return nil, fmt.Errorf("Could not load operator [%s] due to unknown modes %v", name, unknownChanges)
+		}
+		oper.Modes = modeChanges
 
 		// successful, attach to list of opers
 		operators[name] = oper

irc/gateways.go

@@ -81,11 +81,7 @@ func (client *Client) ApplyProxiedIP(proxiedIP string, tls bool) (exiting bool)
 
 	// set tls info
 	client.certfp = ""
-	if tls {
-		client.flags[modes.TLS] = true
-	} else {
-		delete(client.flags, modes.TLS)
-	}
+	client.SetMode(modes.TLS, tls)
 
 	return false
 }

irc/getters.go

@@ -188,9 +188,12 @@ func (client *Client) SetPreregNick(preregNick string) {
 }
 
 func (client *Client) HasMode(mode modes.Mode) bool {
-	client.stateMutex.RLock()
-	defer client.stateMutex.RUnlock()
-	return client.flags[mode]
+	// client.flags has its own synch
+	return client.flags.HasMode(mode)
+}
+
+func (client *Client) SetMode(mode modes.Mode, on bool) bool {
+	return client.flags.SetMode(mode, on)
 }
 
 func (client *Client) Channels() (result []*Channel) {
@@ -260,29 +263,8 @@ func (channel *Channel) setKey(key string) {
 	channel.key = key
 }
 
-func (channel *Channel) HasMode(mode modes.Mode) bool {
-	channel.stateMutex.RLock()
-	defer channel.stateMutex.RUnlock()
-	return channel.flags[mode]
-}
-
 func (channel *Channel) Founder() string {
 	channel.stateMutex.RLock()
 	defer channel.stateMutex.RUnlock()
 	return channel.registeredFounder
 }
-
-// set a channel mode, return whether it was already set
-func (channel *Channel) setMode(mode modes.Mode, enable bool) (already bool) {
-	channel.stateMutex.Lock()
-	already = (channel.flags[mode] == enable)
-	if !already {
-		if enable {
-			channel.flags[mode] = true
-		} else {
-			delete(channel.flags, mode)
-		}
-	}
-	channel.stateMutex.Unlock()
-	return
-}

irc/handlers.go

@@ -405,15 +405,11 @@ func awayHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp
 		}
 	}
 
-	if isAway {
-		client.flags[modes.Away] = true
-	} else {
-		delete(client.flags, modes.Away)
-	}
+	client.SetMode(modes.Away, isAway)
 	client.awayMessage = text
 
 	var op modes.ModeOp
-	if client.flags[modes.Away] {
+	if isAway {
 		op = modes.Add
 		rb.Add(nil, server.name, RPL_NOWAWAY, client.nick, client.t("You have been marked as being away"))
 	} else {
@@ -429,7 +425,7 @@ func awayHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp
 
 	// dispatch away-notify
 	for friend := range client.Friends(caps.AwayNotify) {
-		if client.flags[modes.Away] {
+		if isAway {
 			friend.SendFromClient("", client, nil, "AWAY", client.awayMessage)
 		} else {
 			friend.SendFromClient("", client, nil, "AWAY")
@@ -777,7 +773,7 @@ Get an explanation of <argument>, or "index" for a list of help topics.`), rb)
 
 	// handle index
 	if argument == "index" {
-		if client.flags[modes.Operator] {
+		if client.HasMode(modes.Operator) {
 			client.sendHelp("HELP", GetHelpIndex(client.languages, HelpIndexOpers), rb)
 		} else {
 			client.sendHelp("HELP", GetHelpIndex(client.languages, HelpIndex), rb)
@@ -787,7 +783,7 @@ Get an explanation of <argument>, or "index" for a list of help topics.`), rb)
 
 	helpHandler, exists := Help[argument]
 
-	if exists && (!helpHandler.oper || (helpHandler.oper && client.flags[modes.Operator])) {
+	if exists && (!helpHandler.oper || (helpHandler.oper && client.HasMode(modes.Operator))) {
 		if helpHandler.textGenerator != nil {
 			client.sendHelp(strings.ToUpper(argument), client.t(helpHandler.textGenerator(client)), rb)
 		} else {
@@ -1257,9 +1253,10 @@ func listHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp
 		}
 	}
 
+	clientIsOp := client.HasMode(modes.Operator)
 	if len(channels) == 0 {
 		for _, channel := range server.channels.Channels() {
-			if !client.flags[modes.Operator] && channel.flags[modes.Secret] {
+			if !clientIsOp && channel.flags.HasMode(modes.Secret) {
 				continue
 			}
 			if matcher.Matches(channel) {
@@ -1268,14 +1265,14 @@ func listHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp
 		}
 	} else {
 		// limit regular users to only listing one channel
-		if !client.flags[modes.Operator] {
+		if !clientIsOp {
 			channels = channels[:1]
 		}
 
 		for _, chname := range channels {
 			casefoldedChname, err := CasefoldChannel(chname)
 			channel := server.channels.Get(casefoldedChname)
-			if err != nil || channel == nil || (!client.flags[modes.Operator] && channel.flags[modes.Secret]) {
+			if err != nil || channel == nil || (!clientIsOp && channel.flags.HasMode(modes.Secret)) {
 				if len(chname) > 0 {
 					rb.Add(nil, server.name, ERR_NOSUCHCHANNEL, client.nick, chname, client.t("No such channel"))
 				}
@@ -1329,7 +1326,7 @@ func cmodeHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Res
 	if 1 < len(msg.Params) {
 		// parse out real mode changes
 		params := msg.Params[1:]
-		changes, unknown := ParseChannelModeChanges(params...)
+		changes, unknown := modes.ParseChannelModeChanges(params...)
 
 		// alert for unknown mode changes
 		for char := range unknown {
@@ -1415,14 +1412,14 @@ func umodeHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Res
 		}
 
 		// apply mode changes
-		applied = target.applyUserModeChanges(msg.Command == "SAMODE", changes)
+		applied = ApplyUserModeChanges(client, changes, msg.Command == "SAMODE")
 	}
 
 	if len(applied) > 0 {
 		rb.Add(nil, client.nickMaskString, "MODE", targetNick, applied.String())
 	} else if hasPrivs {
 		rb.Add(nil, target.nickMaskString, RPL_UMODEIS, targetNick, target.ModeString())
-		if client.flags[modes.LocalOperator] || client.flags[modes.Operator] {
+		if client.HasMode(modes.LocalOperator) || client.HasMode(modes.Operator) {
 			masks := server.snomasks.String(client)
 			if 0 < len(masks) {
 				rb.Add(nil, target.nickMaskString, RPL_SNOMASKIS, targetNick, masks, client.t("Server notice masks"))
@@ -1670,7 +1667,7 @@ func noticeHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Re
 			msgid := server.generateMessageID()
 			// restrict messages appropriately when +R is set
 			// intentionally make the sending user think the message went through fine
-			if !user.flags[modes.RegisteredOnly] || client.registered {
+			if !user.HasMode(modes.RegisteredOnly) || client.LoggedIntoAccount() {
 				user.SendSplitMsgFromClient(msgid, client, clientOnlyTags, "NOTICE", user.nick, splitMsg)
 			}
 			if client.capabilities.Has(caps.EchoMessage) {
@@ -1731,7 +1728,7 @@ func operHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp
 		rb.Add(nil, server.name, ERR_PASSWDMISMATCH, client.nick, client.t("Password incorrect"))
 		return true
 	}
-	if client.flags[modes.Operator] == true {
+	if client.HasMode(modes.Operator) == true {
 		rb.Add(nil, server.name, ERR_UNKNOWNERROR, "OPER", client.t("You're already opered-up!"))
 		return false
 	}
@@ -1760,37 +1757,23 @@ func operHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp
 		client.updateNickMask("")
 	}
 
-	// set new modes
-	var applied modes.ModeChanges
-	if 0 < len(oper.Modes) {
-		modeChanges, unknownChanges := modes.ParseUserModeChanges(strings.Split(oper.Modes, " ")...)
-		applied = client.applyUserModeChanges(true, modeChanges)
-		if 0 < len(unknownChanges) {
-			var runes string
-			for r := range unknownChanges {
-				runes += string(r)
-			}
-			rb.Notice(fmt.Sprintf(client.t("Could not apply mode changes: +%s"), runes))
-		}
+	// set new modes: modes.Operator, plus anything specified in the config
+	modeChanges := make([]modes.ModeChange, len(oper.Modes)+1)
+	modeChanges[0] = modes.ModeChange{
+		Mode: modes.Operator,
+		Op:   modes.Add,
 	}
+	copy(modeChanges[1:], oper.Modes)
+	applied := ApplyUserModeChanges(client, modeChanges, true)
 
 	rb.Add(nil, server.name, RPL_YOUREOPER, client.nick, client.t("You are now an IRC operator"))
-
-	applied = append(applied, modes.ModeChange{
-		Mode: modes.Operator,
-		Op:   modes.Add,
-	})
 	rb.Add(nil, server.name, "MODE", client.nick, applied.String())
 
 	server.snomasks.Send(sno.LocalOpers, fmt.Sprintf(ircfmt.Unescape("Client opered up $c[grey][$r%s$c[grey], $r%s$c[grey]]"), client.nickMaskString, client.operName))
 
-	// increase oper count
-	server.stats.ChangeOperators(1)
-
 	// client may now be unthrottled by the fakelag system
 	client.resetFakelag()
 
-	client.flags[modes.Operator] = true
 	return false
 }
 
@@ -1905,13 +1888,13 @@ func privmsgHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *R
 			msgid := server.generateMessageID()
 			// restrict messages appropriately when +R is set
 			// intentionally make the sending user think the message went through fine
-			if !user.flags[modes.RegisteredOnly] || client.registered {
+			if !user.HasMode(modes.RegisteredOnly) || client.LoggedIntoAccount() {
 				user.SendSplitMsgFromClient(msgid, client, clientOnlyTags, "PRIVMSG", user.nick, splitMsg)
 			}
 			if client.capabilities.Has(caps.EchoMessage) {
 				rb.AddSplitMessageFromClient(msgid, client, clientOnlyTags, "PRIVMSG", user.nick, splitMsg)
 			}
-			if user.flags[modes.Away] {
+			if user.HasMode(modes.Away) {
 				//TODO(dan): possibly implement cooldown of away notifications to users
 				rb.Add(nil, server.name, RPL_AWAY, user.nick, user.awayMessage)
 			}
@@ -2159,7 +2142,7 @@ func tagmsgHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Re
 			if client.capabilities.Has(caps.EchoMessage) {
 				rb.AddFromClient(msgid, client, clientOnlyTags, "TAGMSG", user.nick)
 			}
-			if user.flags[modes.Away] {
+			if user.HasMode(modes.Away) {
 				//TODO(dan): possibly implement cooldown of away notifications to users
 				rb.Add(nil, server.name, RPL_AWAY, user.nick, user.awayMessage)
 			}
@@ -2355,10 +2338,10 @@ func userhostHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *
 
 		var isOper, isAway string
 
-		if target.flags[modes.Operator] {
+		if target.HasMode(modes.Operator) {
 			isOper = "*"
 		}
-		if target.flags[modes.Away] {
+		if target.HasMode(modes.Away) {
 			isAway = "-"
 		} else {
 			isAway = "+"
@@ -2399,7 +2382,7 @@ func webircHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Re
 			lkey := strings.ToLower(key)
 			if lkey == "tls" || lkey == "secure" {
 				// only accept "tls" flag if the gateway's connection to us is secure as well
-				if client.flags[modes.TLS] || utils.AddrIsLocal(client.socket.conn.RemoteAddr()) {
+				if client.HasMode(modes.TLS) || utils.AddrIsLocal(client.socket.conn.RemoteAddr()) {
 					secure = true
 				}
 			}
@@ -2488,7 +2471,7 @@ func whoisHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Res
 		return false
 	}
 
-	if client.flags[modes.Operator] {
+	if client.HasMode(modes.Operator) {
 		masks := strings.Split(masksString, ",")
 		for _, mask := range masks {
 			casefoldedMask, err := Casefold(mask)

irc/modes.go

@@ -21,8 +21,8 @@ var (
 	}
 )
 
-// applyUserModeChanges applies the given changes, and returns the applied changes.
-func (client *Client) applyUserModeChanges(force bool, changes modes.ModeChanges) modes.ModeChanges {
+// ApplyUserModeChanges applies the given changes, and returns the applied changes.
+func ApplyUserModeChanges(client *Client, changes modes.ModeChanges, force bool) modes.ModeChanges {
 	applied := make(modes.ModeChanges, 0)
 
 	for _, change := range changes {
@@ -34,36 +34,28 @@ func (client *Client) applyUserModeChanges(force bool, changes modes.ModeChanges
 					continue
 				}
 
-				if client.flags[change.Mode] {
-					continue
-				}
-
-				if change.Mode == modes.Invisible {
-					client.server.stats.ChangeInvisible(1)
+				if changed := client.SetMode(change.Mode, true); changed {
+					if change.Mode == modes.Invisible {
+						client.server.stats.ChangeInvisible(1)
+					} else if change.Mode == modes.Operator || change.Mode == modes.LocalOperator {
+						client.server.stats.ChangeOperators(1)
+					}
+					applied = append(applied, change)
 				}
 
-				client.flags[change.Mode] = true
-				applied = append(applied, change)
-
 			case modes.Remove:
-				if !client.flags[change.Mode] {
-					continue
-				}
-
-				if change.Mode == modes.Invisible {
-					client.server.stats.ChangeInvisible(-1)
-				}
-
-				if change.Mode == modes.Operator || change.Mode == modes.LocalOperator {
-					client.server.stats.ChangeOperators(-1)
+				if changed := client.SetMode(change.Mode, false); changed {
+					if change.Mode == modes.Invisible {
+						client.server.stats.ChangeInvisible(-1)
+					} else if change.Mode == modes.Operator || change.Mode == modes.LocalOperator {
+						client.server.stats.ChangeOperators(-1)
+					}
+					applied = append(applied, change)
 				}
-
-				delete(client.flags, change.Mode)
-				applied = append(applied, change)
 			}
 
 		case modes.ServerNotice:
-			if !client.flags[modes.Operator] {
+			if !client.HasMode(modes.Operator) {
 				continue
 			}
 			var masks []sno.Mask
@@ -101,7 +93,7 @@ func ParseDefaultChannelModes(config *Config) modes.Modes {
 		return DefaultChannelModes
 	}
 	modeChangeStrings := strings.Split(strings.TrimSpace(*config.Channels.DefaultModes), " ")
-	modeChanges, _ := ParseChannelModeChanges(modeChangeStrings...)
+	modeChanges, _ := modes.ParseChannelModeChanges(modeChangeStrings...)
 	defaultChannelModes := make(modes.Modes, 0)
 	for _, modeChange := range modeChanges {
 		if modeChange.Op == modes.Add {
@@ -111,83 +103,6 @@ func ParseDefaultChannelModes(config *Config) modes.Modes {
 	return defaultChannelModes
 }
 
-// ParseChannelModeChanges returns the valid changes, and the list of unknown chars.
-func ParseChannelModeChanges(params ...string) (modes.ModeChanges, map[rune]bool) {
-	changes := make(modes.ModeChanges, 0)
-	unknown := make(map[rune]bool)
-
-	op := modes.List
-
-	if 0 < len(params) {
-		modeArg := params[0]
-		skipArgs := 1
-
-		for _, mode := range modeArg {
-			if mode == '-' || mode == '+' {
-				op = modes.ModeOp(mode)
-				continue
-			}
-			change := modes.ModeChange{
-				Mode: modes.Mode(mode),
-				Op:   op,
-			}
-
-			// put arg into modechange if needed
-			switch modes.Mode(mode) {
-			case modes.BanMask, modes.ExceptMask, modes.InviteMask:
-				if len(params) > skipArgs {
-					change.Arg = params[skipArgs]
-					skipArgs++
-				} else {
-					change.Op = modes.List
-				}
-			case modes.ChannelFounder, modes.ChannelAdmin, modes.ChannelOperator, modes.Halfop, modes.Voice:
-				if len(params) > skipArgs {
-					change.Arg = params[skipArgs]
-					skipArgs++
-				} else {
-					continue
-				}
-			case modes.Key, modes.UserLimit:
-				// don't require value when removing
-				if change.Op == modes.Add {
-					if len(params) > skipArgs {
-						change.Arg = params[skipArgs]
-						skipArgs++
-					} else {
-						continue
-					}
-				}
-			}
-
-			var isKnown bool
-			for _, supportedMode := range modes.SupportedChannelModes {
-				if rune(supportedMode) == mode {
-					isKnown = true
-					break
-				}
-			}
-			for _, supportedMode := range modes.ChannelPrivModes {
-				if rune(supportedMode) == mode {
-					isKnown = true
-					break
-				}
-			}
-			if mode == rune(modes.Voice) {
-				isKnown = true
-			}
-			if !isKnown {
-				unknown[mode] = true
-				continue
-			}
-
-			changes = append(changes, change)
-		}
-	}
-
-	return changes, unknown
-}
-
 // ApplyChannelModeChanges applies a given set of mode changes.
 func (channel *Channel) ApplyChannelModeChanges(client *Client, isSamode bool, changes modes.ModeChanges, rb *ResponseBuffer) modes.ModeChanges {
 	// so we only output one warning for each list type when full
@@ -208,15 +123,17 @@ func (channel *Channel) ApplyChannelModeChanges(client *Client, isSamode bool, c
 		}
 		switch change.Mode {
 		case modes.ChannelFounder, modes.ChannelAdmin, modes.ChannelOperator, modes.Halfop, modes.Voice:
-			// Admins can't give other people Admin or remove it from others
-			if change.Mode == modes.ChannelAdmin {
-				return false
-			}
+			// List on these modes is a no-op anyway
 			if change.Op == modes.List {
 				return true
 			}
 			cfarg, _ := CasefoldName(change.Arg)
-			if change.Op == modes.Remove && cfarg == client.nickCasefolded {
+			isSelfChange := cfarg == client.NickCasefolded()
+			// Admins can't give other people Admin or remove it from others
+			if change.Mode == modes.ChannelAdmin && !isSelfChange {
+				return false
+			}
+			if change.Op == modes.Remove && isSelfChange {
 				// "There is no restriction, however, on anyone `deopping' themselves"
 				// <https://tools.ietf.org/html/rfc2812#section-3.1.5>
 				return true
@@ -299,8 +216,7 @@ func (channel *Channel) ApplyChannelModeChanges(client *Client, isSamode bool, c
 				continue
 			}
 
-			already := channel.setMode(change.Mode, change.Op == modes.Add)
-			if !already {
+			if changed := channel.flags.SetMode(change.Mode, change.Op == modes.Add); changed {
 				applied = append(applied, change)
 			}
 
@@ -309,7 +225,13 @@ func (channel *Channel) ApplyChannelModeChanges(client *Client, isSamode bool, c
 				continue
 			}
 
-			change := channel.applyModeMemberNoMutex(client, change.Mode, change.Op, change.Arg, rb)
+			nick := change.Arg
+			if nick == "" {
+				rb.Add(nil, client.server.name, ERR_NEEDMOREPARAMS, "MODE", client.t("Not enough parameters"))
+				return nil
+			}
+
+			change := channel.applyModeToMember(client, change.Mode, change.Op, nick, rb)
 			if change != nil {
 				applied = append(applied, *change)
 			}

irc/modes/modes.go

@@ -7,6 +7,7 @@ package modes
 
 import (
 	"strings"
+	"sync"
 )
 
 var (
@@ -247,34 +248,156 @@ func ParseUserModeChanges(params ...string) (ModeChanges, map[rune]bool) {
 	return changes, unknown
 }
 
+// ParseChannelModeChanges returns the valid changes, and the list of unknown chars.
+func ParseChannelModeChanges(params ...string) (ModeChanges, map[rune]bool) {
+	changes := make(ModeChanges, 0)
+	unknown := make(map[rune]bool)
+
+	op := List
+
+	if 0 < len(params) {
+		modeArg := params[0]
+		skipArgs := 1
+
+		for _, mode := range modeArg {
+			if mode == '-' || mode == '+' {
+				op = ModeOp(mode)
+				continue
+			}
+			change := ModeChange{
+				Mode: Mode(mode),
+				Op:   op,
+			}
+
+			// put arg into modechange if needed
+			switch Mode(mode) {
+			case BanMask, ExceptMask, InviteMask:
+				if len(params) > skipArgs {
+					change.Arg = params[skipArgs]
+					skipArgs++
+				} else {
+					change.Op = List
+				}
+			case ChannelFounder, ChannelAdmin, ChannelOperator, Halfop, Voice:
+				if len(params) > skipArgs {
+					change.Arg = params[skipArgs]
+					skipArgs++
+				} else {
+					continue
+				}
+			case Key, UserLimit:
+				// don't require value when removing
+				if change.Op == Add {
+					if len(params) > skipArgs {
+						change.Arg = params[skipArgs]
+						skipArgs++
+					} else {
+						continue
+					}
+				}
+			}
+
+			var isKnown bool
+			for _, supportedMode := range SupportedChannelModes {
+				if rune(supportedMode) == mode {
+					isKnown = true
+					break
+				}
+			}
+			for _, supportedMode := range ChannelPrivModes {
+				if rune(supportedMode) == mode {
+					isKnown = true
+					break
+				}
+			}
+			if mode == rune(Voice) {
+				isKnown = true
+			}
+			if !isKnown {
+				unknown[mode] = true
+				continue
+			}
+
+			changes = append(changes, change)
+		}
+	}
+
+	return changes, unknown
+}
+
 // ModeSet holds a set of modes.
-type ModeSet map[Mode]bool
+type ModeSet struct {
+	sync.RWMutex // tier 0
+	modes        map[Mode]bool
+}
+
+// returns a pointer to a new ModeSet
+func NewModeSet() *ModeSet {
+	return &ModeSet{
+		modes: make(map[Mode]bool),
+	}
+}
+
+// test whether `mode` is set
+func (set *ModeSet) HasMode(mode Mode) bool {
+	set.RLock()
+	defer set.RUnlock()
+	return set.modes[mode]
+}
+
+// set `mode` to be on or off, return whether the value actually changed
+func (set *ModeSet) SetMode(mode Mode, on bool) (applied bool) {
+	set.Lock()
+	defer set.Unlock()
+
+	previouslyOn := set.modes[mode]
+	needsApply := (on != previouslyOn)
+	if on && needsApply {
+		set.modes[mode] = true
+	} else if !on && needsApply {
+		delete(set.modes, mode)
+	}
+	return needsApply
+}
+
+// return the modes in the set as a slice
+func (set *ModeSet) AllModes() (result []Mode) {
+	set.RLock()
+	defer set.RUnlock()
+
+	for mode := range set.modes {
+		result = append(result, mode)
+	}
+	return
+}
 
 // String returns the modes in this set.
-func (set ModeSet) String() string {
-	if len(set) == 0 {
+func (set *ModeSet) String() string {
+	set.RLock()
+	defer set.RUnlock()
+
+	if len(set.modes) == 0 {
 		return ""
 	}
-	strs := make([]string, len(set))
-	index := 0
-	for mode := range set {
-		strs[index] = mode.String()
-		index++
+	var result []byte
+	for mode := range set.modes {
+		result = append(result, mode.String()...)
 	}
-	return strings.Join(strs, "")
+	return string(result)
 }
 
 // Prefixes returns a list of prefixes for the given set of channel modes.
-func (set ModeSet) Prefixes(isMultiPrefix bool) string {
-	var prefixes string
+func (set *ModeSet) Prefixes(isMultiPrefix bool) (prefixes string) {
+	set.RLock()
+	defer set.RUnlock()
 
 	// add prefixes in order from highest to lowest privs
 	for _, mode := range ChannelPrivModes {
-		if set[mode] {
+		if set.modes[mode] {
 			prefixes += ChannelModePrefixes[mode]
 		}
 	}
-	if set[Voice] {
+	if set.modes[Voice] {
 		prefixes += ChannelModePrefixes[Voice]
 	}
 

irc/modes/modes_test.go

@@ -0,0 +1,37 @@
+// Copyright (c) 2018 Shivaram Lingamneni
+// released under the MIT license
+
+package modes
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestSetMode(t *testing.T) {
+	set := NewModeSet()
+
+	if applied := set.SetMode(Invisible, false); applied != false {
+		t.Errorf("all modes should be false by default")
+	}
+
+	if applied := set.SetMode(Invisible, true); applied != true {
+		t.Errorf("initial SetMode call should return true")
+	}
+
+	set.SetMode(Operator, true)
+
+	if applied := set.SetMode(Invisible, true); applied != false {
+		t.Errorf("redundant SetMode call should return false")
+	}
+
+	expected1 := []Mode{Invisible, Operator}
+	expected2 := []Mode{Operator, Invisible}
+	if allModes := set.AllModes(); !(reflect.DeepEqual(allModes, expected1) || reflect.DeepEqual(allModes, expected2)) {
+		t.Errorf("unexpected AllModes value: %v", allModes)
+	}
+
+	if modeString := set.String(); !(modeString == "io" || modeString == "oi") {
+		t.Errorf("unexpected modestring: %s", modeString)
+	}
+}

irc/roleplay.go

@@ -35,7 +35,7 @@ func sendRoleplayMessage(server *Server, client *Client, source string, targetSt
 			return
 		}
 
-		if !channel.flags[modes.ChanRoleplaying] {
+		if !channel.flags.HasMode(modes.ChanRoleplaying) {
 			rb.Add(nil, client.server.name, ERR_CANNOTSENDRP, channel.name, client.t("Channel doesn't have roleplaying mode available"))
 			return
 		}
@@ -58,7 +58,7 @@ func sendRoleplayMessage(server *Server, client *Client, source string, targetSt
 			return
 		}
 
-		if !user.flags[modes.UserRoleplaying] {
+		if !user.HasMode(modes.UserRoleplaying) {
 			rb.Add(nil, client.server.name, ERR_CANNOTSENDRP, user.nick, client.t("User doesn't have roleplaying mode enabled"))
 			return
 		}
@@ -67,7 +67,7 @@ func sendRoleplayMessage(server *Server, client *Client, source string, targetSt
 		if client.capabilities.Has(caps.EchoMessage) {
 			rb.Add(nil, source, "PRIVMSG", user.nick, message)
 		}
-		if user.flags[modes.Away] {
+		if user.HasMode(modes.Away) {
 			//TODO(dan): possibly implement cooldown of away notifications to users
 			rb.Add(nil, server.name, RPL_AWAY, user.nick, user.awayMessage)
 		}

irc/server.go

@@ -637,8 +637,8 @@ func (client *Client) WhoisChannelsNames(target *Client) []string {
 	var chstrs []string
 	for _, channel := range target.Channels() {
 		// channel is secret and the target can't see it
-		if !client.flags[modes.Operator] {
-			if (target.HasMode(modes.Invisible) || channel.HasMode(modes.Secret)) && !channel.hasClient(client) {
+		if !client.HasMode(modes.Operator) {
+			if (target.HasMode(modes.Invisible) || channel.flags.HasMode(modes.Secret)) && !channel.hasClient(client) {
 				continue
 			}
 		}
@@ -660,16 +660,16 @@ func (client *Client) getWhoisOf(target *Client, rb *ResponseBuffer) {
 	if target.class != nil {
 		rb.Add(nil, client.server.name, RPL_WHOISOPERATOR, client.nick, target.nick, target.whoisLine)
 	}
-	if client.flags[modes.Operator] || client == target {
+	if client.HasMode(modes.Operator) || client == target {
 		rb.Add(nil, client.server.name, RPL_WHOISACTUALLY, client.nick, target.nick, fmt.Sprintf("%s@%s", target.username, utils.LookupHostname(target.IPString())), target.IPString(), client.t("Actual user@host, Actual IP"))
 	}
-	if target.flags[modes.TLS] {
+	if target.HasMode(modes.TLS) {
 		rb.Add(nil, client.server.name, RPL_WHOISSECURE, client.nick, target.nick, client.t("is using a secure connection"))
 	}
 	if target.LoggedIntoAccount() {
 		rb.Add(nil, client.server.name, RPL_WHOISACCOUNT, client.nick, target.AccountName(), client.t("is logged in as"))
 	}
-	if target.flags[modes.Bot] {
+	if target.HasMode(modes.Bot) {
 		rb.Add(nil, client.server.name, RPL_WHOISBOT, client.nick, target.nick, ircfmt.Unescape(fmt.Sprintf(client.t("is a $bBot$b on %s"), client.server.networkName)))
 	}
 
@@ -682,7 +682,7 @@ func (client *Client) getWhoisOf(target *Client, rb *ResponseBuffer) {
 		rb.Add(nil, client.server.name, RPL_WHOISLANGUAGE, params...)
 	}
 
-	if target.certfp != "" && (client.flags[modes.Operator] || client == target) {
+	if target.certfp != "" && (client.HasMode(modes.Operator) || client == target) {
 		rb.Add(nil, client.server.name, RPL_WHOISCERTFP, client.nick, target.nick, fmt.Sprintf(client.t("has client certificate fingerprint %s"), target.certfp))
 	}
 	rb.Add(nil, client.server.name, RPL_WHOISIDLE, client.nick, target.nick, strconv.FormatUint(target.IdleSeconds(), 10), strconv.FormatInt(target.SignonTime(), 10), client.t("seconds idle, signon time"))
@@ -713,7 +713,7 @@ func (target *Client) rplWhoReply(channel *Channel, client *Client, rb *Response
 
 func whoChannel(client *Client, channel *Channel, friends ClientSet, rb *ResponseBuffer) {
 	for _, member := range channel.Members() {
-		if !client.flags[modes.Invisible] || friends[client] {
+		if !client.HasMode(modes.Invisible) || friends[client] {
 			client.rplWhoReply(channel, member, rb)
 		}
 	}
@@ -1209,7 +1209,7 @@ func (matcher *elistMatcher) Matches(channel *Channel) bool {
 func (target *Client) RplList(channel *Channel, rb *ResponseBuffer) {
 	// get the correct number of channel members
 	var memberCount int
-	if target.flags[modes.Operator] || channel.hasClient(target) {
+	if target.HasMode(modes.Operator) || channel.hasClient(target) {
 		memberCount = len(channel.Members())
 	} else {
 		for _, member := range channel.Members() {

irc/types.go

@@ -26,11 +26,11 @@ func (clients ClientSet) Has(client *Client) bool {
 }
 
 // MemberSet is a set of members with modes.
-type MemberSet map[*Client]modes.ModeSet
+type MemberSet map[*Client]*modes.ModeSet
 
 // Add adds the given client to this set.
 func (members MemberSet) Add(member *Client) {
-	members[member] = make(modes.ModeSet)
+	members[member] = modes.NewModeSet()
 }
 
 // Remove removes the given client from this set.
@@ -44,19 +44,10 @@ func (members MemberSet) Has(member *Client) bool {
 	return ok
 }
 
-// HasMode returns true if the given client is in this set with the given mode.
-func (members MemberSet) HasMode(member *Client, mode modes.Mode) bool {
-	modes, ok := members[member]
-	if !ok {
-		return false
-	}
-	return modes[mode]
-}
-
 // AnyHasMode returns true if any of our clients has the given mode.
 func (members MemberSet) AnyHasMode(mode modes.Mode) bool {
 	for _, modes := range members {
-		if modes[mode] {
+		if modes.HasMode(mode) {
 			return true
 		}
 	}