JedisSentinelPool contains a buggy constructor

fancige commented 3 years ago

Expected behavior

Can connect to sentinel redis with password

Actual behavior

redis.clients.jedis.exceptions.JedisDataException: NOAUTH Authentication required. at redis.clients.jedis.Protocol.processError(Protocol.java:135) at redis.clients.jedis.Protocol.process(Protocol.java:169) at redis.clients.jedis.Protocol.read(Protocol.java:223) at redis.clients.jedis.Connection.readProtocolWithCheckingBroken(Connection.java:352) at redis.clients.jedis.Connection.getUnflushedObjectMultiBulkReply(Connection.java:314) at redis.clients.jedis.Connection.getObjectMultiBulkReply(Connection.java:319) at redis.clients.jedis.Jedis.sentinelGetMasterAddrByName(Jedis.java:3451) at redis.clients.jedis.JedisSentinelPool.initSentinels(JedisSentinelPool.java:253) at redis.clients.jedis.JedisSentinelPool.(JedisSentinelPool.java:205) at redis.clients.jedis.JedisSentinelPool.(JedisSentinelPool.java:186) at redis.clients.jedis.JedisSentinelPool.(JedisSentinelPool.java:169) at redis.clients.jedis.JedisSentinelPool.(JedisSentinelPool.java:159)

Steps to reproduce:

Please create a reproducible case of your problem. Make sure that case repeats consistently and it's not random 1.use this constructor below, source code line 154, to create a JedisSentinelPool

JedisSentinelPool(String masterName, Set sentinels, final GenericObjectPoolConfig poolConfig, final int connectionTimeout, final int soTimeout, final String user, final String password, final int database, final String clientName, final int sentinelConnectionTimeout, final int sentinelSoTimeout, final String sentinelUser, final String sentinelPassword, final String sentinelClientName)

2.then the constructor below, source code line 186, invoke DefaultJedisClientConfig.builder().build() to create an empty JedisClientConfig which would cause the all informations passed by the first step lose

public JedisSentinelPool(String masterName, Set sentinels, final GenericObjectPoolConfig poolConfig, final JedisFactory factory) { this(masterName, parseHostAndPorts(sentinels), poolConfig, factory, DefaultJedisClientConfig.builder().build()); }

3.

Redis / Jedis Configuration

Jedis version:

3.6.1

Redis version:

5.0.4

Java version:

1.8

sazzad16 commented 3 years ago

@fancige

  public JedisSentinelPool(String masterName, Set<String> sentinels,
      final GenericObjectPoolConfig<Jedis> poolConfig,
      final int connectionTimeout, final int soTimeout, final int infiniteSoTimeout,
      final String user, final String password, final int database, final String clientName,
      final int sentinelConnectionTimeout, final int sentinelSoTimeout, final String sentinelUser,
      final String sentinelPassword, final String sentinelClientName)

which calls

  public JedisSentinelPool(String masterName, Set<HostAndPort> sentinels,
      final GenericObjectPoolConfig<Jedis> poolConfig, final JedisClientConfig masteClientConfig,
      final JedisClientConfig sentinelClientConfig)

which then finally calls

  public JedisSentinelPool(String masterName, Set<HostAndPort> sentinels,
      final GenericObjectPoolConfig<Jedis> poolConfig, final JedisFactory factory,
      final JedisClientConfig sentinelClientConfig)

Nothing is lost in any of those stages.

sazzad16 commented 3 years ago

PS: DefaultJedisClientConfig.builder().build() is used only in

  public JedisSentinelPool(String masterName, Set<String> sentinels,
      final GenericObjectPoolConfig<Jedis> poolConfig, final JedisFactory factory)

where no sentinel socket configs are provided.

fancige commented 3 years ago

PS: DefaultJedisClientConfig.builder().build() is used only in
  public JedisSentinelPool(String masterName, Set<String> sentinels,
      final GenericObjectPoolConfig<Jedis> poolConfig, final JedisFactory factory)
where no sentinel socket configs are provided.

but the constructor is called by annother constructor, check the code at line 169，

this(masterName, sentinels, poolConfig, new JedisFactory(connectionTimeout, soTimeout, infiniteSoTimeout, user, password, database, clientName));

jedis version is 3.6.1

full code (line 154-188) look like this

public JedisSentinelPool(String masterName, Set<String> sentinels,
      final GenericObjectPoolConfig<Jedis> poolConfig, final int connectionTimeout, final int soTimeout,
      final String user, final String password, final int database, final String clientName,
      final int sentinelConnectionTimeout, final int sentinelSoTimeout, final String sentinelUser,
      final String sentinelPassword, final String sentinelClientName) {
    this(masterName, sentinels, poolConfig, connectionTimeout, soTimeout, 0, user, password, database, clientName,
        sentinelConnectionTimeout, sentinelSoTimeout, sentinelUser, sentinelPassword, sentinelClientName);
  }

  public JedisSentinelPool(String masterName, Set<String> sentinels,
      final GenericObjectPoolConfig<Jedis> poolConfig,
      final int connectionTimeout, final int soTimeout, final int infiniteSoTimeout,
      final String user, final String password, final int database, final String clientName,
      final int sentinelConnectionTimeout, final int sentinelSoTimeout, final String sentinelUser,
      final String sentinelPassword, final String sentinelClientName) {
    this(masterName, sentinels, poolConfig, new JedisFactory(connectionTimeout, soTimeout, infiniteSoTimeout, user, password, database, clientName));
    this.connectionTimeout = connectionTimeout;
    this.soTimeout = soTimeout;
    this.infiniteSoTimeout = infiniteSoTimeout;
    this.user = user;
    this.password = password;
    this.database = database;
    this.clientName = clientName;
    this.sentinelConnectionTimeout = sentinelConnectionTimeout;
    this.sentinelSoTimeout = sentinelSoTimeout;
    this.sentinelUser = sentinelUser;
    this.sentinelPassword = sentinelPassword;
    this.sentinelClientName = sentinelClientName;
  }

  public JedisSentinelPool(String masterName, Set<String> sentinels,
      final GenericObjectPoolConfig<Jedis> poolConfig, final JedisFactory factory) {
    this(masterName, parseHostAndPorts(sentinels), poolConfig, factory,
        DefaultJedisClientConfig.builder().build());
  }

sazzad16 commented 3 years ago

@fancige Ahh, got it. Thanks!

sazzad16 commented 3 years ago

Fixed by #2574 and #2582

redis / jedis