Client setinfo ignore all exception (#3449) (#3458)

This PR contains the following changes:

1. Check the character legality of clientName in advance through `validateClientInfo`, so that character exceptions are ignored in `initializeFromClientConfig`.
2. Move the `select` command out of the fire-and-forget message (we care about its return value).
3. For the test of RedisCredentials, we gave it "invalidPass" for the first time to make it fail the verification instead of skipping the verification.

---------

Co-authored-by: bodong.ybd <[email protected]>

Author: sazzad16

src/main/java/redis/clients/jedis/Connection.java

@@ -352,70 +352,69 @@ public List<Object> getMany(final int count) {
     return responses;
   }
 
+  /**
+   * Check if the client name libname, libver, characters are legal
+   * @param info the name
+   * @return Returns true if legal, false throws exception
+   * @throws JedisException if characters illegal
+   */
+  private static boolean validateClientInfo(String info) {
+    for (int i = 0; i < info.length(); i++) {
+      char c = info.charAt(i);
+      if (c < '!' || c > '~') {
+        throw new JedisException("client info cannot contain spaces, "
+            + "newlines or special characters.");
+      }
+    }
+    return true;
+  }
+
   private void initializeFromClientConfig(JedisClientConfig config) {
     try {
       connect();
 
       Supplier<RedisCredentials> credentialsProvider = config.getCredentialsProvider();
       if (credentialsProvider instanceof RedisCredentialsProvider) {
-        ((RedisCredentialsProvider) credentialsProvider).prepare();
-        auth(credentialsProvider);
-        ((RedisCredentialsProvider) credentialsProvider).cleanUp();
+        try {
+            ((RedisCredentialsProvider) credentialsProvider).prepare();
+            auth(credentialsProvider);
+          } finally {
+            ((RedisCredentialsProvider) credentialsProvider).cleanUp();
+          }
       } else {
         auth(credentialsProvider);
       }
 
-      List<CommandArguments> fireAndForgetMsg = new ArrayList<>();
-
       int dbIndex = config.getDatabase();
       if (dbIndex > 0) {
-        fireAndForgetMsg.add(new CommandArguments(Command.SELECT).add(Protocol.toByteArray(dbIndex)));
+        select(dbIndex);
       }
 
+      List<CommandArguments> fireAndForgetMsg = new ArrayList<>();
+
       String clientName = config.getClientName();
-      if (clientName != null) {
+      if (clientName != null && validateClientInfo(clientName)) {
         fireAndForgetMsg.add(new CommandArguments(Command.CLIENT).add(Keyword.SETNAME).add(clientName));
       }
 
       String libName = JedisMetaInfo.getArtifactId();
-      if (libName != null) {
+      if (libName != null && validateClientInfo(libName)) {
         fireAndForgetMsg.add(new CommandArguments(Command.CLIENT).add(Keyword.SETINFO)
             .add(ClientAttributeOption.LIB_NAME.getRaw()).add(libName));
       }
 
       String libVersion = JedisMetaInfo.getVersion();
-      if (libVersion != null) {
+      if (libVersion != null && validateClientInfo(libVersion)) {
         fireAndForgetMsg.add(new CommandArguments(Command.CLIENT).add(Keyword.SETINFO)
             .add(ClientAttributeOption.LIB_VER.getRaw()).add(libVersion));
       }
 
       for (CommandArguments arg : fireAndForgetMsg) {
         sendCommand(arg);
       }
-
-      List<Object> objects = getMany(fireAndForgetMsg.size());
-      for (Object obj : objects) {
-        if (obj instanceof JedisDataException) {
-          JedisDataException e = (JedisDataException)obj;
-          String errorMsg = e.getMessage().toUpperCase();
-          /**
-           * 1. Redis 4.0 and before, we need to ignore `Syntax error`.
-           * 2. Redis 5.0 and later, we need to ignore `Unknown subcommand error`.
-           * 3. Because Jedis allows Jedis jedis = new Jedis() in advance, and jedis.auth(password) later,
-           * we need to ignore `NOAUTH errors`.
-           */
-          if (errorMsg.contains("SYNTAX") ||
-              errorMsg.contains("UNKNOWN") ||
-              errorMsg.contains("NOAUTH")) { // TODO: not filter out NOAUTH
-            // ignore
-          } else {
-            throw e;
-          }
-        }
-      }
+      getMany(fireAndForgetMsg.size());
     } catch (JedisException je) {
       try {
-        setBroken();
         disconnect();
       } catch (Exception e) {
         // the first exception 'je' will be thrown
@@ -447,7 +446,6 @@ private void auth(final Supplier<RedisCredentials> credentialsProvider) {
     getStatusCodeReply(); // OK
   }
 
-  @Deprecated
   public String select(final int index) {
     sendCommand(Protocol.Command.SELECT, Protocol.toByteArray(index));
     return getStatusCodeReply();

src/test/java/redis/clients/jedis/JedisPoolTest.java

@@ -13,6 +13,7 @@
 import org.apache.commons.pool2.PooledObjectFactory;
 import org.apache.commons.pool2.impl.DefaultPooledObject;
 import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
+import org.junit.Assert;
 import org.junit.Test;
 
 import redis.clients.jedis.exceptions.InvalidURIException;
@@ -219,6 +220,17 @@ public void customClientName() {
     }
   }
 
+  @Test
+  public void invalidClientName() {
+    try (JedisPool pool = new JedisPool(new JedisPoolConfig(), hnp.getHost(), hnp.getPort(), 2000,
+        "foobared", 0, "invalid client name"); Jedis jedis = pool.getResource()) {
+    } catch (Exception e) {
+      if (!e.getMessage().startsWith("client info cannot contain space")) {
+        Assert.fail("invalid client name test fail");
+      }
+    }
+  }
+
   @Test
   public void returnResourceDestroysResourceOnException() {
 

src/test/java/redis/clients/jedis/JedisPooledTest.java

@@ -9,6 +9,7 @@
 import java.net.URISyntaxException;
 import java.util.concurrent.atomic.AtomicInteger;
 import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
+import org.junit.Assert;
 import org.junit.Test;
 
 import redis.clients.jedis.exceptions.JedisConnectionException;
@@ -98,6 +99,18 @@ public void customClientName() {
     }
   }
 
+  @Test
+  public void invalidClientName() {
+    try (JedisPooled pool = new JedisPooled(hnp, DefaultJedisClientConfig.builder()
+        .clientName("invalid client name").build());
+         Connection jedis = pool.getPool().getResource()) {
+    } catch (Exception e) {
+      if (!e.getMessage().startsWith("client info cannot contain space")) {
+        Assert.fail("invalid client name test fail");
+      }
+    }
+  }
+
   @Test
   public void getNumActiveWhenPoolIsClosed() {
     JedisPooled pool = new JedisPooled(hnp);
@@ -209,7 +222,12 @@ public void prepare() {
       public RedisCredentials get() {
         if (firstCall) {
           firstCall = false;
-          return new RedisCredentials() { };
+          return new RedisCredentials() {
+            @Override
+            public char[] getPassword() {
+              return "invalidPass".toCharArray();
+            }
+          };
         }
 
         return new RedisCredentials() {