Created attachment 48046 [details] Fix the issue Version: Git (using KDE 4.4.3) OS: Linux Currently a PING request is scheduled for each PONG received. When receiving some unrequested PONG, too many PING are sent causing server flood. This occur when using an irc proxy with multiple clients connected, number of PING requests grows exponentially. A simple check can be added to konversation code to avoid scheduling multiple PING by ignoring PONG when no PING were sent. See attached patch. Reproducible: Always Steps to Reproduce: Connect two konversation clients to a znc irc proxy on freenode. and you end up being disconnected for flood in a few minutes. Actual Results: You end up being disconnected for flood in a few minutes.
I think it would make more sense to turn the singleShot in pongReceived() (and connectionEstablished()) into a QTimer class member and respond to PONG with PING only if QTimer::isActive() doesn't return true. This should address your problem by preventing a PING from being sent more than once per 60 seconds and is a bit cleaner since it doesn't need that bool for state tracking. Interested in whipping that up?
Ok, thanks for the comment, I'll submit a new patch soon.
Created attachment 48296 [details] Added QTimer as requested
commit 86dc0790b28e3793b9ed6f738ffda07fe8da3a23 Author: Eike Hein <hein@kde.org> Date: Thu Jun 24 21:00:49 2010 +0200 Don't send PING in response to PONG if a PING is already scheduled. Avoids getting kicked off a server for flooding with multiple clients connected to a bouncer that forwards PONGs to all of them. Patch by Alexandre Becoulet, many thanks! BUG:241858 diff --git a/ChangeLog b/ChangeLog index 5d23067..c28d3b2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -15,6 +15,10 @@ Changes since 1.3: * Fixed a bug that could cause outdated status information for nicks to be displayed in channel nickname lists after a reconnect. * Efficiency improvements for channel join. +* Don't send PING in response to PONG if another PING is already scheduled + to be sent in the future. This avoids getting kicked off a server for + flooding when multiple clients are connected to a bouncer that forwards + PONGs to all of them. Changes from 1.3-beta1 to 1.3: diff --git a/src/commit.h b/src/commit.h index 778619a..7c73225 100644 --- a/src/commit.h +++ b/src/commit.h @@ -1,4 +1,4 @@ // This COMMIT number is added to version string to be used as "patch level" #ifndef COMMIT -#define COMMIT 4052 +#define COMMIT 4053 #endif diff --git a/src/irc/server.cpp b/src/irc/server.cpp index 2a8255c..bbb4f84 100644 --- a/src/irc/server.cpp +++ b/src/irc/server.cpp @@ -240,6 +240,7 @@ void Server::initTimers() m_notifyTimer.setObjectName("notify_timer"); m_notifyTimer.setSingleShot(true); m_incomingTimer.setObjectName("incoming_timer"); + m_pingSendTimer.setSingleShot(true); } void Server::connectSignals() @@ -248,6 +249,7 @@ void Server::connectSignals() connect(&m_incomingTimer, SIGNAL(timeout()), this, SLOT(processIncomingData())); connect(&m_notifyTimer, SIGNAL(timeout()), this, SLOT(notifyTimeout())); connect(&m_pingResponseTimer, SIGNAL(timeout()), this, SLOT(updateLongPongLag())); + connect(&m_pingSendTimer, SIGNAL(timeout()), this, SLOT(sendPing())); // OutputFilter connect(getOutputFilter(), SIGNAL(requestDccSend()), this,SLOT(requestDccSend()), Qt::QueuedConnection); @@ -732,7 +734,7 @@ void Server::connectionEstablished(const QString& ownHost) requestUserhost(getNickname()); // Start the PINGPONG match - QTimer::singleShot(1000 /*1 sec*/, this, SLOT(sendPing())); + m_pingSendTimer.start(1000 /*1 sec*/); // Recreate away state if we were set away prior to a reconnect. if (m_away) @@ -3792,14 +3794,18 @@ void Server::sendPing() void Server::pongReceived() { + // ignore unrequested PONGs + if (m_pingSendTimer.isActive()) + return; + m_currentLag = m_lagTime.elapsed(); m_inputFilter.setLagMeasuring(false); m_pingResponseTimer.stop(); emit serverLag(this, m_currentLag); - // Send another PING in 60 seconds - QTimer::singleShot(60000 /*60 sec*/, this, SLOT(sendPing())); + // Send another PING in 60 seconds + m_pingSendTimer.start(60000 /*60 sec*/); } void Server::updateLongPongLag() diff --git a/src/irc/server.h b/src/irc/server.h index d2807a4..049970c 100644 --- a/src/irc/server.h +++ b/src/irc/server.h @@ -801,6 +801,8 @@ class Server : public QObject QTime m_lagTime; /// Updates the gui when the lag gets too high QTimer m_pingResponseTimer; + /// Wait before sending the next PING + QTimer m_pingSendTimer; /// Previous ISON reply of the server, needed for comparison with the next reply QStringList m_prevISONList;