Enforce no duplicate slots from incoming connections

src/ripple/peerfinder/impl/Logic.h

@@ -273,7 +273,7 @@ class Logic
 
         std::lock_guard _(lock_);
 
-        // Check for duplicate connection
+        // Check for connection limit per address
         if (is_public(remote_endpoint))
         {
             auto const count =
@@ -287,6 +287,15 @@ class Logic
             }
         }
 
+        // Check for duplicate connection
+        if (slots_.find(remote_endpoint) != slots_.end())
+        {
+            JLOG(m_journal.debug())
+                << beast::leftw(18) << "Logic dropping " << remote_endpoint
+                << " as duplicate incoming";
+            return SlotImp::ptr();
+        }
+
         // Create the slot
         SlotImp::ptr const slot(std::make_shared<SlotImp>(
             local_endpoint,

src/test/peerfinder/PeerFinder_test.cpp

@@ -20,6 +20,7 @@
 #include <ripple/basics/Slice.h>
 #include <ripple/basics/chrono.h>
 #include <ripple/beast/unit_test.h>
+#include <ripple/beast/unit_test/suite.hpp>
 #include <ripple/core/Config.h>
 #include <ripple/peerfinder/impl/Logic.h>
 #include <ripple/protocol/PublicKey.h>
@@ -157,6 +158,104 @@ class PeerFinder_test : public beast::unit_test::suite
         BEAST_EXPECT(n <= (seconds + 59) / 60);
     }
 
+    void
+    test_duplicateOutIn()
+    {
+        auto const seconds = 10000;
+        testcase("duplicate out/in");
+        TestStore store;
+        TestChecker checker;
+        TestStopwatch clock;
+        Logic<TestChecker> logic(clock, store, checker, journal_);
+        logic.addFixedPeer(
+            "test", beast::IP::Endpoint::from_string("65.0.0.1:5"));
+        {
+            Config c;
+            c.autoConnect = false;
+            c.listeningPort = 1024;
+            c.ipLimit = 2;
+            logic.config(c);
+        }
+
+        std::size_t n = 0;
+        for (std::size_t i = 0; i < seconds; ++i)
+        {
+            auto const list = logic.autoconnect();
+            if (!list.empty())
+            {
+                BEAST_EXPECT(list.size() == 1);
+                auto const remote = list.front();
+                auto const slot1 = logic.new_outbound_slot(remote);
+                if (BEAST_EXPECT(slot1 != nullptr))
+                {
+                    BEAST_EXPECT(
+                        logic.connectedAddresses_.count(remote.address()) == 1);
+                    auto const local =
+                        beast::IP::Endpoint::from_string("65.0.0.2:1024");
+                    auto const slot2 = logic.new_inbound_slot(local, remote);
+                    BEAST_EXPECT(
+                        logic.connectedAddresses_.count(remote.address()) == 1);
+                    if (!BEAST_EXPECT(slot2 == nullptr))
+                        logic.on_closed(slot2);
+                    logic.on_closed(slot1);
+                }
+                ++n;
+            }
+            clock.advance(std::chrono::seconds(1));
+            logic.once_per_second();
+        }
+        BEAST_EXPECT(n <= (seconds + 59) / 60);
+    }
+
+    void
+    test_duplicateInOut()
+    {
+        auto const seconds = 10000;
+        testcase("duplicate in/out");
+        TestStore store;
+        TestChecker checker;
+        TestStopwatch clock;
+        Logic<TestChecker> logic(clock, store, checker, journal_);
+        logic.addFixedPeer(
+            "test", beast::IP::Endpoint::from_string("65.0.0.1:5"));
+        {
+            Config c;
+            c.autoConnect = false;
+            c.listeningPort = 1024;
+            c.ipLimit = 2;
+            logic.config(c);
+        }
+
+        std::size_t n = 0;
+        for (std::size_t i = 0; i < seconds; ++i)
+        {
+            auto const list = logic.autoconnect();
+            if (!list.empty())
+            {
+                BEAST_EXPECT(list.size() == 1);
+                auto const remote = list.front();
+                auto const local =
+                    beast::IP::Endpoint::from_string("65.0.0.2:1024");
+                auto const slot1 = logic.new_inbound_slot(local, remote);
+                if (BEAST_EXPECT(slot1 != nullptr))
+                {
+                    BEAST_EXPECT(
+                        logic.connectedAddresses_.count(remote.address()) == 1);
+                    auto const slot2 = logic.new_outbound_slot(remote);
+                    BEAST_EXPECT(
+                        logic.connectedAddresses_.count(remote.address()) == 1);
+                    if (!BEAST_EXPECT(slot2 == nullptr))
+                        logic.on_closed(slot2);
+                    logic.on_closed(slot1);
+                }
+                ++n;
+            }
+            clock.advance(std::chrono::seconds(1));
+            logic.once_per_second();
+        }
+        BEAST_EXPECT(n <= (seconds + 59) / 60);
+    }
+
     void
     test_config()
     {
@@ -279,6 +378,8 @@ class PeerFinder_test : public beast::unit_test::suite
     {
         test_backoff1();
         test_backoff2();
+        test_duplicateOutIn();
+        test_duplicateInOut();
         test_config();
         test_invalid_config();
     }