modernize: Use ranged-for loops in some obvious cases
[quassel.git] / src / core / coresessioneventprocessor.cpp
index 69d7569..e0fa892 100644 (file)
@@ -1,5 +1,5 @@
 /***************************************************************************
- *   Copyright (C) 2005-2016 by the Quassel Project                        *
+ *   Copyright (C) 2005-2018 by the Quassel Project                        *
  *   devel@quassel-irc.org                                                 *
  *                                                                         *
  *   This program is free software; you can redistribute it and/or modify  *
@@ -28,7 +28,7 @@
 #include "ctcpevent.h"
 #include "ircevent.h"
 #include "ircuser.h"
-#include "logger.h"
+#include "logmessage.h"
 #include "messageevent.h"
 #include "netsplit.h"
 #include "quassel.h"
@@ -155,79 +155,143 @@ 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) {
-        CoreNetwork *coreNet = coreNetwork(e);
-        QString capCommand = e->params().at(1).trimmed().toUpper();
-        if (capCommand == "LS" || capCommand == "NEW") {
-            // Either we've gotten a list of capabilities, or new capabilities we may want
-            // Server: CAP * LS * :multi-prefix extended-join account-notify batch invite-notify tls
-            // Server: CAP * LS * :cap-notify server-time example.org/dummy-cap=dummyvalue example.org/second-dummy-cap
-            // Server: CAP * LS :userhost-in-names sasl=EXTERNAL,DH-AES,DH-BLOWFISH,ECDSA-NIST256P-CHALLENGE,PLAIN
-            bool capListFinished;
-            QStringList availableCaps;
-            if (e->params().count() == 4) {
-                // Middle of multi-line reply, ignore the asterisk
-                capListFinished = false;
-                availableCaps = e->params().at(3).split(' ');
-            } else {
-                // Single line reply
-                capListFinished = true;
+
+    // All commands require at least 2 parameters
+    if (!checkParamCount(e, 2))
+        return;
+
+    CoreNetwork *coreNet = coreNetwork(e);
+    QString capCommand = e->params().at(1).trimmed().toUpper();
+    if (capCommand == "LS" || capCommand == "NEW") {
+        // Either we've gotten a list of capabilities, or new capabilities we may want
+        // Server: CAP * LS * :multi-prefix extended-join account-notify batch invite-notify tls
+        // Server: CAP * LS * :cap-notify server-time example.org/dummy-cap=dummyvalue example.org/second-dummy-cap
+        // Server: CAP * LS :userhost-in-names sasl=EXTERNAL,DH-AES,DH-BLOWFISH,ECDSA-NIST256P-CHALLENGE,PLAIN
+        bool capListFinished;
+        QStringList availableCaps;
+        if (e->params().count() == 4) {
+            // Middle of multi-line reply, ignore the asterisk
+            capListFinished = false;
+            availableCaps = e->params().at(3).split(' ');
+        } else {
+            // Single line reply
+            capListFinished = true;
+            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();
             }
-            // Store what capabilities are available
-            QStringList availableCapPair;
-            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());
-                }
+        }
+        // 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
+        QString availableCapName, availableCapValue;
+        for (int i = 0; i < availableCaps.count(); ++i) {
+            // Capability may include values, e.g. CAP * LS :multi-prefix sasl=EXTERNAL
+            // 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);
             }
+        }
+
+        // Begin capability requests when capability listing complete
+        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;
+        }
 
-            // Begin capability requests when capability listing complete
-            if (capListFinished)
-                coreNet->beginCapNegotiation();
-        } else if (capCommand == "ACK") {
-            // 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
-            QString acceptedCap = e->params().at(2).trimmed().toLower();
+        // Server: CAP * ACK :multi-prefix sasl
+        // Got the capabilities we want, handle as needed.
+        QStringList acceptedCaps;
+        acceptedCaps = e->params().at(2).split(' ');
 
+        // Store what capability was acknowledged
+        QString acceptedCap;
+
+        // Keep track of whether or not a capability requires further configuration.  Due to queuing
+        // logic in CoreNetwork::queueCap(), this shouldn't ever happen when more than one
+        // capability is requested, but it's better to handle edge cases or faulty servers.
+        bool capsRequireConfiguration = false;
+
+        for (int i = 0; i < acceptedCaps.count(); ++i) {
+            acceptedCap = acceptedCaps[i].trimmed().toLower();
             // Mark this cap as accepted
             coreNet->acknowledgeCap(acceptedCap);
-
-            if (!coreNet->capsRequiringConfiguration.contains(acceptedCap)) {
+            if (!capsRequireConfiguration &&
+                    coreNet->capsRequiringConfiguration.contains(acceptedCap)) {
+                capsRequireConfiguration = true;
                 // Some capabilities (e.g. SASL) require further messages to finish.  If so, do NOT
                 // send the next capability; it will be handled elsewhere in CoreNetwork.
-                // Otherwise, move on to the next capability
-                coreNet->sendNextCap();
-            }
-        } else if (capCommand == "NAK" || capCommand == "DEL") {
-            // Either something went wrong with this capability, or it is no longer supported
-            // > For CAP NAK
-            // Server: CAP * NAK :multi-prefix sasl
-            // > For CAP DEL
-            // Server: :irc.example.com CAP modernclient DEL :multi-prefix sasl
-            // CAP NAK and CAP DEL replies are always single-line
-
-            QStringList removedCaps;
-            removedCaps = e->params().at(2).split(' ');
-
-            // Store what capability was denied or removed
-            QString removedCap;
-            for (int i = 0; i < removedCaps.count(); ++i) {
-                removedCap = removedCaps[i].trimmed().toLower();
-                // Mark this cap as removed
-                coreNet->removeCap(removedCap);
+                // Otherwise, allow moving on to the next capability.
             }
+        }
 
+        if (!capsRequireConfiguration) {
+            // No additional configuration required, move on to the next capability
+            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") {
-                // Continue negotiation when capability listing complete only if this is the result
-                // of a denied cap, not a removed cap
+                // 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 the capabilities, or they are no longer supported
+        // > For CAP NAK
+        // Server: CAP * NAK :multi-prefix sasl
+        // > For CAP DEL
+        // Server: :irc.example.com CAP modernclient DEL :multi-prefix sasl
+        // CAP NAK and CAP DEL replies are always single-line
+
+        QStringList removedCaps;
+        removedCaps = e->params().at(2).split(' ');
+
+        // Store the capabilities that were denied or removed
+        QString removedCap;
+        for (int i = 0; i < removedCaps.count(); ++i) {
+            removedCap = removedCaps[i].trimmed().toLower();
+            // Mark this cap as removed.
+            // For CAP DEL, removes it from use.
+            // For CAP NAK when received before negotiation enabled these capabilities, removeCap()
+            // should do nothing.  This merely guards against non-spec servers sending an
+            // unsolicited CAP ACK then later removing that capability.
+            coreNet->removeCap(removedCap);
+        }
+
+        if (capCommand == "NAK") {
+            // Continue negotiation only if this is the result of denied caps, not removed caps
+            if (removedCaps.count() > 1) {
+                // We've received a CAP NAK reply to multiple capabilities at once.  Unfortunately,
+                // we don't know which capability failed and which ones are valid to re-request, so
+                // individually retry each capability from the failed bundle.
+                // See CoreNetwork::retryCapsIndividually() for more details.
+                coreNet->retryCapsIndividually();
+                // Still need to call sendNextCap() to carry on.
+            }
+            // Carry on with negotiation
+            coreNet->sendNextCap();
         }
     }
 }
@@ -242,15 +306,9 @@ void CoreSessionEventProcessor::processIrcEventAccount(IrcEvent *e)
 
     IrcUser *ircuser = e->network()->updateNickFromMask(e->prefix());
     if (ircuser) {
-        QString newAccount = e->params().at(0);
-        // WHOX uses '0' to indicate logged-out, account-notify uses '*'
-        if (newAccount != "*") {
-            // Account logged in, set account name
-            ircuser->setAccount(newAccount);
-        } else {
-            // Account logged out, set account name to logged-out
-            ircuser->setAccount("*");
-        }
+        // WHOX uses '0' to indicate logged-out, account-notify and extended-join uses '*'.
+        // As '*' is used internally to represent logged-out, no need to handle that differently.
+        ircuser->setAccount(e->params().at(0));
     } else {
         qDebug() << "Received account-notify data for unknown user" << e->prefix();
     }
@@ -259,13 +317,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 {
@@ -324,7 +386,9 @@ void CoreSessionEventProcessor::processIrcEventJoin(IrcEvent *e)
             // If logged in, :nick!user@host JOIN #channelname accountname :Real Name
             // If logged out, :nick!user@host JOIN #channelname * :Real Name
             // See:  http://ircv3.net/specs/extensions/extended-join-3.1.html
-            // FIXME Keep track of authed user account, requires adding support to ircuser.h/cpp
+            // WHOX uses '0' to indicate logged-out, account-notify and extended-join uses '*'.
+            // As '*' is used internally to represent logged-out, no need to handle that differently.
+            ircuser->setAccount(e->params()[1]);
             // Update the user's real name, too
             ircuser->setRealName(e->params()[2]);
         }
@@ -338,11 +402,16 @@ void CoreSessionEventProcessor::processIrcEventJoin(IrcEvent *e)
             break;
     }
 
-    // If using away-notify, check new users.  Works around buggy IRC servers
-    // forgetting to send :away messages for users who join channels when away.
-    if (net->capEnabled(IrcCap::AWAY_NOTIFY)) {
-        net->queueAutoWhoOneshot(ircuser->nick());
-    }
+    // With "away-notify" enabled, some IRC servers forget to send :away messages for users who join
+    // channels while away.  Unfortunately, working around this involves WHO'ng every single user as
+    // they join, which is not very efficient.  If at all possible, it's better to get the issue
+    // fixed in the IRC server instead.
+    //
+    // If pursuing a workaround instead, this is where you'd do it.  Check the version control
+    // history for the commit that added this comment to see how to implement it - there's some
+    // unexpected situations to watch out for!
+    //
+    // See https://ircv3.net/specs/extensions/away-notify-3.1.html
 
     if (!handledByNetsplit)
         ircuser->joinChannel(channel);
@@ -351,6 +420,8 @@ void CoreSessionEventProcessor::processIrcEventJoin(IrcEvent *e)
 
     if (net->isMe(ircuser)) {
         net->setChannelJoined(channel);
+        // Mark the message as Self
+        e->setFlag(EventManager::Self);
         // FIXME use event
         net->putRawLine(net->serverEncode("MODE " + channel)); // we want to know the modes of the channel we just joined, so we ask politely
     }
@@ -388,17 +459,17 @@ void CoreSessionEventProcessor::processIrcEventMode(IrcEvent *e)
         QString modes = e->params()[1];
         bool add = true;
         int paramOffset = 2;
-        for (int c = 0; c < modes.length(); c++) {
-            if (modes[c] == '+') {
+        for (auto mode : modes) {
+            if (mode == '+') {
                 add = true;
                 continue;
             }
-            if (modes[c] == '-') {
+            if (mode == '-') {
                 add = false;
                 continue;
             }
 
-            if (e->network()->prefixModes().contains(modes[c])) {
+            if (e->network()->prefixModes().contains(mode)) {
                 // user channel modes (op, voice, etc...)
                 if (paramOffset < e->params().count()) {
                     IrcUser *ircUser = e->network()->ircUser(e->params()[paramOffset]);
@@ -412,15 +483,15 @@ void CoreSessionEventProcessor::processIrcEventMode(IrcEvent *e)
                             foreach(Netsplit* n, _netsplits.value(e->network())) {
                                 handledByNetsplit = n->userAlreadyJoined(ircUser->hostmask(), channel->name());
                                 if (handledByNetsplit) {
-                                    n->addMode(ircUser->hostmask(), channel->name(), QString(modes[c]));
+                                    n->addMode(ircUser->hostmask(), channel->name(), QString(mode));
                                     break;
                                 }
                             }
                             if (!handledByNetsplit)
-                                channel->addUserMode(ircUser, QString(modes[c]));
+                                channel->addUserMode(ircUser, QString(mode));
                         }
                         else
-                            channel->removeUserMode(ircUser, QString(modes[c]));
+                            channel->removeUserMode(ircUser, QString(mode));
                     }
                 }
                 else {
@@ -431,7 +502,7 @@ void CoreSessionEventProcessor::processIrcEventMode(IrcEvent *e)
             else {
                 // regular channel modes
                 QString value;
-                Network::ChannelModeType modeType = e->network()->channelModeType(modes[c]);
+                Network::ChannelModeType modeType = e->network()->channelModeType(mode);
                 if (modeType == Network::A_CHANMODE || modeType == Network::B_CHANMODE || (modeType == Network::C_CHANMODE && add)) {
                     if (paramOffset < e->params().count()) {
                         value = e->params()[paramOffset];
@@ -443,9 +514,9 @@ void CoreSessionEventProcessor::processIrcEventMode(IrcEvent *e)
                 }
 
                 if (add)
-                    channel->addChannelMode(modes[c], value);
+                    channel->addChannelMode(mode, value);
                 else
-                    channel->removeChannelMode(modes[c], value);
+                    channel->removeChannelMode(mode, value);
             }
         }
     }
@@ -476,12 +547,33 @@ void CoreSessionEventProcessor::processIrcEventMode(IrcEvent *e)
             ircUser->removeUserModes(removeModes);
 
         if (e->network()->isMe(ircUser)) {
+            // Mark the message as Self
+            e->setFlag(EventManager::Self);
             coreNetwork(e)->updatePersistentModes(addModes, removeModes);
         }
     }
 }
 
 
+void CoreSessionEventProcessor::processIrcEventNick(IrcEvent *e)
+{
+    if (checkParamCount(e, 1)) {
+        IrcUser *ircuser = e->network()->updateNickFromMask(e->prefix());
+        if (!ircuser) {
+            qWarning() << Q_FUNC_INFO << "Unknown IrcUser!";
+            return;
+        }
+
+        if (e->network()->isMe(ircuser)) {
+            // Mark the message as Self
+            e->setFlag(EventManager::Self);
+        }
+
+        // Actual processing is handled in lateProcessIrcEventNick(), this just sets the event flag
+    }
+}
+
+
 void CoreSessionEventProcessor::lateProcessIrcEventNick(IrcEvent *e)
 {
     if (checkParamCount(e, 1)) {
@@ -502,6 +594,25 @@ void CoreSessionEventProcessor::lateProcessIrcEventNick(IrcEvent *e)
 }
 
 
+void CoreSessionEventProcessor::processIrcEventPart(IrcEvent *e)
+{
+    if (checkParamCount(e, 1)) {
+        IrcUser *ircuser = e->network()->updateNickFromMask(e->prefix());
+        if (!ircuser) {
+            qWarning() << Q_FUNC_INFO<< "Unknown IrcUser!";
+            return;
+        }
+
+        if (e->network()->isMe(ircuser)) {
+            // Mark the message as Self
+            e->setFlag(EventManager::Self);
+        }
+
+        // Actual processing is handled in lateProcessIrcEventNick(), this just sets the event flag
+    }
+}
+
+
 void CoreSessionEventProcessor::lateProcessIrcEventPart(IrcEvent *e)
 {
     if (checkParamCount(e, 1)) {
@@ -512,8 +623,9 @@ void CoreSessionEventProcessor::lateProcessIrcEventPart(IrcEvent *e)
         }
         QString channel = e->params().at(0);
         ircuser->partChannel(channel);
-        if (e->network()->isMe(ircuser))
+        if (e->network()->isMe(ircuser)) {
             qobject_cast<CoreNetwork *>(e->network())->setChannelParted(channel);
+        }
     }
 }
 
@@ -529,14 +641,71 @@ void CoreSessionEventProcessor::processIrcEventPing(IrcEvent *e)
 
 void CoreSessionEventProcessor::processIrcEventPong(IrcEvent *e)
 {
-    // the server is supposed to send back what we passed as param. and we send a timestamp
-    // but using quote and whatnought one can send arbitrary pings, so we have to do some sanity checks
-    if (checkParamCount(e, 2)) {
-        QString timestamp = e->params().at(1);
-        QTime sendTime = QTime::fromString(timestamp, "hh:mm:ss.zzz");
-        if (sendTime.isValid())
-            e->network()->setLatency(sendTime.msecsTo(QTime::currentTime()) / 2);
+    // Ensure we get at least one parameter
+    if (!checkParamCount(e, 1))
+        return;
+
+    // Some IRC servers respond with only one parameter, others respond with two, with the latter
+    // being the text sent.  Handle both situations.
+    QString timestamp;
+    if (e->params().count() < 2) {
+        // Only one parameter received
+        // :localhost PONG 02:43:49.565
+        timestamp = e->params().at(0);
+    } else {
+        // Two parameters received, pick the second
+        // :localhost PONG localhost :02:43:49.565
+        timestamp = e->params().at(1);
     }
+
+    // The server is supposed to send back what we passed as parameter, and we send a timestamp.
+    // However, using quote and whatnot, one can send arbitrary pings, and IRC servers may decide to
+    // ignore our requests entirely and send whatever they want, so we have to do some sanity
+    // checks.
+    //
+    // Attempt to parse the timestamp
+    QTime sendTime = QTime::fromString(timestamp, "hh:mm:ss.zzz");
+    if (sendTime.isValid()) {
+        // Mark IRC server as sending valid ping replies
+        if (!coreNetwork(e)->isPongTimestampValid()) {
+            coreNetwork(e)->setPongTimestampValid(true);
+            // Add a message the first time it happens
+            qDebug().nospace() << "Received PONG with valid timestamp, marking pong replies on "
+                                  "network "
+                               << "\"" << qPrintable(e->network()->networkName()) << "\" (ID: "
+                               << qPrintable(QString::number(e->network()->networkId().toInt()))
+                               << ") as usable for latency measurement";
+        }
+        // Remove pending flag
+        coreNetwork(e)->resetPongReplyPending();
+
+        // Don't show this in the UI
+        e->setFlag(EventManager::Silent);
+        // TODO:  To allow for a user-sent /ping (without arguments, so default timestamp is used),
+        // this could track how many automated PINGs have been sent by the core and subtract one
+        // each time, only marking the PING as silent if there's pending automated pong replies.
+        // However, that's a behavior change which warrants further testing.  For now, take the
+        // simpler, previous approach that errs on the side of silencing too much.
+
+        // Calculate latency from time difference, divided by 2 to account for round-trip time
+        e->network()->setLatency(sendTime.msecsTo(QTime::currentTime()) / 2);
+    } else if (coreNetwork(e)->isPongReplyPending() && !coreNetwork(e)->isPongTimestampValid()) {
+        // There's an auto-PING reply pending and we've not yet received a PONG reply with a valid
+        // timestamp.  It's possible this server will never respond with a valid timestamp, and thus
+        // any automated PINGs will result in unwanted spamming of the server buffer.
+
+        // Don't show this in the UI
+        e->setFlag(EventManager::Silent);
+        // Remove pending flag
+        coreNetwork(e)->resetPongReplyPending();
+
+        // Log a message
+        qDebug().nospace() << "Received PONG with invalid timestamp from network "
+                           << "\"" << qPrintable(e->network()->networkName()) << "\" (ID: "
+                           << qPrintable(QString::number(e->network()->networkId().toInt()))
+                           << "), silencing, parameters are " << e->params();
+    }
+    // else: We're not expecting a PONG reply and timestamp is not valid, assume it's from the user
 }
 
 
@@ -546,6 +715,11 @@ void CoreSessionEventProcessor::processIrcEventQuit(IrcEvent *e)
     if (!ircuser)
         return;
 
+    if (e->network()->isMe(ircuser)) {
+        // Mark the message as Self
+        e->setFlag(EventManager::Self);
+    }
+
     QString msg;
     if (e->params().count() > 0)
         msg = e->params()[0];
@@ -591,13 +765,34 @@ void CoreSessionEventProcessor::lateProcessIrcEventQuit(IrcEvent *e)
 void CoreSessionEventProcessor::processIrcEventTopic(IrcEvent *e)
 {
     if (checkParamCount(e, 2)) {
-        e->network()->updateNickFromMask(e->prefix());
+        IrcUser *ircuser = e->network()->updateNickFromMask(e->prefix());
+
+        if (e->network()->isMe(ircuser)) {
+            // Mark the message as Self
+            e->setFlag(EventManager::Self);
+        }
+
         IrcChannel *channel = e->network()->ircChannel(e->params().at(0));
         if (channel)
             channel->setTopic(e->params().at(1));
     }
 }
 
+/* ERROR - "ERROR :reason"
+Example:  ERROR :Closing Link: nickname[xxx.xxx.xxx.xxx] (Large base64 image paste.)
+See https://tools.ietf.org/html/rfc2812#section-3.7.4 */
+void CoreSessionEventProcessor::processIrcEventError(IrcEvent *e)
+{
+    if (!checkParamCount(e, 1))
+        return;
+
+    if (coreNetwork(e)->disconnectExpected()) {
+        // During QUIT, the server should send an error (often, but not always, "Closing Link"). As
+        // we're expecting it, don't show this to the user.
+        e->setFlag(EventManager::Silent);
+    }
+}
+
 
 #ifdef HAVE_QCA2
 void CoreSessionEventProcessor::processKeyEvent(KeyEvent *e)
@@ -606,7 +801,7 @@ void CoreSessionEventProcessor::processKeyEvent(KeyEvent *e)
         emit newEvent(new MessageEvent(Message::Error, e->network(), tr("Unable to perform key exchange, missing qca-ossl plugin."), e->prefix(), e->target(), Message::None, e->timestamp()));
         return;
     }
-    CoreNetwork *net = qobject_cast<CoreNetwork*>(e->network());
+    auto *net = qobject_cast<CoreNetwork*>(e->network());
     Cipher *c = net->cipher(e->target());
     if (!c) // happens when there is no CoreIrcChannel for the target (i.e. never?)
         return;
@@ -713,7 +908,9 @@ void CoreSessionEventProcessor::processIrcEvent301(IrcEvent *e)
     if (ircuser) {
         ircuser->setAway(true);
         ircuser->setAwayMessage(e->params().at(1));
-        //ircuser->setLastAwayMessage(now);
+        // lastAwayMessageTime is set in EventStringifier::processIrcEvent301(), no need to set it
+        // here too
+        //ircuser->setLastAwayMessageTime(now);
     }
 }
 
@@ -826,8 +1023,18 @@ void CoreSessionEventProcessor::processIrcEvent317(IrcEvent *e)
 
     int idleSecs = e->params()[1].toInt();
     if (e->params().count() > 3) { // if we have more then 3 params we have the above mentioned "real life" situation
-        int logintime = e->params()[2].toInt();
-        loginTime = QDateTime::fromTime_t(logintime);
+        // Allow for 64-bit time
+        qint64 logintime = e->params()[2].toLongLong();
+        // Time in IRC protocol is defined as seconds.  Convert from seconds instead.
+        // See https://doc.qt.io/qt-5/qdatetime.html#fromSecsSinceEpoch
+#if QT_VERSION >= 0x050800
+        loginTime = QDateTime::fromSecsSinceEpoch(logintime);
+#else
+        // fromSecsSinceEpoch() was added in Qt 5.8.  Manually downconvert to seconds for
+        // now.
+        // See https://doc.qt.io/qt-5/qdatetime.html#fromMSecsSinceEpoch
+        loginTime = QDateTime::fromMSecsSinceEpoch((qint64)(logintime * 1000));
+#endif
     }
 
     IrcUser *ircuser = e->network()->ircUser(e->params()[0]);
@@ -852,10 +1059,13 @@ void CoreSessionEventProcessor::processIrcEvent322(IrcEvent *e)
     switch (e->params().count()) {
     case 3:
         topic = e->params()[2];
+        // fallthrough
     case 2:
         userCount = e->params()[1].toUInt();
+        // fallthrough
     case 1:
         channelName = e->params()[0];
+        // fallthrough
     default:
         break;
     }
@@ -882,10 +1092,12 @@ void CoreSessionEventProcessor::processIrcEvent324(IrcEvent *e)
 }
 
 
-/*  RPL_WHOISACCOUNT: "<nick> <account> :is authed as */
+/*  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));
@@ -929,14 +1141,17 @@ void CoreSessionEventProcessor::processIrcEvent352(IrcEvent *e)
     QString channel = e->params()[0];
     IrcUser *ircuser = e->network()->ircUser(e->params()[4]);
     if (ircuser) {
+        // Only process the WHO information if an IRC user exists.  Don't create an IRC user here;
+        // there's no way to track when the user quits, which would leave a phantom IrcUser lying
+        // around.
+        // NOTE:  Whenever MONITOR support is introduced, the IrcUser will be created by an
+        // RPL_MONONLINE numeric before any WHO commands are run.
         processWhoInformation(e->network(), channel, ircuser, e->params()[3], e->params()[1],
                 e->params()[2], e->params()[5], e->params().last().section(" ", 1));
     }
 
     // Check if channel name has a who in progress.
-    // If not, then check if user nick exists and has a who in progress.
-    if (coreNetwork(e)->isAutoWhoInProgress(channel) ||
-        (ircuser && coreNetwork(e)->isAutoWhoInProgress(ircuser->nick()))) {
+    if (coreNetwork(e)->isAutoWhoInProgress(channel)) {
         e->setFlag(EventManager::Silent);
     }
 }
@@ -1025,12 +1240,17 @@ void CoreSessionEventProcessor::processIrcEvent354(IrcEvent *e)
     QString channel = e->params()[1];
     IrcUser *ircuser = e->network()->ircUser(e->params()[5]);
     if (ircuser) {
+        // Only process the WHO information if an IRC user exists.  Don't create an IRC user here;
+        // there's no way to track when the user quits, which would leave a phantom IrcUser lying
+        // around.
+        // NOTE:  Whenever MONITOR support is introduced, the IrcUser will be created by an
+        // RPL_MONONLINE numeric before any WHO commands are run.
         processWhoInformation(e->network(), channel, ircuser, e->params()[4], e->params()[2],
                 e->params()[3], e->params()[6], e->params().last());
         // Don't use .section(" ", 1) with WHOX replies, for there's no hopcount to trim out
 
         // As part of IRCv3 account-notify, check account name
-        // WHOX uses '0' to indicate logged-out, account-notify uses '*'
+        // WHOX uses '0' to indicate logged-out, account-notify and extended-join uses '*'.
         QString newAccount = e->params()[7];
         if (newAccount != "0") {
             // Account logged in, set account name
@@ -1042,9 +1262,7 @@ void CoreSessionEventProcessor::processIrcEvent354(IrcEvent *e)
     }
 
     // Check if channel name has a who in progress.
-    // If not, then check if user nick exists and has a who in progress.
-    if (coreNetwork(e)->isAutoWhoInProgress(channel) ||
-        (ircuser && coreNetwork(e)->isAutoWhoInProgress(ircuser->nick()))) {
+    if (coreNetwork(e)->isAutoWhoInProgress(channel)) {
         e->setFlag(EventManager::Silent);
     }
 }
@@ -1104,7 +1322,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];
@@ -1258,7 +1478,7 @@ void CoreSessionEventProcessor::handleEarlyNetsplitJoin(Network *net, const QStr
 
 void CoreSessionEventProcessor::handleNetsplitFinished()
 {
-    Netsplit *n = qobject_cast<Netsplit *>(sender());
+    auto *n = qobject_cast<Netsplit *>(sender());
     Q_ASSERT(n);
     QHash<QString, Netsplit *> splithash  = _netsplits.take(n->network());
     splithash.remove(splithash.key(n));
@@ -1389,12 +1609,22 @@ void CoreSessionEventProcessor::handleCtcpPing(CtcpEvent *e)
 
 void CoreSessionEventProcessor::handleCtcpTime(CtcpEvent *e)
 {
-    e->setReply(QDateTime::currentDateTime().toString());
+    // Use the ISO standard to avoid locale-specific translated names
+    // Include timezone offset data to show which timezone a user's in, otherwise we're providing
+    // NTP-over-IRC with terrible accuracy.
+    e->setReply(formatDateTimeToOffsetISO(QDateTime::currentDateTime()));
 }
 
 
 void CoreSessionEventProcessor::handleCtcpVersion(CtcpEvent *e)
 {
-    e->setReply(QString("Quassel IRC %1 (built on %2) -- http://www.quassel-irc.org")
-        .arg(Quassel::buildInfo().plainVersionString).arg(Quassel::buildInfo().commitDate));
+    // Deliberately do not translate project name
+    // Use the ISO standard to avoid locale-specific translated names
+    // Use UTC time to provide a consistent string regardless of timezone
+    // (Statistics tracking tools usually only group client versions by exact string matching)
+    e->setReply(QString("Quassel IRC %1 (version date %2) -- https://www.quassel-irc.org")
+                .arg(Quassel::buildInfo().plainVersionString)
+                .arg(Quassel::buildInfo().commitDate.isEmpty() ?
+                      "unknown" : tryFormatUnixEpoch(Quassel::buildInfo().commitDate,
+                                                     Qt::DateFormat::ISODate, true)));
 }