diff --git a/irc/client_lookup_set.go b/irc/client_lookup_set.go index edf792fe..87775fad 100644 --- a/irc/client_lookup_set.go +++ b/irc/client_lookup_set.go @@ -268,10 +268,10 @@ func NewUserMaskSet() *UserMaskSet { } // Add adds the given mask to this set. -func (set *UserMaskSet) Add(mask, creatorNickmask, creatorAccount string) (added bool) { +func (set *UserMaskSet) Add(mask, creatorNickmask, creatorAccount string) (maskAdded string, err error) { casefoldedMask, err := CanonicalizeMaskWildcard(mask) if err != nil { - return false + return } set.Lock() @@ -279,8 +279,8 @@ func (set *UserMaskSet) Add(mask, creatorNickmask, creatorAccount string) (added set.masks = make(map[string]MaskInfo) } _, present := set.masks[casefoldedMask] - added = !present - if added { + if !present { + maskAdded = casefoldedMask set.masks[casefoldedMask] = MaskInfo{ TimeCreated: time.Now().UTC(), CreatorNickmask: creatorNickmask, @@ -289,22 +289,23 @@ func (set *UserMaskSet) Add(mask, creatorNickmask, creatorAccount string) (added } set.Unlock() - if added { + if !present { set.setRegexp() } return } // Remove removes the given mask from this set. -func (set *UserMaskSet) Remove(mask string) (removed bool) { - mask, err := CanonicalizeMaskWildcard(mask) +func (set *UserMaskSet) Remove(mask string) (maskRemoved string, err error) { + mask, err = CanonicalizeMaskWildcard(mask) if err != nil { - return false + return } set.Lock() - _, removed = set.masks[mask] + _, removed := set.masks[mask] if removed { + maskRemoved = mask delete(set.masks, mask) } set.Unlock() diff --git a/irc/handlers.go b/irc/handlers.go index 66329f4b..82077aa7 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -1685,13 +1685,12 @@ func cmodeHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Res return false } - // applied mode changes - applied := make(modes.ModeChanges, 0) - + var changes modes.ModeChanges if 1 < len(msg.Params) { // parse out real mode changes params := msg.Params[1:] - changes, unknown := modes.ParseChannelModeChanges(params...) + var unknown map[rune]bool + changes, unknown = modes.ParseChannelModeChanges(params...) // alert for unknown mode changes for char := range unknown { @@ -1700,10 +1699,9 @@ func cmodeHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Res if len(unknown) == 1 && len(changes) == 0 { return false } - - // apply mode changes - applied = channel.ApplyChannelModeChanges(client, msg.Command == "SAMODE", changes, rb) } + // process mode changes, include list operations (an empty set of changes does a list) + applied := channel.ApplyChannelModeChanges(client, msg.Command == "SAMODE", changes, rb) // save changes var includeFlags uint @@ -1719,8 +1717,8 @@ func cmodeHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Res } // send out changes - prefix := client.NickMaskString() 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(), " ")...) for _, member := range channel.Members() { @@ -1735,10 +1733,6 @@ func cmodeHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Res member.Send(nil, prefix, "MODE", args...) } } - } else { - args := append([]string{client.nick, channel.name}, channel.modeStrings(client)...) - rb.Add(nil, prefix, RPL_CHANNELMODEIS, args...) - rb.Add(nil, client.nickMaskString, RPL_CREATIONTIME, client.nick, channel.name, strconv.FormatInt(channel.createdTime.Unix(), 10)) } return false } diff --git a/irc/modes.go b/irc/modes.go index b05eb6ee..66c17859 100644 --- a/irc/modes.go +++ b/irc/modes.go @@ -6,6 +6,7 @@ package irc import ( + "fmt" "strconv" "strings" @@ -104,17 +105,15 @@ func ParseDefaultChannelModes(rawModes *string) modes.Modes { } // ApplyChannelModeChanges applies a given set of mode changes. -func (channel *Channel) ApplyChannelModeChanges(client *Client, isSamode bool, changes modes.ModeChanges, rb *ResponseBuffer) modes.ModeChanges { +func (channel *Channel) ApplyChannelModeChanges(client *Client, isSamode bool, changes modes.ModeChanges, rb *ResponseBuffer) (applied modes.ModeChanges) { // so we only output one warning for each list type when full listFullWarned := make(map[modes.Mode]bool) var alreadySentPrivError bool - applied := make(modes.ModeChanges, 0) - - isListOp := func(change modes.ModeChange) bool { - return (change.Op == modes.List) || (change.Arg == "") - } + maskOpCount := 0 + chname := channel.Name() + details := client.Details() hasPrivs := func(change modes.ModeChange) bool { if isSamode { @@ -127,18 +126,19 @@ func (channel *Channel) ApplyChannelModeChanges(client *Client, isSamode bool, c return true } cfarg, _ := CasefoldName(change.Arg) - isSelfChange := cfarg == client.NickCasefolded() + isSelfChange := cfarg == details.nickCasefolded if change.Op == modes.Remove && isSelfChange { // "There is no restriction, however, on anyone `deopping' themselves" // return true } return channelUserModeHasPrivsOver(channel.HighestUserMode(client), change.Mode) - case modes.BanMask: - // #163: allow unprivileged users to list ban masks - return isListOp(change) || channel.ClientIsAtLeast(client, modes.ChannelOperator) - default: + case modes.InviteMask, modes.ExceptMask: + // listing these requires privileges return channel.ClientIsAtLeast(client, modes.ChannelOperator) + default: + // #163: allow unprivileged users to list ban masks, and any other modes + return change.Op == modes.List || channel.ClientIsAtLeast(client, modes.ChannelOperator) } } @@ -146,14 +146,15 @@ func (channel *Channel) ApplyChannelModeChanges(client *Client, isSamode bool, c if !hasPrivs(change) { if !alreadySentPrivError { alreadySentPrivError = true - rb.Add(nil, client.server.name, ERR_CHANOPRIVSNEEDED, client.Nick(), channel.name, client.t("You're not a channel operator")) + rb.Add(nil, client.server.name, ERR_CHANOPRIVSNEEDED, details.nick, channel.name, client.t("You're not a channel operator")) } continue } switch change.Mode { case modes.BanMask, modes.ExceptMask, modes.InviteMask: - if isListOp(change) { + maskOpCount += 1 + if change.Op == modes.List { channel.ShowMaskList(client, change.Mode, rb) continue } @@ -163,20 +164,33 @@ func (channel *Channel) ApplyChannelModeChanges(client *Client, isSamode bool, c case modes.Add: if channel.lists[change.Mode].Length() >= client.server.Config().Limits.ChanListModes { if !listFullWarned[change.Mode] { - rb.Add(nil, client.server.name, ERR_BANLISTFULL, client.Nick(), channel.Name(), change.Mode.String(), client.t("Channel list is full")) + rb.Add(nil, client.server.name, ERR_BANLISTFULL, details.nick, chname, change.Mode.String(), client.t("Channel list is full")) listFullWarned[change.Mode] = true } continue } - details := client.Details() - if success := channel.lists[change.Mode].Add(mask, details.nickMask, details.accountName); success { - applied = append(applied, change) + maskAdded, err := channel.lists[change.Mode].Add(mask, details.nickMask, details.accountName) + if maskAdded != "" { + appliedChange := change + appliedChange.Arg = maskAdded + applied = append(applied, appliedChange) + } else if err != nil { + rb.Add(nil, client.server.name, ERR_INVALIDMODEPARAM, details.nick, mask, fmt.Sprintf(client.t("Invalid mode %s parameter: %s"), string(change.Mode), mask)) + } else { + rb.Add(nil, client.server.name, ERR_LISTMODEALREADYSET, chname, mask, string(change.Mode), fmt.Sprintf(client.t("Channel %s list already contains %s"), chname, mask)) } case modes.Remove: - if success := channel.lists[change.Mode].Remove(mask); success { - applied = append(applied, change) + maskRemoved, err := channel.lists[change.Mode].Remove(mask) + if maskRemoved != "" { + appliedChange := change + appliedChange.Arg = maskRemoved + applied = append(applied, appliedChange) + } else if err != nil { + rb.Add(nil, client.server.name, ERR_INVALIDMODEPARAM, details.nick, mask, fmt.Sprintf(client.t("Invalid mode %s parameter: %s"), string(change.Mode), mask)) + } else { + rb.Add(nil, client.server.name, ERR_LISTMODENOTSET, chname, mask, string(change.Mode), fmt.Sprintf(client.t("Channel %s list does not contain %s"), chname, mask)) } } @@ -221,7 +235,7 @@ func (channel *Channel) ApplyChannelModeChanges(client *Client, isSamode bool, c nick := change.Arg if nick == "" { rb.Add(nil, client.server.name, ERR_NEEDMOREPARAMS, client.Nick(), "MODE", client.t("Not enough parameters")) - return nil + continue } change := channel.applyModeToMember(client, change.Mode, change.Op, nick, rb) @@ -231,6 +245,13 @@ func (channel *Channel) ApplyChannelModeChanges(client *Client, isSamode bool, c } } + // #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)...) + rb.Add(nil, client.server.name, RPL_CHANNELMODEIS, args...) + rb.Add(nil, client.server.name, RPL_CREATIONTIME, details.nick, chname, strconv.FormatInt(channel.createdTime.Unix(), 10)) + } + return applied } diff --git a/irc/modes/modes_test.go b/irc/modes/modes_test.go index de1761c4..8572986e 100644 --- a/irc/modes/modes_test.go +++ b/irc/modes/modes_test.go @@ -47,6 +47,19 @@ func TestParseChannelModeChanges(t *testing.T) { if len(modes) != 1 || modes[0] != expected { t.Errorf("unexpected mode change: %v", modes) } + + modes, unknown = ParseChannelModeChanges("+b") + if len(unknown) > 0 { + t.Errorf("unexpected unknown mode change: %v", unknown) + } + // +b with no argument becomes a list operation + expectedChanges := ModeChanges{{ + Op: List, + Mode: BanMask, + }} + if !reflect.DeepEqual(modes, expectedChanges) { + t.Errorf("unexpected mode change: %v instead of %v", modes, expectedChanges) + } } func TestSetMode(t *testing.T) { diff --git a/irc/numerics.go b/irc/numerics.go index 640bff50..e4af7a5c 100644 --- a/irc/numerics.go +++ b/irc/numerics.go @@ -169,6 +169,9 @@ const ( RPL_YOURLANGUAGESARE = "687" ERR_CHANNAMEINUSE = "692" ERR_CANNOTRENAME = "693" + ERR_INVALIDMODEPARAM = "696" + ERR_LISTMODEALREADYSET = "697" + ERR_LISTMODENOTSET = "698" RPL_HELPSTART = "704" RPL_HELPTXT = "705" RPL_ENDOFHELP = "706"