Changeset 293 in code for trunk

Timestamp:

May 21, 2020, 8:36:54 PM (5 years ago)

Author:

delthas

Message:

Fix parsing MODE messages by updating channel memberships

Previously, we only considered channel modes in the modes of a MODE
messages, which means channel membership changes were ignored. This
resulted in bugs where users channel memberships would not be properly
updated and cached with wrong values. Further, mode arguments
representing entities were not properly marshaled.

This adds support for correctly parsing and updating channel memberships
when processing MODE messages. Mode arguments corresponding to channel
memberships updates are now also properly marshaled.

MODE messages can't be easily sent from history because marshaling these
messages require knowing about the upstream available channel types and
channel membership types, which is currently only possible when
connected. For now this is not an issue since we do not send MODE
messages in history.

Location:

trunk

Files:

• downstream.go (modified) (2 diffs)
• irc.go (modified) (3 diffs)
• upstream.go (modified) (2 diffs)

trunk/downstream.go

@@ -269 +269 @@
 // marshalMessage re-formats a message coming from an upstream connection so
 // that it's suitable for being sent on this downstream connection. Only
-// messages that may appear in logs are supported.
+// messages that may appear in logs are supported, except MODE.
 func (dc *downstreamConn) marshalMessage(msg *irc.Message, net *network) *irc.Message {
         msg = msg.Copy()
@@ -286 +286 @@
                 msg.Params[1] = dc.marshalEntity(net, msg.Params[1])
         case "TOPIC":
-                msg.Params[0] = dc.marshalEntity(net, msg.Params[0])
-        case "MODE":
                 msg.Params[0] = dc.marshalEntity(net, msg.Params[0])
         case "QUIT":

trunk/irc.go

@@ -85 +85 @@
 type channelModes map[byte]string
 
-func (cm channelModes) Apply(modeTypes map[byte]channelModeType, modeStr string, arguments ...string) error {
+// applyChannelModes parses a mode string and mode arguments from a MODE message,
+// and applies the corresponding channel mode and user membership changes on that channel.
+//
+// If ch.modes is nil, channel modes are not updated.
+//
+// needMarshaling is a list of indexes of mode arguments that represent entities
+// that must be marshaled when sent downstream.
+func applyChannelModes(ch *upstreamChannel, modeStr string, arguments []string) (needMarshaling map[int]struct{}, err error) {
+        needMarshaling = make(map[int]struct{}, len(arguments))
         nextArgument := 0
         var plusMinus byte
+outer:
         for i := 0; i < len(modeStr); i++ {
                 mode := modeStr[i]
@@ -95 +104 @@
                 }
                 if plusMinus != '+' && plusMinus != '-' {
-                        return fmt.Errorf("malformed modestring %q: missing plus/minus", modeStr)
-                }
-
-                mt, ok := modeTypes[mode]
+                        return nil, fmt.Errorf("malformed modestring %q: missing plus/minus", modeStr)
+                }
+
+                for _, membership := range ch.conn.availableMemberships {
+                        if membership.Mode == mode {
+                                if nextArgument >= len(arguments) {
+                                        return nil, fmt.Errorf("malformed modestring %q: missing mode argument for %c%c", modeStr, plusMinus, mode)
+                                }
+                                member := arguments[nextArgument]
+                                if _, ok := ch.Members[member]; ok {
+                                        if plusMinus == '+' {
+                                                ch.Members[member].Add(ch.conn.availableMemberships, membership)
+                                        } else {
+                                                // TODO: for upstreams without multi-prefix, query the user modes again
+                                                ch.Members[member].Remove(membership)
+                                        }
+                                }
+                                needMarshaling[nextArgument] = struct{}{}
+                                nextArgument++
+                                continue outer
+                        }
+                }
+
+                mt, ok := ch.conn.availableChannelModes[mode]
                 if !ok {
                         continue
@@ -110 +139 @@
                                         argument = arguments[nextArgument]
                                 }
-                                cm[mode] = argument
+                                if ch.modes != nil {
+                                        ch.modes[mode] = argument
+                                }
                         } else {
-                                delete(cm, mode)
+                                delete(ch.modes, mode)
                         }
                         nextArgument++
                 } else if mt == modeTypeC || mt == modeTypeD {
                         if plusMinus == '+' {
-                                cm[mode] = ""
+                                if ch.modes != nil {
+                                        ch.modes[mode] = ""
+                                }
                         } else {
-                                delete(cm, mode)
-                        }
-                }
-        }
-        return nil
+                                delete(ch.modes, mode)
+                        }
+                }
+        }
+        return needMarshaling, nil
 }
 

trunk/upstream.go

@@ -818 +818 @@
                         }
 
-                        if ch.modes != nil {
-                                if err := ch.modes.Apply(uc.availableChannelModes, modeStr, msg.Params[2:]...); err != nil {
-                                        return err
+                        needMarshaling, err := applyChannelModes(ch, modeStr, msg.Params[2:])
+                        if err != nil {
+                                return err
+                        }
+
+                        uc.appendLog(ch.Name, msg)
+                        uc.forEachDownstream(func(dc *downstreamConn) {
+                                params := make([]string, len(msg.Params))
+                                params[0] = dc.marshalEntity(uc.network, name)
+                                params[1] = modeStr
+
+                                copy(params[2:], msg.Params[2:])
+                                for i, modeParam := range params[2:] {
+                                        if _, ok := needMarshaling[i]; ok {
+                                                params[2+i] = dc.marshalEntity(uc.network, modeParam)
+                                        }
                                 }
-                        }
-
-                        uc.produce(ch.Name, msg, nil)
+
+                                dc.SendMessage(&irc.Message{
+                                        Prefix:  dc.marshalUserPrefix(uc.network, msg.Prefix),
+                                        Command: "MODE",
+                                        Params:  params,
+                                })
+                        })
                 }
         case irc.RPL_UMODEIS:
@@ -857 +874 @@
                 firstMode := ch.modes == nil
                 ch.modes = make(map[byte]string)
-                if err := ch.modes.Apply(uc.availableChannelModes, modeStr, msg.Params[3:]...); err != nil {
+                if _, err := applyChannelModes(ch, modeStr, msg.Params[3:]); err != nil {
                         return err
                 }