Merge pull request #467 from slingamn/issue400.3

three fixes

irc/channel.go

@@ -335,38 +335,44 @@ func (channel *Channel) regenerateMembersCache() {
 
 // Names sends the list of users joined to the channel to the given client.
 func (channel *Channel) Names(client *Client, rb *ResponseBuffer) {
+	isJoined := channel.hasClient(client)
 	isMultiPrefix := rb.session.capabilities.Has(caps.MultiPrefix)
 	isUserhostInNames := rb.session.capabilities.Has(caps.UserhostInNames)
 
 	maxNamLen := 480 - len(client.server.name) - len(client.Nick())
 	var namesLines []string
 	var buffer bytes.Buffer
-	for _, target := range channel.Members() {
-		var nick string
-		if isUserhostInNames {
-			nick = target.NickMaskString()
-		} else {
-			nick = target.Nick()
-		}
-		channel.stateMutex.RLock()
-		modes := channel.members[target]
-		channel.stateMutex.RUnlock()
-		if modes == nil {
-			continue
-		}
-		prefix := modes.Prefixes(isMultiPrefix)
-		if buffer.Len()+len(nick)+len(prefix)+1 > maxNamLen {
-			namesLines = append(namesLines, buffer.String())
-			buffer.Reset()
+	if isJoined || !channel.flags.HasMode(modes.Secret) {
+		for _, target := range channel.Members() {
+			var nick string
+			if isUserhostInNames {
+				nick = target.NickMaskString()
+			} else {
+				nick = target.Nick()
+			}
+			channel.stateMutex.RLock()
+			modeSet := channel.members[target]
+			channel.stateMutex.RUnlock()
+			if modeSet == nil {
+				continue
+			}
+			if !isJoined && target.flags.HasMode(modes.Invisible) {
+				continue
+			}
+			prefix := modeSet.Prefixes(isMultiPrefix)
+			if buffer.Len()+len(nick)+len(prefix)+1 > maxNamLen {
+				namesLines = append(namesLines, buffer.String())
+				buffer.Reset()
+			}
+			if buffer.Len() > 0 {
+				buffer.WriteString(" ")
+			}
+			buffer.WriteString(prefix)
+			buffer.WriteString(nick)
 		}
 		if buffer.Len() > 0 {
-			buffer.WriteString(" ")
+			namesLines = append(namesLines, buffer.String())
 		}
-		buffer.WriteString(prefix)
-		buffer.WriteString(nick)
-	}
-	if buffer.Len() > 0 {
-		namesLines = append(namesLines, buffer.String())
 	}
 
 	for _, line := range namesLines {
@@ -377,22 +383,22 @@ func (channel *Channel) Names(client *Client, rb *ResponseBuffer) {
 	rb.Add(nil, client.server.name, RPL_ENDOFNAMES, client.nick, channel.name, client.t("End of NAMES list"))
 }
 
-func channelUserModeIsAtLeast(clientModes *modes.ModeSet, permission modes.Mode) bool {
-	if clientModes == nil {
+// does `clientMode` give you privileges to grant/remove `targetMode` to/from people,
+// or to kick them?
+func channelUserModeHasPrivsOver(clientMode modes.Mode, targetMode modes.Mode) bool {
+	switch clientMode {
+	case modes.ChannelFounder:
+		return true
+	case modes.ChannelAdmin, modes.ChannelOperator:
+		// admins cannot kick other admins, operators *can* kick other operators
+		return targetMode != modes.ChannelFounder && targetMode != modes.ChannelAdmin
+	case modes.Halfop:
+		// halfops cannot kick other halfops
+		return targetMode != modes.ChannelFounder && targetMode != modes.ChannelAdmin && targetMode != modes.Halfop
+	default:
+		// voice and unprivileged cannot kick anyone
 		return false
 	}
-
-	for _, mode := range modes.ChannelUserModes {
-		if clientModes.HasMode(mode) {
-			return true
-		}
-
-		if mode == permission {
-			break
-		}
-	}
-
-	return false
 }
 
 // ClientIsAtLeast returns whether the client has at least the given channel privilege.
@@ -400,7 +406,16 @@ func (channel *Channel) ClientIsAtLeast(client *Client, permission modes.Mode) b
 	channel.stateMutex.RLock()
 	clientModes := channel.members[client]
 	channel.stateMutex.RUnlock()
-	return channelUserModeIsAtLeast(clientModes, permission)
+
+	for _, mode := range modes.ChannelUserModes {
+		if clientModes.HasMode(mode) {
+			return true
+		}
+		if mode == permission {
+			break
+		}
+	}
+	return false
 }
 
 func (channel *Channel) ClientPrefixes(client *Client, isMultiPrefix bool) string {
@@ -420,28 +435,13 @@ func (channel *Channel) ClientHasPrivsOver(client *Client, target *Client) bool
 	targetModes := channel.members[target]
 	channel.stateMutex.RUnlock()
 
-	if clientModes.HasMode(modes.ChannelFounder) {
-		// founder can kick anyone
-		return true
-	} else if clientModes.HasMode(modes.ChannelAdmin) {
-		// admins cannot kick other admins
-		return !channelUserModeIsAtLeast(targetModes, modes.ChannelAdmin)
-	} else if clientModes.HasMode(modes.ChannelOperator) {
-		// operators *can* kick other operators
-		return !channelUserModeIsAtLeast(targetModes, modes.ChannelAdmin)
-	} else if clientModes.HasMode(modes.Halfop) {
-		// halfops cannot kick other halfops
-		return !channelUserModeIsAtLeast(targetModes, modes.Halfop)
-	} else {
-		// voice and unprivileged cannot kick anyone
-		return false
-	}
+	return channelUserModeHasPrivsOver(clientModes.HighestChannelUserMode(), targetModes.HighestChannelUserMode())
 }
 
 func (channel *Channel) hasClient(client *Client) bool {
 	channel.stateMutex.RLock()
-	defer channel.stateMutex.RUnlock()
 	_, present := channel.members[client]
+	channel.stateMutex.RUnlock()
 	return present
 }
 
@@ -902,7 +902,7 @@ func msgCommandToHistType(server *Server, command string) (history.ItemType, err
 	}
 }
 
-func (channel *Channel) SendSplitMessage(command string, minPrefix *modes.Mode, clientOnlyTags map[string]string, client *Client, message utils.SplitMessage, rb *ResponseBuffer) {
+func (channel *Channel) SendSplitMessage(command string, minPrefixMode modes.Mode, clientOnlyTags map[string]string, client *Client, message utils.SplitMessage, rb *ResponseBuffer) {
 	histType, err := msgCommandToHistType(channel.server, command)
 	if err != nil {
 		return
@@ -920,11 +920,11 @@ func (channel *Channel) SendSplitMessage(command string, minPrefix *modes.Mode,
 	chname := channel.Name()
 	now := time.Now().UTC()
 
-	// for STATUSMSG
-	var minPrefixMode modes.Mode
-	if minPrefix != nil {
-		minPrefixMode = *minPrefix
+	// STATUSMSG targets are prefixed with the supplied min-prefix, e.g., @#channel
+	if minPrefixMode != modes.Mode(0) {
+		chname = fmt.Sprintf("%s%s", modes.ChannelModePrefixes[minPrefixMode], chname)
 	}
+
 	// send echo-message
 	// TODO this should use `now` as the time for consistency
 	if rb.session.capabilities.Has(caps.EchoMessage) {
@@ -959,7 +959,7 @@ func (channel *Channel) SendSplitMessage(command string, minPrefix *modes.Mode,
 		if member == client {
 			continue
 		}
-		if minPrefix != nil && !channel.ClientIsAtLeast(member, minPrefixMode) {
+		if minPrefixMode != modes.Mode(0) && !channel.ClientIsAtLeast(member, minPrefixMode) {
 			// STATUSMSG
 			continue
 		}

irc/getters.go

@@ -352,3 +352,10 @@ func (channel *Channel) DirtyBits() (dirtyBits uint) {
 	channel.stateMutex.Unlock()
 	return
 }
+
+func (channel *Channel) HighestUserMode(client *Client) (result modes.Mode) {
+	channel.stateMutex.RLock()
+	clientModes := channel.members[client]
+	channel.stateMutex.RUnlock()
+	return clientModes.HighestChannelUserMode()
+}

irc/modes.go

@@ -128,16 +128,12 @@ func (channel *Channel) ApplyChannelModeChanges(client *Client, isSamode bool, c
 			}
 			cfarg, _ := CasefoldName(change.Arg)
 			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
 			}
-			return channel.ClientIsAtLeast(client, change.Mode)
+			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)
@@ -254,13 +250,6 @@ func umodeGreaterThan(l modes.Mode, r modes.Mode) bool {
 
 // ProcessAccountToUmodeChange processes Add/Remove/List operations for channel persistent usermodes.
 func (channel *Channel) ProcessAccountToUmodeChange(client *Client, change modes.ModeChange) (results []modes.ModeChange, err error) {
-	hasPrivsOver := func(l modes.Mode, r modes.Mode) bool {
-		if l == modes.ChannelAdmin {
-			return umodeGreaterThan(l, r)
-		}
-		return l == r || umodeGreaterThan(l, r)
-	}
-
 	changed := false
 	defer func() {
 		if changed {
@@ -284,9 +273,9 @@ func (channel *Channel) ProcessAccountToUmodeChange(client *Client, change modes
 	// operators and founders can do anything
 	hasPrivs := isOperChange || (account != "" && account == channel.registeredFounder)
 	// halfop and up can list, and do add/removes at levels <= their own
-	if change.Op == modes.List && hasPrivsOver(clientMode, modes.Halfop) {
+	if change.Op == modes.List && (clientMode == modes.Halfop || umodeGreaterThan(clientMode, modes.Halfop)) {
 		hasPrivs = true
-	} else if hasPrivsOver(clientMode, modes.Halfop) && hasPrivsOver(clientMode, targetModeNow) && hasPrivsOver(clientMode, targetModeAfter) {
+	} else if channelUserModeHasPrivsOver(clientMode, targetModeNow) && channelUserModeHasPrivsOver(clientMode, targetModeAfter) {
 		hasPrivs = true
 	}
 	if !hasPrivs {

irc/modes/modes.go

@@ -143,12 +143,6 @@ var (
 	Halfop          Mode = 'h' // arg
 	Voice           Mode = 'v' // arg
 
-	// ChannelPrivModes holds the list of modes that are privileged, ie founder/op/halfop, in order.
-	// voice is not in this list because it cannot perform channel operator actions.
-	ChannelPrivModes = Modes{
-		ChannelFounder, ChannelAdmin, ChannelOperator, Halfop,
-	}
-
 	// ChannelUserModes holds the list of all modes that can be applied to a user in a channel,
 	// including Voice, in descending order of precedence
 	ChannelUserModes = Modes{
@@ -171,23 +165,24 @@ var (
 // SplitChannelMembershipPrefixes takes a target and returns the prefixes on it, then the name.
 func SplitChannelMembershipPrefixes(target string) (prefixes string, name string) {
 	name = target
-	for {
-		if len(name) > 0 && strings.Contains("~&@%+", string(name[0])) {
-			prefixes += string(name[0])
-			name = name[1:]
-		} else {
+	for i := 0; i < len(name); i++ {
+		switch name[i] {
+		case '~', '&', '@', '%', '+':
+			prefixes = target[:i+1]
+			name = target[i+1:]
+		default:
 			break
 		}
 	}
 
-	return prefixes, name
+	return
 }
 
 // GetLowestChannelModePrefix returns the lowest channel prefix mode out of the given prefixes.
-func GetLowestChannelModePrefix(prefixes string) (lowest *Mode) {
+func GetLowestChannelModePrefix(prefixes string) (lowest Mode) {
 	for i, mode := range ChannelUserModes {
 		if strings.Contains(prefixes, ChannelModePrefixes[mode]) {
-			lowest = &ChannelPrivModes[i]
+			lowest = ChannelUserModes[i]
 		}
 	}
 	return
@@ -405,3 +400,15 @@ func (set *ModeSet) Prefixes(isMultiPrefix bool) (prefixes string) {
 
 	return prefixes
 }
+
+// HighestChannelUserMode returns the most privileged channel-user mode
+// (e.g., ChannelFounder, Halfop, Voice) present in the ModeSet.
+// If no such modes are present, or `set` is nil, returns the zero mode.
+func (set *ModeSet) HighestChannelUserMode() (result Mode) {
+	for _, mode := range ChannelUserModes {
+		if set.HasMode(mode) {
+			return mode
+		}
+	}
+	return
+}

irc/modes/modes_test.go

@@ -90,3 +90,26 @@ func TestNilReceivers(t *testing.T) {
 		t.Errorf("nil Modeset should have empty String(), got %v instead", str)
 	}
 }
+
+func TestHighestChannelUserMode(t *testing.T) {
+	set := NewModeSet()
+
+	if set.HighestChannelUserMode() != Mode(0) {
+		t.Errorf("no channel user modes should be present yet")
+	}
+
+	set.SetMode(Voice, true)
+	if set.HighestChannelUserMode() != Voice {
+		t.Errorf("should see that user is voiced")
+	}
+
+	set.SetMode(ChannelAdmin, true)
+	if set.HighestChannelUserMode() != ChannelAdmin {
+		t.Errorf("should see that user has channel admin")
+	}
+
+	set = nil
+	if set.HighestChannelUserMode() != Mode(0) {
+		t.Errorf("nil modeset should have the zero mode as highest channel-user mode")
+	}
+}

irc/modes_test.go

@@ -48,3 +48,22 @@ func TestUmodeGreaterThan(t *testing.T) {
 		t.Errorf("modes should not be greater than themselves")
 	}
 }
+
+func assertEqual(supplied, expected interface{}, t *testing.T) {
+	if !reflect.DeepEqual(supplied, expected) {
+		t.Errorf("expected %v but got %v", expected, supplied)
+	}
+}
+
+func TestChannelUserModeHasPrivsOver(t *testing.T) {
+	assertEqual(channelUserModeHasPrivsOver(modes.Voice, modes.Halfop), false, t)
+	assertEqual(channelUserModeHasPrivsOver(modes.Mode(0), modes.Halfop), false, t)
+	assertEqual(channelUserModeHasPrivsOver(modes.Voice, modes.Mode(0)), false, t)
+	assertEqual(channelUserModeHasPrivsOver(modes.ChannelAdmin, modes.ChannelAdmin), false, t)
+	assertEqual(channelUserModeHasPrivsOver(modes.Halfop, modes.Halfop), false, t)
+	assertEqual(channelUserModeHasPrivsOver(modes.Voice, modes.Voice), false, t)
+
+	assertEqual(channelUserModeHasPrivsOver(modes.Halfop, modes.Voice), true, t)
+	assertEqual(channelUserModeHasPrivsOver(modes.ChannelFounder, modes.ChannelAdmin), true, t)
+	assertEqual(channelUserModeHasPrivsOver(modes.ChannelOperator, modes.ChannelOperator), true, t)
+}