[mina-sshd] 04/04: [SSHD-1050] Make first error before first auth attempt sticky

[lg...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

lgoldstein pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git

commit a966070d8b793a00f0e479d984f2e431fcee229c
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Thu Aug 6 18:33:54 2020 +0300

    [SSHD-1050] Make first error before first auth attempt sticky
---
 .../sshd/client/session/ClientSessionImpl.java     | 13 ++++++++-
 .../sshd/client/session/ClientSessionTest.java     | 32 ++++++++++++++++------
 2 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java b/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java
index a7c07c9..72c41ae 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java
@@ -59,6 +59,7 @@ public class ClientSessionImpl extends AbstractClientSession {
      */
     private volatile AuthFuture authFuture;
 
+    private final AtomicReference<Throwable> beforeAuthErrorHolder = new AtomicReference<>();
     /** Also guards setting an earlyError and the authFuture together. */
     private final AtomicReference<Throwable> authErrorHolder = new AtomicReference<>();
 
@@ -136,7 +137,13 @@ public class ClientSessionImpl extends AbstractClientSession {
         synchronized (authErrorHolder) {
             future = ValidateUtils.checkNotNull(
                     authService.auth(serviceName), "No auth future generated by service=%s", serviceName);
-            earlyError = authErrorHolder.getAndSet(null);
+            // If have an error before the 1st auth then make it "sticky"
+            Throwable beforeAuthError = beforeAuthErrorHolder.get();
+            if (authFuture != null) {
+                earlyError = authErrorHolder.getAndSet(beforeAuthError);
+            } else {
+                earlyError = beforeAuthError;
+            }
             authFuture = future;
         }
 
@@ -180,6 +187,10 @@ public class ClientSessionImpl extends AbstractClientSession {
                 // save only the 1st newly signaled exception
                 firstError = authErrorHolder.compareAndSet(null, t);
                 future = authFuture;
+                // save in special location errors before the 1st auth attempt
+                if (future == null) {
+                    beforeAuthErrorHolder.compareAndSet(null, t);
+                }
             }
         }
 
diff --git a/sshd-core/src/test/java/org/apache/sshd/client/session/ClientSessionTest.java b/sshd-core/src/test/java/org/apache/sshd/client/session/ClientSessionTest.java
index d9e022f..d6c4cf2 100644
--- a/sshd-core/src/test/java/org/apache/sshd/client/session/ClientSessionTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/client/session/ClientSessionTest.java
@@ -252,7 +252,16 @@ public class ClientSessionTest extends BaseTestSupport {
     }
 
     @Test // SSHD-1050
-    public void testAuthGetsNotifiedOnLongPreamble() throws Exception {
+    public void testAuthGetsNotifiedIfErrorBeforeFirstAuth() throws Exception {
+        testEarlyErrorAuthAttempts(1);
+    }
+
+    @Test // SSHD-1050
+    public void testSecondAuthNotifiedAfterEarlyError() throws Exception {
+        testEarlyErrorAuthAttempts(3);
+    }
+
+    private void testEarlyErrorAuthAttempts(int maxAttempts) throws Exception {
         int limit = CoreModuleProperties.MAX_IDENTIFICATION_SIZE.getRequired(sshd);
         String line = getClass().getCanonicalName() + "#" + getCurrentTestName();
         StringBuilder sb = new StringBuilder(limit + line.length());
@@ -271,15 +280,20 @@ public class ClientSessionTest extends BaseTestSupport {
             // Give time to the client to signal the overflow in server identification
             Thread.sleep(AUTH_TIMEOUT.toMillis() / 2L);
 
-            AuthFuture future = session.auth();
-            assertTrue("Auth not completed on time", future.await(AUTH_TIMEOUT));
-            assertTrue("No auth result", future.isDone());
-            assertFalse("Unexpected auth success", future.isSuccess());
-            assertTrue("Auth not marked as failed", future.isFailure());
+            for (int index = 1; index <= maxAttempts; index++) {
+                String authId = "Auth " + index + "/" + maxAttempts;
+                outputDebugMessage("%s(%s)", getCurrentTestName(), authId);
+
+                AuthFuture future = session.auth();
+                assertTrue(authId + " not completed on time", future.await(AUTH_TIMEOUT));
+                assertTrue(authId + " has no result", future.isDone());
+                assertFalse(authId + " unexpected success", future.isSuccess());
+                assertTrue(authId + " not marked as failed", future.isFailure());
 
-            Throwable exception = future.getException();
-            String message = exception.getLocalizedMessage();
-            assertTrue("Invalid exception message: " + message, message.contains("too many header lines"));
+                Throwable exception = future.getException();
+                String message = exception.getMessage();
+                assertTrue(authId + " invalid exception message: " + message, message.contains("too many header lines"));
+            }
         } finally {
             CoreModuleProperties.SERVER_EXTRA_IDENTIFICATION_LINES.set(sshd, null);
         }