Merge pull request #886 from slingamn/issue874.2

fix #874

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))
 	}

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

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 {