Merge pull request #377 from slingamn/kickfail

fix another three bugs that were reported today

irc/channel.go

@@ -241,12 +241,7 @@ 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"))
 }
 
-// ClientIsAtLeast returns whether the client has at least the given channel privilege.
+func channelUserModeIsAtLeast(clientModes *modes.ModeSet, permission modes.Mode) bool {
-func (channel *Channel) ClientIsAtLeast(client *Client, permission modes.Mode) bool {
-	channel.stateMutex.RLock()
-	clientModes := channel.members[client]
-	channel.stateMutex.RUnlock()
-
 	if clientModes == nil {
 		return false
 	}
@@ -264,6 +259,14 @@ func (channel *Channel) ClientIsAtLeast(client *Client, permission modes.Mode) b
 	return false
 }
 
+// ClientIsAtLeast returns whether the client has at least the given channel privilege.
+func (channel *Channel) ClientIsAtLeast(client *Client, permission modes.Mode) bool {
+	channel.stateMutex.RLock()
+	clientModes := channel.members[client]
+	channel.stateMutex.RUnlock()
+	return channelUserModeIsAtLeast(clientModes, permission)
+}
+
 func (channel *Channel) ClientPrefixes(client *Client, isMultiPrefix bool) string {
 	channel.stateMutex.RLock()
 	defer channel.stateMutex.RUnlock()
@@ -277,24 +280,26 @@ func (channel *Channel) ClientPrefixes(client *Client, isMultiPrefix bool) strin
 
 func (channel *Channel) ClientHasPrivsOver(client *Client, target *Client) bool {
 	channel.stateMutex.RLock()
-	defer channel.stateMutex.RUnlock()
-
 	clientModes := channel.members[client]
 	targetModes := channel.members[target]
-	result := false
+	channel.stateMutex.RUnlock()
-	for _, mode := range modes.ChannelPrivModes {
+
-		if clientModes.HasMode(mode) {
+	if clientModes.HasMode(modes.ChannelFounder) {
-			result = true
+		// founder can kick anyone
-			// admins cannot kick other admins
+		return true
-			if mode == modes.ChannelAdmin && targetModes.HasMode(modes.ChannelAdmin) {
+	} else if clientModes.HasMode(modes.ChannelAdmin) {
-				result = false
+		// admins cannot kick other admins
-			}
+		return !channelUserModeIsAtLeast(targetModes, modes.ChannelAdmin)
-			break
+	} else if clientModes.HasMode(modes.ChannelOperator) {
-		} else if targetModes.HasMode(mode) {
+		// operators *can* kick other operators
-			break
+		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 result
 }
 
 func (channel *Channel) hasClient(client *Client) bool {
@@ -952,10 +957,6 @@ func (channel *Channel) Kick(client *Client, target *Client, comment string, rb
 		rb.Add(nil, client.server.name, ERR_NOTONCHANNEL, channel.name, client.t("You're not on that channel"))
 		return
 	}
-	if !channel.ClientIsAtLeast(client, modes.ChannelOperator) {
-		rb.Add(nil, client.server.name, ERR_CANNOTSENDTOCHAN, channel.name, client.t("Cannot send to channel"))
-		return
-	}
 	if !channel.hasClient(target) {
 		rb.Add(nil, client.server.name, ERR_USERNOTINCHANNEL, client.nick, channel.name, client.t("They aren't on that channel"))
 		return

irc/handlers.go

@@ -555,19 +555,21 @@ func chathistoryHandler(server *Server, client *Client, msg ircmsg.IrcMessage, r
 		newRb := NewResponseBuffer(client)
 		newRb.Label = rb.Label // same label, new batch
 		// TODO: send `WARN CHATHISTORY MAX_MESSAGES_EXCEEDED` when appropriate
-		if !success {
+		if hist == nil {
-			newRb.Add(nil, server.name, "ERR", "CHATHISTORY", "NEED_MORE_PARAMS")
-		} else if hist == nil {
 			newRb.Add(nil, server.name, "ERR", "CHATHISTORY", "NO_SUCH_CHANNEL")
 		} else if len(items) == 0 {
 			newRb.Add(nil, server.name, "ERR", "CHATHISTORY", "NO_TEXT_TO_SEND")
+		} else if !success {
+			newRb.Add(nil, server.name, "ERR", "CHATHISTORY", "NEED_MORE_PARAMS")
 		}
 		newRb.Send(true)
 	}()
 
 	target := msg.Params[0]
 	channel = server.channels.Get(target)
-	if channel != nil {
+	if channel != nil && channel.hasClient(client) {
+		// "If [...] the user does not have permission to view the requested content, [...]
+		// NO_SUCH_CHANNEL SHOULD be returned"
 		hist = &channel.history
 	} else {
 		targetClient := server.clients.Get(target)
@@ -1019,7 +1021,7 @@ func historyHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *R
 	target := msg.Params[0]
 	var hist *history.Buffer
 	channel := server.channels.Get(target)
-	if channel != nil {
+	if channel != nil && channel.hasClient(client) {
 		hist = &channel.history
 	} else {
 		if strings.ToLower(target) == "me" {
@@ -1036,7 +1038,11 @@ func historyHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *R
 	}
 
 	if hist == nil {
-		rb.Add(nil, server.name, ERR_NOSUCHCHANNEL, client.Nick(), target, client.t("No such channel"))
+		if channel == nil {
+			rb.Add(nil, server.name, ERR_NOSUCHCHANNEL, client.Nick(), target, client.t("No such channel"))
+		} else {
+			rb.Add(nil, server.name, ERR_NOTONCHANNEL, client.Nick(), target, client.t("You're not on that channel"))
+		}
 		return false
 	}
 

irc/modes.go

@@ -204,11 +204,11 @@ func (channel *Channel) ApplyChannelModeChanges(client *Client, isSamode bool, c
 			switch change.Op {
 			case modes.Add:
 				channel.setKey(change.Arg)
-
+				applied = append(applied, change)
 			case modes.Remove:
 				channel.setKey("")
+				applied = append(applied, change)
 			}
-			applied = append(applied, change)
 
 		case modes.InviteOnly, modes.Moderated, modes.NoOutside, modes.OpOnlyTopic, modes.RegisteredOnly, modes.Secret, modes.ChanRoleplaying:
 			if change.Op == modes.List {