Propagate exceptions during connection handshake after protocol downg…

…rade #2313 We now correctly propagate exceptions that can happen from RESP2 handshake initiator. Previously, these exceptions didn't bubble up and went unnoticed resulting in a handshake timeout.
redis · Jul 25, 2023 · f43581c · f43581c
1 parent 249e40f
commit f43581c
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 19 deletions.
diff --git a/src/main/java/io/lettuce/core/RedisCommandBuilder.java b/src/main/java/io/lettuce/core/RedisCommandBuilder.java
@@ -205,7 +205,6 @@ Command<K, V, String> asking() {
 
     Command<K, V, String> auth(CharSequence password) {
         LettuceAssert.notNull(password, "Password " + MUST_NOT_BE_NULL);
-        LettuceAssert.notEmpty(password, "Password " + MUST_NOT_BE_EMPTY);
 
         char[] chars = new char[password.length()];
         for (int i = 0; i < password.length(); i++) {
@@ -216,7 +215,6 @@ Command<K, V, String> auth(CharSequence password) {
 
     Command<K, V, String> auth(char[] password) {
         LettuceAssert.notNull(password, "Password " + MUST_NOT_BE_NULL);
-        LettuceAssert.isTrue(password.length > 0, "Password " + MUST_NOT_BE_EMPTY);
 
         CommandArgs<K, V> args = new CommandArgs<>(codec).add(password);
         return createCommand(AUTH, new StatusOutput<>(codec), args);
@@ -226,7 +224,6 @@ Command<K, V, String> auth(String username, CharSequence password) {
         LettuceAssert.notNull(username, "Username " + MUST_NOT_BE_NULL);
         LettuceAssert.isTrue(!username.isEmpty(), "Username " + MUST_NOT_BE_EMPTY);
         LettuceAssert.notNull(password, "Password " + MUST_NOT_BE_NULL);
-        LettuceAssert.notEmpty(password, "Password " + MUST_NOT_BE_EMPTY);
 
         char[] chars = new char[password.length()];
         for (int i = 0; i < password.length(); i++) {
@@ -239,7 +236,6 @@ Command<K, V, String> auth(String username, char[] password) {
         LettuceAssert.notNull(username, "Username " + MUST_NOT_BE_NULL);
         LettuceAssert.isTrue(!username.isEmpty(), "Username " + MUST_NOT_BE_EMPTY);
         LettuceAssert.notNull(password, "Password " + MUST_NOT_BE_NULL);
-        LettuceAssert.isTrue(password.length > 0, "Password " + MUST_NOT_BE_EMPTY);
 
         CommandArgs<K, V> args = new CommandArgs<>(codec).add(username).add(password);
         return createCommand(AUTH, new StatusOutput<>(codec), args);

diff --git a/src/main/java/io/lettuce/core/RedisCredentials.java b/src/main/java/io/lettuce/core/RedisCredentials.java
@@ -45,7 +45,7 @@ public interface RedisCredentials {
     /**
      * Retrieve the Redis password, used to authenticate the user interacting with Redis.
      *
-     * @return the password Can be {@code null} if not set.
+     * @return the password. Can be {@code null} if not set.
      * @see #hasUsername()
      */
     char[] getPassword();

diff --git a/src/main/java/io/lettuce/core/RedisHandshake.java b/src/main/java/io/lettuce/core/RedisHandshake.java
@@ -105,7 +105,12 @@ private CompletionStage<?> tryHandshakeResp3(Channel channel) {
 
             if (throwable != null) {
                 if (isUnknownCommand(throwable)) {
-                    fallbackToResp2(channel, handshake);
+                    try {
+                        fallbackToResp2(channel, handshake);
+                    } catch (Exception e) {
+                        e.addSuppressed(throwable);
+                        handshake.completeExceptionally(e);
+                    }
                 } else {
                     handshake.completeExceptionally(throwable);
                 }

diff --git a/src/main/java/io/lettuce/core/RedisURI.java b/src/main/java/io/lettuce/core/RedisURI.java
@@ -473,7 +473,7 @@ public char[] getPassword() {
      * Sets the password. Use empty string to skip authentication.
      * <p>
      * This method is deprecated as of Lettuce 6.0. The reason is that {@link String} has a strong caching affinity and the JVM
-     * cannot easily GC {@code String} instances. Therefore we suggest using either {@code char[]} or a custom
+     * cannot easily GC {@code String} instances. Therefore, we suggest using either {@code char[]} or a custom
      * {@link CharSequence} (e.g. {@link StringBuilder} or netty's {@link io.netty.util.AsciiString}).
      *
      * @param password the password, must not be {@code null}.
@@ -487,7 +487,8 @@ public void setPassword(String password) {
     }
 
     /**
-     * Sets the password. Use empty string to skip authentication.
+     * Sets the password. Use {@code null} to skip authentication. Empty password is supported (although not recommended for
+     * security reasons).
      *
      * @param password the password, must not be {@code null}.
      * @throws IllegalStateException if a {@link RedisCredentialsProvider} is configured
@@ -502,7 +503,8 @@ public void setPassword(CharSequence password) {
     }
 
     /**
-     * Sets the password. Use empty char array to skip authentication.
+     * Sets the password. Use {@code null} to skip authentication. Empty password is supported (although not recommended for
+     * security reasons).
      *
      * @param password the password, can be {@code null}.
      * @throws IllegalStateException if a {@link RedisCredentialsProvider} is configured
@@ -1586,7 +1588,7 @@ public Builder withAuthentication(RedisURI source) {
         }
 
         /**
-         * Configures authentication.
+         * Configures authentication. Empty password is supported (although not recommended for security reasons).
          *
          * @param username the user name
          * @param password the password name
@@ -1617,7 +1619,7 @@ public Builder withAuthentication(RedisCredentialsProvider credentialsProvider)
         }
 
         /**
-         * Configures authentication.
+         * Configures authentication. Empty password is supported (although not recommended for security reasons).
          * <p>
          * This method is deprecated as of Lettuce 6.0. The reason is that {@link String} has a strong caching affinity and the
          * JVM cannot easily GC {@code String} instances. Therefore, we suggest using either {@code char[]} or a custom
@@ -1637,7 +1639,7 @@ public Builder withPassword(String password) {
         }
 
         /**
-         * Configures authentication.
+         * Configures authentication. Empty password is supported (although not recommended for security reasons).
          *
          * @param password the password
          * @return the builder
@@ -1656,7 +1658,7 @@ public Builder withPassword(CharSequence password) {
         }
 
         /**
-         * Configures authentication.
+         * Configures authentication. Empty password is supported (although not recommended for security reasons).
          *
          * @param password the password
          * @return the builder

diff --git a/src/test/java/io/lettuce/core/ConnectionCommandIntegrationTests.java b/src/test/java/io/lettuce/core/ConnectionCommandIntegrationTests.java
@@ -214,12 +214,6 @@ void authNull() {
         assertThatThrownBy(() -> redis.auth(null, "x")).isInstanceOf(IllegalArgumentException.class);
     }
 
-    @Test
-    void authEmpty() {
-        assertThatThrownBy(() -> redis.auth("")).isInstanceOf(IllegalArgumentException.class);
-        assertThatThrownBy(() -> redis.auth("", "x")).isInstanceOf(IllegalArgumentException.class);
-    }
-
     @Test
     void authReconnect() {
         WithPassword.run(client, () -> {