Fix on handshake failure not disconnecting connection (#6179)

* Fix on handshake failure not disconnecting * Addressing comment
NethermindEth · Oct 23, 2023 · 7a43ff0 · 7a43ff0
1 parent 645717b
commit 7a43ff0
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 2 deletions.
diff --git a/src/Nethermind/Nethermind.Network.Test/Rlpx/Handshake/NettyHandshakeHandlerTests.cs b/src/Nethermind/Nethermind.Network.Test/Rlpx/Handshake/NettyHandshakeHandlerTests.cs
@@ -16,6 +16,7 @@
 using Nethermind.Network.Rlpx;
 using Nethermind.Network.Rlpx.Handshake;
 using NSubstitute;
+using NSubstitute.ExceptionExtensions;
 using NUnit.Framework;
 
 namespace Nethermind.Network.Test.Rlpx.Handshake
@@ -64,6 +65,11 @@ public void Setup()
 
         private NettyHandshakeHandler CreateHandler(HandshakeRole handshakeRole = HandshakeRole.Recipient)
         {
+            if (handshakeRole == HandshakeRole.Recipient)
+            {
+                _session.Node.Throws(new InvalidOperationException("property throw on incoming connection before handshake"));
+                _session.RemoteNodeId.Returns((PublicKey)null); // Incoming connection have null remote node id until handshake finished
+            }
             return new NettyHandshakeHandler(_serializationService, _handshakeService, _session, handshakeRole, _logger, _group, TimeSpan.Zero);
         }
 
@@ -211,5 +217,16 @@ public void Recipient_sends_ack_on_receiving_auth()
             received.Should().BeTrue();
             _handshakeService.Received(1).Ack(Arg.Any<EncryptionHandshake>(), Arg.Any<Packet>());
         }
+
+        [Test]
+        public async Task Handler_disconnect_on_exception()
+        {
+            NettyHandshakeHandler handler = CreateHandler();
+
+            IChannelHandlerContext context = Substitute.For<IChannelHandlerContext>();
+            handler.ExceptionCaught(context, new Exception("any exception"));
+
+            await context.Received().DisconnectAsync();
+        }
     }
 }
diff --git a/src/Nethermind/Nethermind.Network/Rlpx/NettyHandshakeHandler.cs b/src/Nethermind/Nethermind.Network/Rlpx/NettyHandshakeHandler.cs
@@ -110,7 +110,9 @@ public override void ChannelRegistered(IChannelHandlerContext context)
 
         public override void ExceptionCaught(IChannelHandlerContext context, Exception exception)
         {
-            string clientId = _session?.Node?.ToString(Node.Format.Console) ?? $"unknown {_session?.RemoteHost}";
+            string clientId = $"unknown {_session?.RemoteHost}";
+            if (_session.RemoteNodeId != null) clientId = _session?.Node?.ToString(Node.Format.Console);
+
             //In case of SocketException we log it as debug to avoid noise
             if (exception is SocketException)
             {
@@ -127,7 +129,7 @@ public override void ExceptionCaught(IChannelHandlerContext context, Exception e
                 }
             }
 
-            base.ExceptionCaught(context, exception);
+            _ = context.DisconnectAsync();
         }
 
         protected override void ChannelRead0(IChannelHandlerContext context, IByteBuffer input)