Skip to content

Commit 3895341

Browse files
Client setinfo ignore all exception
Check the character legality of clientName in advance through validateClientInfo, so that character exceptions are ignored in initializeFromClientConfig. The remaining errors, NOAUTH, UNKNOWN command, SYNTAX error, NOPREM should be ignored.
1 parent 6822e33 commit 3895341

File tree

3 files changed

+53
-33
lines changed

3 files changed

+53
-33
lines changed

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

Lines changed: 35 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,23 @@ public List<Object> getMany(final int count) {
370370
return responses;
371371
}
372372

373+
/**
374+
* Check if the client name libname, libver, characters are legal
375+
* @param info the name
376+
* @return Returns true if legal, false throws exception
377+
* @throws JedisException if characters illegal
378+
*/
379+
public static boolean validateClientInfo(String info) {
380+
for (int i = 0; i < info.length(); i++) {
381+
char c = info.charAt(i);
382+
if (c < '!' || c > '~') {
383+
throw new JedisException("client info cannot contain spaces, "
384+
+ "newlines or special characters.");
385+
}
386+
}
387+
return true;
388+
}
389+
373390
private void initializeFromClientConfig(JedisClientConfig config) {
374391
try {
375392
connect();
@@ -387,9 +404,12 @@ private void initializeFromClientConfig(JedisClientConfig config) {
387404

388405
Supplier<RedisCredentials> credentialsProvider = config.getCredentialsProvider();
389406
if (credentialsProvider instanceof RedisCredentialsProvider) {
390-
((RedisCredentialsProvider) credentialsProvider).prepare();
391-
auth(credentialsProvider);
392-
((RedisCredentialsProvider) credentialsProvider).cleanUp();
407+
try {
408+
((RedisCredentialsProvider) credentialsProvider).prepare();
409+
auth(credentialsProvider);
410+
} finally {
411+
((RedisCredentialsProvider) credentialsProvider).cleanUp();
412+
}
393413
} else {
394414
auth(credentialsProvider);
395415
}
@@ -398,59 +418,37 @@ private void initializeFromClientConfig(JedisClientConfig config) {
398418
hello(protocol);
399419
}
400420
}
401-
/// HELLO and AUTH
402-
403-
List<CommandArguments> fireAndForgetMsg = new ArrayList<>();
404421

405422
int dbIndex = config.getDatabase();
406423
if (dbIndex > 0) {
407-
fireAndForgetMsg.add(new CommandArguments(Command.SELECT).add(Protocol.toByteArray(dbIndex)));
424+
select(dbIndex);
408425
}
409426

427+
List<CommandArguments> fireAndForgetMsg = new ArrayList<>();
428+
410429
String clientName = config.getClientName();
411-
if (doClientName && clientName != null) {
430+
if (doClientName && clientName != null && validateClientInfo(clientName)) {
412431
fireAndForgetMsg.add(new CommandArguments(Command.CLIENT).add(Keyword.SETNAME).add(clientName));
413432
}
414433

415434
String libName = JedisMetaInfo.getArtifactId();
416-
if (libName != null) {
435+
if (libName != null && validateClientInfo(libName)) {
417436
fireAndForgetMsg.add(new CommandArguments(Command.CLIENT).add(Keyword.SETINFO)
418437
.add(ClientAttributeOption.LIB_NAME.getRaw()).add(libName));
419438
}
420439

421440
String libVersion = JedisMetaInfo.getVersion();
422-
if (libVersion != null) {
441+
if (libVersion != null && validateClientInfo(libVersion)) {
423442
fireAndForgetMsg.add(new CommandArguments(Command.CLIENT).add(Keyword.SETINFO)
424443
.add(ClientAttributeOption.LIB_VER.getRaw()).add(libVersion));
425444
}
426445

427446
for (CommandArguments arg : fireAndForgetMsg) {
428447
sendCommand(arg);
429448
}
430-
431-
List<Object> objects = getMany(fireAndForgetMsg.size());
432-
for (Object obj : objects) {
433-
if (obj instanceof JedisDataException) {
434-
JedisDataException e = (JedisDataException)obj;
435-
String errorMsg = e.getMessage().toUpperCase();
436-
/**
437-
* 1. Redis 4.0 and before, we need to ignore `Syntax error`.
438-
* 2. Redis 5.0 and later, we need to ignore `Unknown subcommand error`.
439-
* 3. Because Jedis allows Jedis jedis = new Jedis() in advance, and jedis.auth(password) later,
440-
* we need to ignore `NOAUTH errors`.
441-
*/
442-
if (errorMsg.contains("SYNTAX") ||
443-
errorMsg.contains("UNKNOWN") ||
444-
errorMsg.contains("NOAUTH")) { // TODO: not filter out NOAUTH
445-
// ignore
446-
} else {
447-
throw e;
448-
}
449-
}
450-
}
449+
getMany(fireAndForgetMsg.size());
451450
} catch (JedisException je) {
452451
try {
453-
setBroken();
454452
disconnect();
455453
} catch (Exception e) {
456454
// the first exception 'je' will be thrown
@@ -504,6 +502,11 @@ private void auth(final Supplier<RedisCredentials> credentialsProvider) {
504502
getStatusCodeReply(); // OK
505503
}
506504

505+
public String select(final int index) {
506+
sendCommand(Protocol.Command.SELECT, Protocol.toByteArray(index));
507+
return getStatusCodeReply();
508+
}
509+
507510
public boolean ping() {
508511
sendCommand(Protocol.Command.PING);
509512
String status = getStatusCodeReply();

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.apache.commons.pool2.PooledObjectFactory;
1414
import org.apache.commons.pool2.impl.DefaultPooledObject;
1515
import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
16+
import org.junit.Assert;
1617
import org.junit.Test;
1718

1819
import redis.clients.jedis.exceptions.InvalidURIException;
@@ -219,6 +220,17 @@ public void customClientName() {
219220
}
220221
}
221222

223+
@Test
224+
public void invalidClientName() {
225+
try (JedisPool pool = new JedisPool(new JedisPoolConfig(), hnp.getHost(), hnp.getPort(), 2000,
226+
"foobared", 0, "invalid client name"); Jedis jedis = pool.getResource()) {
227+
} catch (Exception e) {
228+
if (!e.getMessage().startsWith("client info cannot contain space")) {
229+
Assert.fail("invalid client name test fail");
230+
}
231+
}
232+
}
233+
222234
@Test
223235
public void returnResourceDestroysResourceOnException() {
224236

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,12 @@ public void prepare() {
194194
@Override
195195
public RedisCredentials get() {
196196
if (!validPassword.get()) {
197-
return new RedisCredentials() { };
197+
return new RedisCredentials() {
198+
@Override
199+
public char[] getPassword() {
200+
return "invalidPass".toCharArray();
201+
}
202+
};
198203
}
199204

200205
return new RedisCredentials() {

0 commit comments

Comments
 (0)