diff --git a/irc/channel.go b/irc/channel.go index c7100c6b..09cb373c 100644 --- a/irc/channel.go +++ b/irc/channel.go @@ -1260,22 +1260,20 @@ func (channel *Channel) SendSplitMessage(command string, minPrefixMode modes.Mod }) } -func (channel *Channel) applyModeToMember(client *Client, mode modes.Mode, op modes.ModeOp, nick string, rb *ResponseBuffer) (result *modes.ModeChange) { - target := channel.server.clients.Get(nick) +func (channel *Channel) applyModeToMember(client *Client, change modes.ModeChange, rb *ResponseBuffer) (applied bool, result modes.ModeChange) { + target := channel.server.clients.Get(change.Arg) if target == nil { - rb.Add(nil, client.server.name, ERR_NOSUCHNICK, client.Nick(), utils.SafeErrorParam(nick), client.t("No such nick")) - return nil + rb.Add(nil, client.server.name, ERR_NOSUCHNICK, client.Nick(), utils.SafeErrorParam(change.Arg), client.t("No such nick")) + return } + change.Arg = target.Nick() channel.stateMutex.Lock() modeset, exists := channel.members[target] if exists { - if modeset.SetMode(mode, op == modes.Add) { - result = &modes.ModeChange{ - Op: op, - Mode: mode, - Arg: nick, - } + if modeset.SetMode(change.Mode, change.Op == modes.Add) { + applied = true + result = change } } channel.stateMutex.Unlock() diff --git a/irc/chanserv.go b/irc/chanserv.go index e1d690d0..1aa53132 100644 --- a/irc/chanserv.go +++ b/irc/chanserv.go @@ -238,7 +238,16 @@ func csAmodeHandler(server *Server, client *Client, command string, params []str } case modes.Add, modes.Remove: if len(affectedModes) > 0 { - csNotice(rb, fmt.Sprintf(client.t("Successfully set mode %s"), change.String())) + csNotice(rb, fmt.Sprintf(client.t("Successfully set persistent mode %s%s on %s"), string(change.Op), string(change.Mode), change.Arg)) + // #729: apply change to current membership + for _, member := range channel.Members() { + if member.Account() == change.Arg { + applied, change := channel.applyModeToMember(client, change, rb) + if applied { + announceCmodeChanges(channel, modes.ModeChanges{change}, chanservMask, rb) + } + } + } } else { csNotice(rb, client.t("No changes were made")) } @@ -275,14 +284,14 @@ func csOpHandler(server *Server, client *Client, command string, params []string if clientAccount == target.Account() { givenMode = modes.ChannelFounder } - change := channelInfo.applyModeToMember(client, givenMode, modes.Add, target.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 - args := append([]string{channelName}, strings.Split(change.String(), " ")...) - for _, member := range channelInfo.Members() { - member.Send(nil, fmt.Sprintf("ChanServ!services@%s", client.server.name), "MODE", args...) - } + applied, change := channelInfo.applyModeToMember(client, + modes.ModeChange{Mode: givenMode, + Op: modes.Add, + Arg: target.NickCasefolded(), + }, + rb) + if applied { + announceCmodeChanges(channelInfo, modes.ModeChanges{change}, chanservMask, rb) } csNotice(rb, fmt.Sprintf(client.t("Successfully op'd in channel %s"), channelName)) @@ -326,14 +335,15 @@ func csRegisterHandler(server *Server, client *Client, command string, params [] 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.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 - args := append([]string{channelName}, strings.Split(change.String(), " ")...) - for _, member := range channelInfo.Members() { - member.Send(nil, fmt.Sprintf("ChanServ!services@%s", client.server.name), "MODE", args...) - } + applied, change := channelInfo.applyModeToMember(client, + modes.ModeChange{ + Mode: modes.ChannelFounder, + Op: modes.Add, + Arg: client.NickCasefolded(), + }, + rb) + if applied { + announceCmodeChanges(channelInfo, modes.ModeChanges{change}, chanservMask, rb) } } diff --git a/irc/handlers.go b/irc/handlers.go index c25b95e4..11c57c5b 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -1530,38 +1530,25 @@ func cmodeHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Res } // process mode changes, include list operations (an empty set of changes does a list) applied := channel.ApplyChannelModeChanges(client, msg.Command == "SAMODE", changes, rb) + announceCmodeChanges(channel, applied, client.NickMaskString(), rb) - // save changes - var includeFlags uint - for _, change := range applied { - includeFlags |= IncludeModes - if change.Mode == modes.BanMask || change.Mode == modes.ExceptMask || change.Mode == modes.InviteMask { - includeFlags |= IncludeLists - } - } - - if includeFlags != 0 { - channel.MarkDirty(includeFlags) - } + return false +} +func announceCmodeChanges(channel *Channel, applied modes.ModeChanges, source string, rb *ResponseBuffer) { // send out changes if len(applied) > 0 { - prefix := client.NickMaskString() //TODO(dan): we should change the name of String and make it return a slice here - args := append([]string{channel.name}, strings.Split(applied.String(), " ")...) - rb.Add(nil, prefix, "MODE", args...) - for _, session := range client.Sessions() { - if session != rb.session { - session.Send(nil, prefix, "MODE", args...) - } - } + args := append([]string{channel.name}, applied.Strings()...) + rb.Add(nil, source, "MODE", args...) for _, member := range channel.Members() { - if member != client { - member.Send(nil, prefix, "MODE", args...) + for _, session := range member.Sessions() { + if session != rb.session { + session.Send(nil, source, "MODE", args...) + } } } } - return false } // MODE [ [...]] @@ -1606,7 +1593,8 @@ func umodeHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Res } if len(applied) > 0 { - rb.Add(nil, cDetails.nickMask, "MODE", targetNick, applied.String()) + args := append([]string{targetNick}, applied.Strings()...) + rb.Add(nil, cDetails.nickMask, "MODE", args...) } else if hasPrivs { rb.Add(nil, server.name, RPL_UMODEIS, targetNick, target.ModeString()) if target.HasMode(modes.LocalOperator) || target.HasMode(modes.Operator) { @@ -2122,7 +2110,8 @@ func applyOper(client *Client, oper *Oper, rb *ResponseBuffer) { client.server.snomasks.Send(sno.LocalOpers, fmt.Sprintf(ircfmt.Unescape("Client opered up $c[grey][$r%s$c[grey], $r%s$c[grey]]"), newDetails.nickMask, oper.Name)) rb.Broadcast(nil, client.server.name, RPL_YOUREOPER, details.nick, client.t("You are now an IRC operator")) - rb.Broadcast(nil, client.server.name, "MODE", details.nick, applied.String()) + args := append([]string{details.nick}, applied.Strings()...) + rb.Broadcast(nil, client.server.name, "MODE", args...) } else { client.server.snomasks.Send(sno.LocalOpers, fmt.Sprintf(ircfmt.Unescape("Client deopered $c[grey][$r%s$c[grey]]"), newDetails.nickMask)) } diff --git a/irc/modes.go b/irc/modes.go index 5d2b7e0c..e6f2c04e 100644 --- a/irc/modes.go +++ b/irc/modes.go @@ -256,13 +256,28 @@ func (channel *Channel) ApplyChannelModeChanges(client *Client, isSamode bool, c continue } - change := channel.applyModeToMember(client, change.Mode, change.Op, nick, rb) - if change != nil { - applied = append(applied, *change) + success, change := channel.applyModeToMember(client, change, rb) + if success { + applied = append(applied, change) } } } + var includeFlags uint + for _, change := range applied { + switch change.Mode { + case modes.BanMask, modes.ExceptMask, modes.InviteMask: + includeFlags |= IncludeLists + case modes.ChannelFounder, modes.ChannelAdmin, modes.ChannelOperator, modes.Halfop, modes.Voice: + // these are never persisted currently, but might be in the future (see discussion on #729) + default: + includeFlags |= IncludeModes + } + } + if includeFlags != 0 { + channel.MarkDirty(includeFlags) + } + // #649: don't send 324 RPL_CHANNELMODEIS if we were only working with mask lists if len(applied) == 0 && !alreadySentPrivError && (maskOpCount == 0 || maskOpCount < len(changes)) { args := append([]string{details.nick, chname}, channel.modeStrings(client)...) diff --git a/irc/modes/modes.go b/irc/modes/modes.go index 075bbb96..9e311298 100644 --- a/irc/modes/modes.go +++ b/irc/modes/modes.go @@ -28,10 +28,6 @@ var ( // ModeOp is an operation performed with modes type ModeOp rune -func (op ModeOp) String() string { - return string(op) -} - const ( // Add is used when adding the given key. Add ModeOp = '+' @@ -55,43 +51,36 @@ type ModeChange struct { Arg string } -func (change *ModeChange) String() (str string) { - if (change.Op == Add) || (change.Op == Remove) { - str = change.Op.String() - } - str += change.Mode.String() - if change.Arg != "" { - str += " " + change.Arg - } - return -} - // ModeChanges are a collection of 'ModeChange's type ModeChanges []ModeChange -func (changes ModeChanges) String() string { +func (changes ModeChanges) Strings() (result []string) { if len(changes) == 0 { - return "" + return } + var builder strings.Builder + op := changes[0].Op - str := changes[0].Op.String() + builder.WriteRune(rune(op)) for _, change := range changes { if change.Op != op { op = change.Op - str += change.Op.String() + builder.WriteRune(rune(op)) } - str += change.Mode.String() + builder.WriteRune(rune(change.Mode)) } + result = append(result, builder.String()) + for _, change := range changes { if change.Arg == "" { continue } - str += " " + change.Arg + result = append(result, change.Arg) } - return str + return } // Modes is just a raw list of modes diff --git a/irc/modes/modes_test.go b/irc/modes/modes_test.go index a3087761..858fa836 100644 --- a/irc/modes/modes_test.go +++ b/irc/modes/modes_test.go @@ -219,6 +219,22 @@ func TestHighestChannelUserMode(t *testing.T) { } } +func TestModeChangesString(t *testing.T) { + m := ModeChanges{ + ModeChange{Op: Add, Mode: RegisteredOnly}, + ModeChange{Op: Add, Mode: Key, Arg: "beer"}, + ModeChange{Op: Add, Mode: BanMask, Arg: "shivaram"}, + } + assertEqual(m.Strings(), []string{"+Rkb", "beer", "shivaram"}, t) + + m = ModeChanges{ + ModeChange{Op: Add, Mode: RegisteredOnly}, + ModeChange{Op: Remove, Mode: Key, Arg: "beer"}, + ModeChange{Op: Add, Mode: BanMask, Arg: "shivaram"}, + } + assertEqual(m.Strings(), []string{"+R-k+b", "beer", "shivaram"}, t) +} + func BenchmarkModeString(b *testing.B) { set := NewModeSet() set.SetMode('A', true)