Skip to content

Commit 79859aa

Browse files
ningyu1chickenlj
authored andcommitted
Merge pull request #2146, fix redis auth problem for RedisProtocol.
Fixes #2017
1 parent 0278a01 commit 79859aa

File tree

3 files changed

+118
-13
lines changed

3 files changed

+118
-13
lines changed

dubbo-registry/dubbo-registry-redis/src/main/java/com/alibaba/dubbo/registry/redis/RedisRegistry.java

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,6 @@ public RedisRegistry(URL url) {
120120
addresses.addAll(Arrays.asList(backups));
121121
}
122122

123-
String password = url.getPassword();
124123
for (String address : addresses) {
125124
int i = address.indexOf(':');
126125
String host;
@@ -132,15 +131,9 @@ public RedisRegistry(URL url) {
132131
host = address;
133132
port = DEFAULT_REDIS_PORT;
134133
}
135-
if (StringUtils.isEmpty(password)) {
136-
this.jedisPools.put(address, new JedisPool(config, host, port,
137-
url.getParameter(Constants.TIMEOUT_KEY, Constants.DEFAULT_TIMEOUT), null,
138-
url.getParameter("db.index", 0)));
139-
} else {
140-
this.jedisPools.put(address, new JedisPool(config, host, port,
141-
url.getParameter(Constants.TIMEOUT_KEY, Constants.DEFAULT_TIMEOUT), password,
142-
url.getParameter("db.index", 0)));
143-
}
134+
this.jedisPools.put(address, new JedisPool(config, host, port,
135+
url.getParameter(Constants.TIMEOUT_KEY, Constants.DEFAULT_TIMEOUT), StringUtils.isEmpty(url.getPassword()) ? null : url.getPassword(),
136+
url.getParameter("db.index", 0)));
144137
}
145138

146139
this.reconnectPeriod = url.getParameter(Constants.REGISTRY_RECONNECT_PERIOD_KEY, Constants.DEFAULT_REGISTRY_RECONNECT_PERIOD);

dubbo-rpc/dubbo-rpc-redis/src/main/java/com/alibaba/dubbo/rpc/protocol/redis/RedisProtocol.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.alibaba.dubbo.common.serialize.ObjectInput;
2323
import com.alibaba.dubbo.common.serialize.ObjectOutput;
2424
import com.alibaba.dubbo.common.serialize.Serialization;
25+
import com.alibaba.dubbo.common.utils.StringUtils;
2526
import com.alibaba.dubbo.rpc.Exporter;
2627
import com.alibaba.dubbo.rpc.Invocation;
2728
import com.alibaba.dubbo.rpc.Invoker;
@@ -90,7 +91,9 @@ public <T> Invoker<T> refer(final Class<T> type, final URL url) throws RpcExcept
9091
if (url.getParameter("min.evictable.idle.time.millis", 0) > 0)
9192
config.setMinEvictableIdleTimeMillis(url.getParameter("min.evictable.idle.time.millis", 0));
9293
final JedisPool jedisPool = new JedisPool(config, url.getHost(), url.getPort(DEFAULT_PORT),
93-
url.getParameter(Constants.TIMEOUT_KEY, Constants.DEFAULT_TIMEOUT));
94+
url.getParameter(Constants.TIMEOUT_KEY, Constants.DEFAULT_TIMEOUT),
95+
StringUtils.isBlank(url.getPassword()) ? null : url.getPassword(),
96+
url.getParameter("db.index", 0));
9497
final int expiry = url.getParameter("expiry", 0);
9598
final String get = url.getParameter("get", "get");
9699
final String set = url.getParameter("set", Map.class.equals(type) ? "put" : "set");

dubbo-rpc/dubbo-rpc-redis/src/test/java/com/alibaba/dubbo/rpc/protocol/redis/RedisProtocolTest.java

Lines changed: 111 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,32 @@
1616
*/
1717
package com.alibaba.dubbo.rpc.protocol.redis;
1818

19+
import com.alibaba.dubbo.common.Constants;
1920
import com.alibaba.dubbo.common.URL;
2021
import com.alibaba.dubbo.common.extension.ExtensionLoader;
22+
import com.alibaba.dubbo.common.serialize.ObjectInput;
23+
import com.alibaba.dubbo.common.serialize.Serialization;
2124
import com.alibaba.dubbo.common.utils.NetUtils;
2225
import com.alibaba.dubbo.rpc.Invoker;
2326
import com.alibaba.dubbo.rpc.Protocol;
2427
import com.alibaba.dubbo.rpc.ProxyFactory;
2528
import com.alibaba.dubbo.rpc.RpcException;
29+
30+
import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
2631
import org.junit.After;
32+
import org.junit.Assert;
2733
import org.junit.Before;
34+
import org.junit.Rule;
2835
import org.junit.Test;
36+
import org.junit.rules.TestName;
37+
import redis.clients.jedis.Jedis;
38+
import redis.clients.jedis.JedisPool;
39+
import redis.clients.jedis.exceptions.JedisConnectionException;
40+
import redis.clients.jedis.exceptions.JedisDataException;
2941
import redis.embedded.RedisServer;
3042

43+
import java.io.ByteArrayInputStream;
44+
3145
import static org.hamcrest.CoreMatchers.is;
3246
import static org.hamcrest.CoreMatchers.nullValue;
3347
import static org.junit.Assert.assertThat;
@@ -38,12 +52,21 @@ public class RedisProtocolTest {
3852
private RedisServer redisServer;
3953
private URL registryUrl;
4054

55+
@Rule
56+
public TestName name = new TestName();
57+
4158
@Before
4259
public void setUp() throws Exception {
4360
int redisPort = NetUtils.getAvailablePort();
44-
this.redisServer = new RedisServer(redisPort);
61+
if (name.getMethodName().equals("testAuthRedis") || name.getMethodName().equals("testWrongAuthRedis")) {
62+
String password = "123456";
63+
this.redisServer = RedisServer.builder().port(redisPort).setting("requirepass " + password).build();
64+
this.registryUrl = URL.valueOf("redis://username:"+password+"@localhost:"+redisPort+"?db.index=0");
65+
} else {
66+
this.redisServer = RedisServer.builder().port(redisPort).build();
67+
this.registryUrl = URL.valueOf("redis://localhost:" + redisPort);
68+
}
4569
this.redisServer.start();
46-
this.registryUrl = URL.valueOf("redis://localhost:" + redisPort);
4770
}
4871

4972
@After
@@ -109,4 +132,90 @@ public void testWrongRedis() {
109132
public void testExport() {
110133
protocol.export(protocol.refer(IDemoService.class, registryUrl));
111134
}
135+
136+
@Test
137+
public void testAuthRedis() {
138+
// default db.index=0
139+
Invoker<IDemoService> refer = protocol.refer(IDemoService.class,
140+
registryUrl
141+
.addParameter("max.idle", 10)
142+
.addParameter("max.active", 20));
143+
IDemoService demoService = this.proxy.getProxy(refer);
144+
145+
String value = demoService.get("key");
146+
assertThat(value, is(nullValue()));
147+
148+
demoService.set("key", "newValue");
149+
value = demoService.get("key");
150+
assertThat(value, is("newValue"));
151+
152+
demoService.delete("key");
153+
value = demoService.get("key");
154+
assertThat(value, is(nullValue()));
155+
156+
refer.destroy();
157+
158+
//change db.index=1
159+
String password = "123456";
160+
int database = 1;
161+
this.registryUrl = this.registryUrl.setPassword(password).addParameter("db.index", database);
162+
refer = protocol.refer(IDemoService.class,
163+
registryUrl
164+
.addParameter("max.idle", 10)
165+
.addParameter("max.active", 20));
166+
demoService = this.proxy.getProxy(refer);
167+
168+
demoService.set("key", "newValue");
169+
value = demoService.get("key");
170+
assertThat(value, is("newValue"));
171+
172+
// jedis gets the result comparison
173+
JedisPool pool = new JedisPool(new GenericObjectPoolConfig(), "localhost", registryUrl.getPort(), 2000, password, database, (String)null);
174+
Jedis jedis = null;
175+
try {
176+
jedis = pool.getResource();
177+
byte[] valueByte = jedis.get("key".getBytes());
178+
Serialization serialization = ExtensionLoader.getExtensionLoader(Serialization.class).getExtension(this.registryUrl.getParameter(Constants.SERIALIZATION_KEY, "java"));
179+
ObjectInput oin = serialization.deserialize(this.registryUrl, new ByteArrayInputStream(valueByte));
180+
String actual = (String) oin.readObject();
181+
assertThat(value, is(actual));
182+
} catch(Exception e) {
183+
Assert.fail("jedis gets the result comparison is error!");
184+
} finally {
185+
if (jedis != null) {
186+
jedis.close();
187+
}
188+
pool.destroy();
189+
}
190+
191+
demoService.delete("key");
192+
value = demoService.get("key");
193+
assertThat(value, is(nullValue()));
194+
195+
refer.destroy();
196+
}
197+
198+
@Test
199+
public void testWrongAuthRedis() {
200+
String password = "1234567";
201+
this.registryUrl = this.registryUrl.setPassword(password);
202+
Invoker<IDemoService> refer = protocol.refer(IDemoService.class,
203+
registryUrl
204+
.addParameter("max.idle", 10)
205+
.addParameter("max.active", 20));
206+
IDemoService demoService = this.proxy.getProxy(refer);
207+
208+
try {
209+
String value = demoService.get("key");
210+
assertThat(value, is(nullValue()));
211+
} catch (RpcException e) {
212+
if (e.getCause() instanceof JedisConnectionException && e.getCause().getCause() instanceof JedisDataException) {
213+
Assert.assertEquals("ERR invalid password" , e.getCause().getCause().getMessage());
214+
} else {
215+
Assert.fail("no invalid password exception!");
216+
}
217+
}
218+
219+
refer.destroy();
220+
}
112221
}

0 commit comments

Comments
 (0)