Try to recover from some bad CAP replies, docs
[quassel.git] / src / core / coresessioneventprocessor.cpp
index c7a5b5b..5b639b7 100644 (file)
@@ -155,7 +155,12 @@ void CoreSessionEventProcessor::processIrcEventCap(IrcEvent *e)
     // Handle capability negotiation
     // See: http://ircv3.net/specs/core/capability-negotiation-3.2.html
     // And: http://ircv3.net/specs/core/capability-negotiation-3.1.html
-    if (e->params().count() >= 3) {
+
+    // All commands require at least 2 parameters
+    if (!checkParamCount(e, 2))
+        return;
+
+    if (e->params().count() >= 2) {
         CoreNetwork *coreNet = coreNetwork(e);
         QString capCommand = e->params().at(1).trimmed().toUpper();
         if (capCommand == "LS" || capCommand == "NEW") {
@@ -172,17 +177,33 @@ void CoreSessionEventProcessor::processIrcEventCap(IrcEvent *e)
             } else {
                 // Single line reply
                 capListFinished = true;
-                availableCaps = e->params().at(2).split(' ');
+                if (e->params().count() >= 3) {
+                    // Some capabilities are specified, add them
+                    availableCaps = e->params().at(2).split(' ');
+                } else {
+                    // No capabilities available, add an empty list
+                    availableCaps = QStringList();
+                }
             }
+            // Sort capabilities before requesting for consistency among networks.  This may avoid
+            // unexpected cases when some networks offer capabilities in a different order than
+            // others.  It also looks nicer in logs.  Not required.
+            availableCaps.sort();
             // Store what capabilities are available
-            QStringList availableCapPair;
+            QString availableCapName, availableCapValue;
             for (int i = 0; i < availableCaps.count(); ++i) {
                 // Capability may include values, e.g. CAP * LS :multi-prefix sasl=EXTERNAL
-                availableCapPair = availableCaps[i].trimmed().split('=');
-                if(availableCapPair.count() >= 2) {
-                    coreNet->addCap(availableCapPair.at(0).trimmed().toLower(), availableCapPair.at(1).trimmed());
-                } else {
-                    coreNet->addCap(availableCapPair.at(0).trimmed().toLower());
+                // Capability name comes before the first '='.  If no '=' exists, this gets the
+                // whole string instead.
+                availableCapName = availableCaps[i].section('=', 0, 0).trimmed();
+                // Some capabilities include multiple key=value pairs in the listing,
+                // e.g. "sts=duration=31536000,port=6697"
+                // Include everything after the first equal sign as part of the value.  If no '='
+                // exists, this gets an empty string.
+                availableCapValue = availableCaps[i].section('=', 1).trimmed();
+                // Only add the capability if it's non-empty
+                if (!availableCapName.isEmpty()) {
+                    coreNet->addCap(availableCapName, availableCapValue);
                 }
             }
 
@@ -190,6 +211,13 @@ void CoreSessionEventProcessor::processIrcEventCap(IrcEvent *e)
             if (capListFinished)
                 coreNet->beginCapNegotiation();
         } else if (capCommand == "ACK") {
+            // CAP ACK requires at least 3 parameters (no empty response allowed)
+            if (!checkParamCount(e, 3)) {
+                // If an invalid reply is sent, try to continue rather than getting stuck.
+                coreNet->sendNextCap();
+                return;
+            }
+
             // Server: CAP * ACK :multi-prefix sasl
             // Got the capability we want, handle as needed.
             // As only one capability is requested at a time, no need to split
@@ -205,6 +233,16 @@ void CoreSessionEventProcessor::processIrcEventCap(IrcEvent *e)
                 coreNet->sendNextCap();
             }
         } else if (capCommand == "NAK" || capCommand == "DEL") {
+            // CAP NAK/DEL require at least 3 parameters (no empty response allowed)
+            if (!checkParamCount(e, 3)) {
+                if (capCommand == "NAK") {
+                    // If an invalid reply is sent, try to continue rather than getting stuck.  This
+                    // only matters for denied caps, not removed caps.
+                    coreNet->sendNextCap();
+                }
+                return;
+            }
+
             // Either something went wrong with this capability, or it is no longer supported
             // > For CAP NAK
             // Server: CAP * NAK :multi-prefix sasl
@@ -224,8 +262,8 @@ void CoreSessionEventProcessor::processIrcEventCap(IrcEvent *e)
             }
 
             if (capCommand == "NAK") {
-                // Continue negotiation when capability listing complete only if this is the result
-                // of a denied cap, not a removed cap
+                // Continue negotiation only if this is the result of a denied cap, not a removed
+                // cap
                 coreNet->sendNextCap();
             }
         }
@@ -253,13 +291,17 @@ void CoreSessionEventProcessor::processIrcEventAccount(IrcEvent *e)
 /* IRCv3 away-notify - ":nick!user@host AWAY [:message]" */
 void CoreSessionEventProcessor::processIrcEventAway(IrcEvent *e)
 {
-    if (!checkParamCount(e, 2))
+    if (!checkParamCount(e, 1))
         return;
+    // Don't use checkParamCount(e, 2) since the message is optional.  Some servers respond in a way
+    // that it counts as two parameters, but we shouldn't rely on that.
 
     // Nick is sent as part of parameters in order to split user/server decoding
     IrcUser *ircuser = e->network()->ircUser(e->params().at(0));
     if (ircuser) {
-        if (!e->params().at(1).isEmpty()) {
+        // If two parameters are sent -and- the second parameter isn't empty, then user is away.
+        // Otherwise, mark them as not away.
+        if (e->params().count() >= 2 && !e->params().at(1).isEmpty()) {
             ircuser->setAway(true);
             ircuser->setAwayMessage(e->params().at(1));
         } else {
@@ -881,7 +923,9 @@ void CoreSessionEventProcessor::processIrcEvent324(IrcEvent *e)
 /*  RPL_WHOISACCOUNT - "<nick> <account> :is authed as" */
 void CoreSessionEventProcessor::processIrcEvent330(IrcEvent *e)
 {
-    if (!checkParamCount(e, 3))
+    // Though the ":is authed as" remark should always be there, we should handle cases when it's
+    // not included, too.
+    if (!checkParamCount(e, 2))
         return;
 
     IrcUser *ircuser = e->network()->ircUser(e->params().at(0));
@@ -1100,7 +1144,9 @@ void CoreSessionEventProcessor::processWhoInformation (Network *net, const QStri
 void CoreSessionEventProcessor::processIrcEvent403(IrcEventNumeric *e)
 {
     // If this is the result of an AutoWho, hide it.  It's confusing to show to the user.
-    if (!checkParamCount(e, 2))
+    // Though the ":No such channel" remark should always be there, we should handle cases when it's
+    // not included, too.
+    if (!checkParamCount(e, 1))
         return;
 
     QString channelOrNick = e->params()[0];