diff --git a/irc/config.go b/irc/config.go index 94689f71..d7407b81 100644 --- a/irc/config.go +++ b/irc/config.go @@ -1113,7 +1113,7 @@ func (config *Config) generateISupport() (err error) { isupport.Add("AWAYLEN", strconv.Itoa(config.Limits.AwayLen)) isupport.Add("CASEMAPPING", "ascii") isupport.Add("CHANLIMIT", fmt.Sprintf("%s:%d", chanTypes, config.Channels.MaxChannelsPerClient)) - isupport.Add("CHANMODES", strings.Join([]string{modes.Modes{modes.BanMask, modes.ExceptMask, modes.InviteMask}.String(), "", modes.Modes{modes.UserLimit, modes.Key}.String(), modes.Modes{modes.InviteOnly, modes.Moderated, modes.NoOutside, modes.OpOnlyTopic, modes.ChanRoleplaying, modes.Secret, modes.NoCTCP, modes.RegisteredOnly}.String()}, ",")) + isupport.Add("CHANMODES", strings.Join([]string{modes.Modes{modes.BanMask, modes.ExceptMask, modes.InviteMask}.String(), modes.Modes{modes.Key}.String(), modes.Modes{modes.UserLimit}.String(), modes.Modes{modes.InviteOnly, modes.Moderated, modes.NoOutside, modes.OpOnlyTopic, modes.ChanRoleplaying, modes.Secret, modes.NoCTCP, modes.RegisteredOnly}.String()}, ",")) if config.History.Enabled && config.History.ChathistoryMax > 0 { isupport.Add("draft/CHATHISTORY", strconv.Itoa(config.History.ChathistoryMax)) } diff --git a/irc/modes/modes.go b/irc/modes/modes.go index 64bb56ff..075bbb96 100644 --- a/irc/modes/modes.go +++ b/irc/modes/modes.go @@ -281,7 +281,7 @@ func ParseChannelModeChanges(params ...string) (ModeChanges, map[rune]bool) { } else { continue } - case Key, UserLimit: + case UserLimit: // don't require value when removing if change.Op == Add { if len(params) > skipArgs { @@ -291,6 +291,23 @@ func ParseChannelModeChanges(params ...string) (ModeChanges, map[rune]bool) { continue } } + case Key: + // #874: +k is technically a type B mode, requiring a parameter + // both for add and remove. so attempt to consume a parameter, + // but allow remove (but not add) even if no parameter is available. + // however, the remove parameter should always display as "*", matching + // the freenode behavior. + if len(params) > skipArgs { + if change.Op == Add { + change.Arg = params[skipArgs] + } + skipArgs++ + } else if change.Op == Add { + continue + } + if change.Op == Remove { + change.Arg = "*" + } } var isKnown bool diff --git a/irc/modes/modes_test.go b/irc/modes/modes_test.go index 8572986e..71734e31 100644 --- a/irc/modes/modes_test.go +++ b/irc/modes/modes_test.go @@ -8,6 +8,95 @@ import ( "testing" ) +func assertEqual(supplied, expected interface{}, t *testing.T) { + if !reflect.DeepEqual(supplied, expected) { + t.Errorf("expected %v but got %v", expected, supplied) + } +} + +func TestIssue874(t *testing.T) { + emptyUnknown := make(map[rune]bool) + modes, unknown := ParseChannelModeChanges("+k") + assertEqual(unknown, emptyUnknown, t) + assertEqual(modes, ModeChanges{}, t) + + modes, unknown = ParseChannelModeChanges("+k", "beer") + assertEqual(unknown, emptyUnknown, t) + assertEqual(modes, ModeChanges{ModeChange{Op: Add, Mode: Key, Arg: "beer"}}, t) + + modes, unknown = ParseChannelModeChanges("-k") + assertEqual(unknown, emptyUnknown, t) + assertEqual(modes, ModeChanges{ModeChange{Op: Remove, Mode: Key, Arg: "*"}}, t) + + modes, unknown = ParseChannelModeChanges("-k", "beer") + assertEqual(unknown, emptyUnknown, t) + assertEqual(modes, ModeChanges{ModeChange{Op: Remove, Mode: Key, Arg: "*"}}, t) + + modes, unknown = ParseChannelModeChanges("+kb", "beer") + assertEqual(unknown, emptyUnknown, t) + assertEqual(modes, ModeChanges{ + ModeChange{Op: Add, Mode: Key, Arg: "beer"}, + ModeChange{Op: List, Mode: BanMask, Arg: ""}, + }, t) + + modes, unknown = ParseChannelModeChanges("+kb", "beer") + assertEqual(unknown, emptyUnknown, t) + assertEqual(modes, ModeChanges{ + ModeChange{Op: Add, Mode: Key, Arg: "beer"}, + ModeChange{Op: List, Mode: BanMask, Arg: ""}, + }, t) + + modes, unknown = ParseChannelModeChanges("-kb", "beer") + assertEqual(unknown, emptyUnknown, t) + assertEqual(modes, ModeChanges{ + ModeChange{Op: Remove, Mode: Key, Arg: "*"}, + ModeChange{Op: List, Mode: BanMask, Arg: ""}, + }, t) + + // "beer" is the ban arg, +k with no arg should be ignored + modes, unknown = ParseChannelModeChanges("+bk", "beer") + assertEqual(unknown, emptyUnknown, t) + assertEqual(modes, ModeChanges{ + ModeChange{Op: Add, Mode: BanMask, Arg: "beer"}, + }, t) + + // "beer" is the ban arg again + modes, unknown = ParseChannelModeChanges("-bk", "beer") + assertEqual(unknown, emptyUnknown, t) + assertEqual(modes, ModeChanges{ + ModeChange{Op: Remove, Mode: BanMask, Arg: "beer"}, + ModeChange{Op: Remove, Mode: Key, Arg: "*"}, + }, t) + + modes, unknown = ParseChannelModeChanges("+bk", "shivaram", "beer") + assertEqual(unknown, emptyUnknown, t) + assertEqual(modes, ModeChanges{ + ModeChange{Op: Add, Mode: BanMask, Arg: "shivaram"}, + ModeChange{Op: Add, Mode: Key, Arg: "beer"}, + }, t) + + modes, unknown = ParseChannelModeChanges("+kb", "beer", "shivaram") + assertEqual(unknown, emptyUnknown, t) + assertEqual(modes, ModeChanges{ + ModeChange{Op: Add, Mode: Key, Arg: "beer"}, + ModeChange{Op: Add, Mode: BanMask, Arg: "shivaram"}, + }, t) + + modes, unknown = ParseChannelModeChanges("-bk", "shivaram", "beer") + assertEqual(unknown, emptyUnknown, t) + assertEqual(modes, ModeChanges{ + ModeChange{Op: Remove, Mode: BanMask, Arg: "shivaram"}, + ModeChange{Op: Remove, Mode: Key, Arg: "*"}, + }, t) + + modes, unknown = ParseChannelModeChanges("-kb", "beer", "shivaram") + assertEqual(unknown, emptyUnknown, t) + assertEqual(modes, ModeChanges{ + ModeChange{Op: Remove, Mode: Key, Arg: "*"}, + ModeChange{Op: Remove, Mode: BanMask, Arg: "shivaram"}, + }, t) +} + func TestParseChannelModeChanges(t *testing.T) { modes, unknown := ParseChannelModeChanges("+h", "wrmsr") if len(unknown) > 0 {