Fix race condition where there was a chance to allow all ips between reparsings of the ip filter.

src/base/bittorrent/private/filterparserthread.cpp

@@ -33,17 +33,13 @@
 #include <QDataStream>
 #include <QStringList>
 
-#include <libtorrent/session.hpp>
-#include <libtorrent/ip_filter.hpp>
-
 #include "base/logger.h"
 #include "filterparserthread.h"
 
 namespace libt = libtorrent;
 
-FilterParserThread::FilterParserThread(libt::session *s, QObject *parent)
+FilterParserThread::FilterParserThread(QObject *parent)
     : QThread(parent)
-    , m_session(s)
     , m_abort(false)
 {
 }
@@ -55,10 +51,10 @@ FilterParserThread::~FilterParserThread()
 }
 
 // Parser for eMule ip filter in DAT format
-int FilterParserThread::parseDATFilterFile(QString filePath, libt::ip_filter &filter)
+int FilterParserThread::parseDATFilterFile()
 {
     int ruleCount = 0;
-    QFile file(filePath);
+    QFile file(m_filePath);
     if (!file.exists()) return ruleCount;
 
     if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) {
@@ -136,7 +132,7 @@ int FilterParserThread::parseDATFilterFile(QString filePath, libt::ip_filter &fi
 
         // Now Add to the filter
         try {
-            filter.add_rule(startAddr, endAddr, libt::ip_filter::blocked);
+            m_filter.add_rule(startAddr, endAddr, libt::ip_filter::blocked);
             ++ruleCount;
         }
         catch(std::exception &) {
@@ -149,10 +145,10 @@ int FilterParserThread::parseDATFilterFile(QString filePath, libt::ip_filter &fi
 }
 
 // Parser for PeerGuardian ip filter in p2p format
-int FilterParserThread::parseP2PFilterFile(QString filePath, libt::ip_filter &filter)
+int FilterParserThread::parseP2PFilterFile()
 {
     int ruleCount = 0;
-    QFile file(filePath);
+    QFile file(m_filePath);
     if (!file.exists()) return ruleCount;
 
     if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) {
@@ -219,7 +215,7 @@ int FilterParserThread::parseP2PFilterFile(QString filePath, libt::ip_filter &fi
         }
 
         try {
-            filter.add_rule(startAddr, endAddr, libt::ip_filter::blocked);
+            m_filter.add_rule(startAddr, endAddr, libt::ip_filter::blocked);
             ++ruleCount;
         }
         catch(std::exception &) {
@@ -257,10 +253,10 @@ int FilterParserThread::getlineInStream(QDataStream &stream, std::string &name,
 }
 
 // Parser for PeerGuardian ip filter in p2p format
-int FilterParserThread::parseP2BFilterFile(QString filePath, libt::ip_filter &filter)
+int FilterParserThread::parseP2BFilterFile()
 {
     int ruleCount = 0;
-    QFile file(filePath);
+    QFile file(m_filePath);
     if (!file.exists()) return ruleCount;
 
     if (!file.open(QIODevice::ReadOnly)) {
@@ -298,7 +294,7 @@ int FilterParserThread::parseP2BFilterFile(QString filePath, libt::ip_filter &fi
             libt::address_v4 last(ntohl(end));
             // Apply to bittorrent session
             try {
-                filter.add_rule(first, last, libt::ip_filter::blocked);
+                m_filter.add_rule(first, last, libt::ip_filter::blocked);
                 ++ruleCount;
             }
             catch(std::exception &) {}
@@ -348,7 +344,7 @@ int FilterParserThread::parseP2BFilterFile(QString filePath, libt::ip_filter &fi
             libt::address_v4 last(ntohl(end));
             // Apply to bittorrent session
             try {
-                filter.add_rule(first, last, libt::ip_filter::blocked);
+                m_filter.add_rule(first, last, libt::ip_filter::blocked);
                 ++ruleCount;
             }
             catch(std::exception &) {}
@@ -369,7 +365,7 @@ int FilterParserThread::parseP2BFilterFile(QString filePath, libt::ip_filter &fi
 //  * eMule IP list (DAT): http://wiki.phoenixlabs.org/wiki/DAT_Format
 //  * PeerGuardian Text (P2P): http://wiki.phoenixlabs.org/wiki/P2P_Format
 //  * PeerGuardian Binary (P2B): http://wiki.phoenixlabs.org/wiki/P2B_Format
-void FilterParserThread::processFilterFile(QString filePath)
+void FilterParserThread::processFilterFile(const QString &filePath)
 {
     if (isRunning()) {
         // Already parsing a filter, m_abort first
@@ -379,10 +375,16 @@ void FilterParserThread::processFilterFile(QString filePath)
 
     m_abort = false;
     m_filePath = filePath;
+    m_filter = libt::ip_filter();
     // Run it
     start();
 }
 
+libt::ip_filter FilterParserThread::IPfilter()
+{
+    return m_filter;
+}
+
 QString FilterParserThread::cleanupIPAddress(QString _ip)
 {
     _ip = _ip.trimmed();
@@ -417,25 +419,23 @@ QString FilterParserThread::cleanupIPAddress(QString _ip)
 void FilterParserThread::run()
 {
     qDebug("Processing filter file");
-    libt::ip_filter filter = m_session->get_ip_filter();
     int ruleCount = 0;
     if (m_filePath.endsWith(".p2p", Qt::CaseInsensitive)) {
         // PeerGuardian p2p file
-        ruleCount = parseP2PFilterFile(m_filePath, filter);
+        ruleCount = parseP2PFilterFile();
     }
     else if (m_filePath.endsWith(".p2b", Qt::CaseInsensitive)) {
         // PeerGuardian p2b file
-        ruleCount = parseP2BFilterFile(m_filePath, filter);
+        ruleCount = parseP2BFilterFile();
     }
     else if (m_filePath.endsWith(".dat", Qt::CaseInsensitive)) {
         // eMule DAT format
-        ruleCount = parseDATFilterFile(m_filePath, filter);
+        ruleCount = parseDATFilterFile();
     }
 
     if (m_abort) return;
 
     try {
-        m_session->set_ip_filter(filter);
         emit IPFilterParsed(ruleCount);
     }
     catch(std::exception &) {

src/base/bittorrent/private/filterparserthread.h

@@ -33,23 +33,20 @@
 
 #include <QThread>
 
+#include <libtorrent/ip_filter.hpp>
+
 class QDataStream;
 class QStringList;
 
-namespace libtorrent
-{
-    class session;
-    struct ip_filter;
-}
-
 class FilterParserThread : public QThread
 {
     Q_OBJECT
 
 public:
-    FilterParserThread(libtorrent::session *s, QObject *parent = 0);
+    FilterParserThread(QObject *parent = 0);
     ~FilterParserThread();
-    void processFilterFile(QString filePath);
+    void processFilterFile(const QString &filePath);
+    libtorrent::ip_filter IPfilter();
 
 signals:
     void IPFilterParsed(int ruleCount);
@@ -60,14 +57,14 @@ protected:
     void run();
 
 private:
-    int parseDATFilterFile(QString filePath, libtorrent::ip_filter &filter);
-    int parseP2PFilterFile(QString filePath, libtorrent::ip_filter &filter);
+    int parseDATFilterFile();
+    int parseP2PFilterFile();
     int getlineInStream(QDataStream &stream, std::string &name, char delim);
-    int parseP2BFilterFile(QString filePath, libtorrent::ip_filter &filter);
+    int parseP2BFilterFile();
 
-    libtorrent::session *m_session;
     bool m_abort;
     QString m_filePath;
+    libtorrent::ip_filter m_filter;
 };
 
 #endif // BITTORRENT_FILTERPARSERTHREAD_H

src/base/bittorrent/session.cpp

@@ -385,10 +385,16 @@ Session::Session(QObject *parent)
                        .arg(encryption() == 0 ? tr("ON") : encryption() == 1 ? tr("FORCED") : tr("OFF"))
                        , Log::INFO);
 
-    if (isIPFilteringEnabled())
+    if (isIPFilteringEnabled()) {
+        // Manually banned IPs are handled in that function too(in the slots)
         enableIPFilter();
-    // Add the banned IPs
-    processBannedIPs();
+    }
+    else {
+        // Add the banned IPs
+        libt::ip_filter filter;
+        processBannedIPs(filter);
+        m_nativeSession->set_ip_filter(filter);
+    }
 
     m_categories = map_cast(m_storedCategories);
     if (isSubcategoriesEnabled()) {
@@ -891,10 +897,9 @@ void Session::configure()
     qDebug("Session configured");
 }
 
-void Session::processBannedIPs()
+void Session::processBannedIPs(libt::ip_filter &filter)
 {
     // First, import current filter
-    libt::ip_filter filter = m_nativeSession->get_ip_filter();
     foreach (const QString &ip, m_bannedIPs.value()) {
         boost::system::error_code ec;
         libt::address addr = libt::address::from_string(ip.toLatin1().constData(), ec);
@@ -902,8 +907,6 @@ void Session::processBannedIPs()
         if (!ec)
             filter.add_rule(addr, addr, libt::ip_filter::blocked);
     }
-
-    m_nativeSession->set_ip_filter(filter);
 }
 
 #if LIBTORRENT_VERSION_NUM >= 10100
@@ -3028,13 +3031,12 @@ void Session::configureDeferred()
 void Session::enableIPFilter()
 {
     qDebug("Enabling IPFilter");
-    // 1. Clear existing ban list
-    // 2. Add manual ban list
-    // 3. Add 3rd party ban list
-    m_nativeSession->set_ip_filter(libt::ip_filter());
-    processBannedIPs();
+    // 1. Parse the IP filter
+    // 2. In the slot add the manually banned IPs to the provided libtorrent::ip_filter
+    // 3. Set the ip_filter in one go so there isn't a time window where there isn't an ip_filter
+    //    set between clearing the old one and setting the new one.
     if (!m_filterParser) {
-        m_filterParser = new FilterParserThread(m_nativeSession, this);
+        m_filterParser = new FilterParserThread(this);
         connect(m_filterParser.data(), SIGNAL(IPFilterParsed(int)), SLOT(handleIPFilterParsed(int)));
         connect(m_filterParser.data(), SIGNAL(IPFilterError()), SLOT(handleIPFilterError()));
     }
@@ -3045,7 +3047,6 @@ void Session::enableIPFilter()
 void Session::disableIPFilter()
 {
     qDebug("Disabling IPFilter");
-    m_nativeSession->set_ip_filter(libt::ip_filter());
     if (m_filterParser) {
         disconnect(m_filterParser.data(), 0, this, 0);
         delete m_filterParser;
@@ -3054,7 +3055,9 @@ void Session::disableIPFilter()
     // Add the banned IPs after the IPFilter disabling
     // which creates an empty filter and overrides all previously
     // applied bans.
-    processBannedIPs();
+    libt::ip_filter filter;
+    processBannedIPs(filter);
+    m_nativeSession->set_ip_filter(filter);
 }
 
 void Session::recursiveTorrentDownload(const InfoHash &hash)
@@ -3191,12 +3194,21 @@ void Session::refresh()
 
 void Session::handleIPFilterParsed(int ruleCount)
 {
+    if (!m_filterParser) {
+        libt::ip_filter filter = m_filterParser->IPfilter();
+        processBannedIPs(filter);
+        m_nativeSession->set_ip_filter(filter);
+    }
     Logger::instance()->addMessage(tr("Successfully parsed the provided IP filter: %1 rules were applied.", "%1 is a number").arg(ruleCount));
     emit IPFilterParsed(false, ruleCount);
 }
 
 void Session::handleIPFilterError()
 {
+    libt::ip_filter filter;
+    processBannedIPs(filter);
+    m_nativeSession->set_ip_filter(filter);
+
     Logger::instance()->addMessage(tr("Error: Failed to parse the provided IP filter."), Log::CRITICAL);
     emit IPFilterParsed(true, 0);
 }

src/base/bittorrent/session.h

@@ -57,6 +57,7 @@ namespace libtorrent
     struct torrent_handle;
     class entry;
     struct add_torrent_params;
+    struct ip_filter;
     struct pe_settings;
 #if LIBTORRENT_VERSION_NUM < 10100
     struct session_settings;
@@ -449,7 +450,7 @@ namespace BitTorrent
         void adjustLimits(libtorrent::settings_pack &settingsPack);
 #endif
         void adjustLimits();
-        void processBannedIPs();
+        void processBannedIPs(libtorrent::ip_filter &filter);
         const QStringList getListeningIPs();
         void configureListeningInterface();
         void changeSpeedLimitMode_impl(bool alternative);