From 425364f5f68a37582ddfa0494f4305f98f761e23 Mon Sep 17 00:00:00 2001
From: Shane Synan <digitalcircuit36939@gmail.com>
Date: Sun, 17 Jun 2018 22:50:01 -0500
Subject: [PATCH] core: Handle single-parameter PONG replies

In processIrcEventPong, allow for single-parameter PONG replies.
Some IRC servers reply with server name then quote, others reply with
just the quote.

Log a debug message for invalid timestamps.  It's not a known error
as the user can request arbitrary PING replies, but it should be
logged to simplify tracking down IRC servers that mangle or outright
ignore the PING parameters.

(Yes, some IRC servers do this.)

Handle single-parameter PONG replies in EventStringifier, too.

Clarify and reformat comments.
---
 src/core/coresessioneventprocessor.cpp | 37 +++++++++++++++++++++-----
 src/core/eventstringifier.cpp          | 23 ++++++++++++++--
 2 files changed, 51 insertions(+), 9 deletions(-)

diff --git a/src/core/coresessioneventprocessor.cpp b/src/core/coresessioneventprocessor.cpp
index 0e9bb019..c4c01e5a 100644
--- a/src/core/coresessioneventprocessor.cpp
+++ b/src/core/coresessioneventprocessor.cpp
@@ -636,13 +636,36 @@ 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()) {
+        // Calculate latency from time difference, divided by 2 to account for round-trip time
+        e->network()->setLatency(sendTime.msecsTo(QTime::currentTime()) / 2);
+    } else {
+        // Just in case it's a wonky server, log a debug message to make this easier to track down
+        qDebug() << "Received valid PONG with invalid timestamp, parameters are" << e->params();
     }
 }
 
diff --git a/src/core/eventstringifier.cpp b/src/core/eventstringifier.cpp
index 15e246f3..b4ba3ad5 100644
--- a/src/core/eventstringifier.cpp
+++ b/src/core/eventstringifier.cpp
@@ -365,10 +365,29 @@ void EventStringifier::processIrcEventPart(IrcEvent *e)
 
 void EventStringifier::processIrcEventPong(IrcEvent *e)
 {
-    QString timestamp = e->params().at(1);
+    // 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);
+    }
+
+    // Attempt to parse the timestamp
     QTime sendTime = QTime::fromString(timestamp, "hh:mm:ss.zzz");
-    if (!sendTime.isValid())
+    if (!sendTime.isValid()) {
+        // No valid timestamp found, this is most likely a user-specified PING message.
+        //
+        // Or the IRC server is returning whatever it feels like to PING messages, in which case..
+        // sorry.  Increase the ping timeout delay in Quassel to as high as possible, and go
+        // encourage your IRC server developer to fix their stuff.
         displayMsg(e, Message::Server, "PONG " + e->params().join(" "), e->prefix());
+    }
 }
 
 
-- 
2.20.1