From 74b6d98d041531decf7fcf4b48619bd60d1d7107 Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 25 Jun 2025 07:43:03 -0400 Subject: [PATCH 1/2] AMQNET-848-Failover-Transport-Protocol-Excessive-Reconnection-Attempts-on-Credential-Failure --- src/Transport/Failover/FailoverTransport.cs | 10 ++- src/Transport/InactivityMonitor.cs | 25 ++++-- src/Util/ExceptionFromBrokerError.cs | 78 +++++++++++++++++++ .../Inactivity/InactivityMonitorTest.cs | 45 +++++++++++ 4 files changed, 149 insertions(+), 9 deletions(-) create mode 100644 src/Util/ExceptionFromBrokerError.cs diff --git a/src/Transport/Failover/FailoverTransport.cs b/src/Transport/Failover/FailoverTransport.cs index ce78cc1..e551116 100644 --- a/src/Transport/Failover/FailoverTransport.cs +++ b/src/Transport/Failover/FailoverTransport.cs @@ -468,9 +468,13 @@ public void HandleTransportFailure(Exception e) { if (CanReconnect()) { - Tracer.WarnFormat("Transport failed to {0}, attempting to automatically reconnect due to: {1}", - ConnectedTransportURI, e.Message); - reconnectOk = true; + //Check to see if the exception is a security exception + if (!(e is NMSSecurityException)) + { + Tracer.WarnFormat("Transport failed to {0}, attempting to automatically reconnect due to: {1}", + ConnectedTransportURI, e.Message); + reconnectOk = true; + } } initialized = false; diff --git a/src/Transport/InactivityMonitor.cs b/src/Transport/InactivityMonitor.cs index 4c672f6..b6633bf 100644 --- a/src/Transport/InactivityMonitor.cs +++ b/src/Transport/InactivityMonitor.cs @@ -19,6 +19,7 @@ using System.Threading; using Apache.NMS.ActiveMQ.Commands; using Apache.NMS.ActiveMQ.Threads; +using Apache.NMS.ActiveMQ.Util; using Apache.NMS.ActiveMQ.Util.Synchronization; using Apache.NMS.Util; @@ -230,32 +231,44 @@ protected override async System.Threading.Tasks.Task OnCommand(ITransport sender inRead.Value = true; try { - if(command.IsKeepAliveInfo) + if (command is ExceptionResponse) + { + ExceptionResponse error = command as ExceptionResponse; + NMSException exception = ExceptionFromBrokerError.CreateExceptionFromBrokerError(error.Exception); + if (exception is NMSSecurityException) + { + OnException(this, exception); + } + else + { + Tracer.WarnFormat("ExceptionResponse received from the broker:", command.GetType()); + } + }else if (command.IsKeepAliveInfo) { KeepAliveInfo info = command as KeepAliveInfo; - if(info.ResponseRequired) + if (info.ResponseRequired) { try { info.ResponseRequired = false; Oneway(info); } - catch(IOException ex) + catch (IOException ex) { OnException(this, ex); } } } - else if(command.IsWireFormatInfo) + else if (command.IsWireFormatInfo) { - lock(monitor) + lock (monitor) { remoteWireFormatInfo = command as WireFormatInfo; try { StartMonitorThreads(); } - catch(IOException ex) + catch (IOException ex) { OnException(this, ex); } diff --git a/src/Util/ExceptionFromBrokerError.cs b/src/Util/ExceptionFromBrokerError.cs new file mode 100644 index 0000000..006cf80 --- /dev/null +++ b/src/Util/ExceptionFromBrokerError.cs @@ -0,0 +1,78 @@ +using Apache.NMS.ActiveMQ.Commands; +using System; +using System.Reflection; + + +namespace Apache.NMS.ActiveMQ.Util +{ + internal class ExceptionFromBrokerError + { + public static NMSException CreateExceptionFromBrokerError(BrokerError brokerError) + { + String exceptionClassName = brokerError.ExceptionClass; + + if (String.IsNullOrEmpty(exceptionClassName)) + { + return new BrokerException(brokerError); + } + + NMSException exception = null; + String message = brokerError.Message; + + // We only create instances of exceptions from the NMS API + Assembly nmsAssembly = Assembly.GetAssembly(typeof(NMSException)); + + // First try and see if it's one we populated ourselves in which case + // it will have the correct namespace and exception name. + Type exceptionType = nmsAssembly.GetType(exceptionClassName, false, true); + + // Exceptions from the broker don't have the same namespace, so we + // trim that and try using the NMS namespace to see if we can get an + // NMSException based version of the same type. We have to convert + // the JMS prefixed exceptions to NMS also. + if (null == exceptionType) + { + if (exceptionClassName.StartsWith("java.lang.SecurityException")) + { + exceptionClassName = "Apache.NMS.NMSSecurityException"; + } + else if (!exceptionClassName.StartsWith("Apache.NMS")) + { + string transformClassName; + + if (exceptionClassName.Contains(".")) + { + int pos = exceptionClassName.LastIndexOf("."); + transformClassName = exceptionClassName.Substring(pos + 1).Replace("JMS", "NMS"); + } + else + { + transformClassName = exceptionClassName; + } + + exceptionClassName = "Apache.NMS." + transformClassName; + } + + exceptionType = nmsAssembly.GetType(exceptionClassName, false, true); + } + + if (exceptionType != null) + { + object[] args = null; + if (!String.IsNullOrEmpty(message)) + { + args = new object[1]; + args[0] = message; + } + + exception = Activator.CreateInstance(exceptionType, args) as NMSException; + } + else + { + exception = new BrokerException(brokerError); + } + + return exception; + } + } +} diff --git a/test/Transport/Inactivity/InactivityMonitorTest.cs b/test/Transport/Inactivity/InactivityMonitorTest.cs index ba5fb4f..618a444 100644 --- a/test/Transport/Inactivity/InactivityMonitorTest.cs +++ b/test/Transport/Inactivity/InactivityMonitorTest.cs @@ -133,6 +133,51 @@ public void TestWriteMessageFail() { } } + public class TestableInactivityMonitor : InactivityMonitor + { + public TestableInactivityMonitor(ITransport next) : base(next) { } + + // Expose protected method for testing + public async Task TestOnCommand(ITransport sender, Command command) + { + await OnCommand(sender, command); + } + } + [Test] + public void OnCommand_WithNMSSecurityException_ShouldCallOnException() + { + // Arrange + var brokerError = new BrokerError + { + ExceptionClass = "javax.jms.JMSSecurityException", + Message = "Authentication failed" + }; + + var exceptionResponse = new ExceptionResponse + { + Exception = brokerError + }; + + // Mock the static method call - this would require making ExceptionFromBrokerError testable + // For this test, we'll assume it returns an NMSSecurityException + var securityException = new NMSSecurityException("Authentication failed"); + TestableInactivityMonitor monitor = new TestableInactivityMonitor(this.transport); + monitor.Exception += new ExceptionHandler(OnException); + bool exceptionHandlerCalled = false; + Exception caughtException = null; + monitor.Exception += (sender, args) => + { + exceptionHandlerCalled = true; + caughtException = args; + }; + // Act + _ = monitor.TestOnCommand(transport, exceptionResponse); + // Assert + Assert.IsTrue(exceptionHandlerCalled, "Exception handler should have been called"); + Assert.IsNotNull(caughtException, "Exception should have been caught"); + Assert.IsInstanceOf(caughtException, "Should be NMSSecurityException"); + Assert.AreEqual("Authentication failed", caughtException.Message); + } [Test] public void TestNonFailureSendCase() From d0157e5e96f20c979376cce4f7852671e614116d Mon Sep 17 00:00:00 2001 From: unknown Date: Sun, 13 Jul 2025 07:42:18 -0400 Subject: [PATCH 2/2] Fixed the co pilot review comments --- src/Transport/InactivityMonitor.cs | 2 +- test/Transport/Inactivity/InactivityMonitorTest.cs | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Transport/InactivityMonitor.cs b/src/Transport/InactivityMonitor.cs index b6633bf..1e2d0f0 100644 --- a/src/Transport/InactivityMonitor.cs +++ b/src/Transport/InactivityMonitor.cs @@ -241,7 +241,7 @@ protected override async System.Threading.Tasks.Task OnCommand(ITransport sender } else { - Tracer.WarnFormat("ExceptionResponse received from the broker:", command.GetType()); + Tracer.WarnFormat("ExceptionResponse received from the broker:{0}", command.GetType()); } }else if (command.IsKeepAliveInfo) { diff --git a/test/Transport/Inactivity/InactivityMonitorTest.cs b/test/Transport/Inactivity/InactivityMonitorTest.cs index 618a444..f5aeee2 100644 --- a/test/Transport/Inactivity/InactivityMonitorTest.cs +++ b/test/Transport/Inactivity/InactivityMonitorTest.cs @@ -138,9 +138,9 @@ public class TestableInactivityMonitor : InactivityMonitor public TestableInactivityMonitor(ITransport next) : base(next) { } // Expose protected method for testing - public async Task TestOnCommand(ITransport sender, Command command) + public Task TestOnCommand(ITransport sender, Command command) { - await OnCommand(sender, command); + return OnCommand(sender, command); } } [Test] @@ -163,6 +163,7 @@ public void OnCommand_WithNMSSecurityException_ShouldCallOnException() var securityException = new NMSSecurityException("Authentication failed"); TestableInactivityMonitor monitor = new TestableInactivityMonitor(this.transport); monitor.Exception += new ExceptionHandler(OnException); + monitor.CommandAsync += new CommandHandlerAsync(OnCommand); bool exceptionHandlerCalled = false; Exception caughtException = null; monitor.Exception += (sender, args) => @@ -171,7 +172,8 @@ public void OnCommand_WithNMSSecurityException_ShouldCallOnException() caughtException = args; }; // Act - _ = monitor.TestOnCommand(transport, exceptionResponse); + Task task=monitor.TestOnCommand(transport, exceptionResponse); + task.Wait(); // Assert Assert.IsTrue(exceptionHandlerCalled, "Exception handler should have been called"); Assert.IsNotNull(caughtException, "Exception should have been caught");