You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by GitBox <gi...@apache.org> on 2018/07/31 05:20:31 UTC
[GitHub] chickenlj closed pull request #2146: [Dubbo-2017]Fix redis auth
problem for RedisProtocol(merge to 2.6.x PR)
chickenlj closed pull request #2146: [Dubbo-2017]Fix redis auth problem for RedisProtocol(merge to 2.6.x PR)
URL: https://github.com/apache/incubator-dubbo/pull/2146
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/dubbo-registry/dubbo-registry-redis/src/main/java/com/alibaba/dubbo/registry/redis/RedisRegistry.java b/dubbo-registry/dubbo-registry-redis/src/main/java/com/alibaba/dubbo/registry/redis/RedisRegistry.java
index 3c19f86899..05f010255b 100644
--- a/dubbo-registry/dubbo-registry-redis/src/main/java/com/alibaba/dubbo/registry/redis/RedisRegistry.java
+++ b/dubbo-registry/dubbo-registry-redis/src/main/java/com/alibaba/dubbo/registry/redis/RedisRegistry.java
@@ -120,7 +120,6 @@ public RedisRegistry(URL url) {
addresses.addAll(Arrays.asList(backups));
}
- String password = url.getPassword();
for (String address : addresses) {
int i = address.indexOf(':');
String host;
@@ -132,15 +131,9 @@ public RedisRegistry(URL url) {
host = address;
port = DEFAULT_REDIS_PORT;
}
- if (StringUtils.isEmpty(password)) {
- this.jedisPools.put(address, new JedisPool(config, host, port,
- url.getParameter(Constants.TIMEOUT_KEY, Constants.DEFAULT_TIMEOUT), null,
- url.getParameter("db.index", 0)));
- } else {
- this.jedisPools.put(address, new JedisPool(config, host, port,
- url.getParameter(Constants.TIMEOUT_KEY, Constants.DEFAULT_TIMEOUT), password,
- url.getParameter("db.index", 0)));
- }
+ this.jedisPools.put(address, new JedisPool(config, host, port,
+ url.getParameter(Constants.TIMEOUT_KEY, Constants.DEFAULT_TIMEOUT), StringUtils.isEmpty(url.getPassword()) ? null : url.getPassword(),
+ url.getParameter("db.index", 0)));
}
this.reconnectPeriod = url.getParameter(Constants.REGISTRY_RECONNECT_PERIOD_KEY, Constants.DEFAULT_REGISTRY_RECONNECT_PERIOD);
diff --git a/dubbo-rpc/dubbo-rpc-redis/src/main/java/com/alibaba/dubbo/rpc/protocol/redis/RedisProtocol.java b/dubbo-rpc/dubbo-rpc-redis/src/main/java/com/alibaba/dubbo/rpc/protocol/redis/RedisProtocol.java
index dba2ac1b0e..ae856e5854 100644
--- a/dubbo-rpc/dubbo-rpc-redis/src/main/java/com/alibaba/dubbo/rpc/protocol/redis/RedisProtocol.java
+++ b/dubbo-rpc/dubbo-rpc-redis/src/main/java/com/alibaba/dubbo/rpc/protocol/redis/RedisProtocol.java
@@ -22,6 +22,7 @@
import com.alibaba.dubbo.common.serialize.ObjectInput;
import com.alibaba.dubbo.common.serialize.ObjectOutput;
import com.alibaba.dubbo.common.serialize.Serialization;
+import com.alibaba.dubbo.common.utils.StringUtils;
import com.alibaba.dubbo.rpc.Exporter;
import com.alibaba.dubbo.rpc.Invocation;
import com.alibaba.dubbo.rpc.Invoker;
@@ -90,7 +91,9 @@ private Serialization getSerialization(URL url) {
if (url.getParameter("min.evictable.idle.time.millis", 0) > 0)
config.setMinEvictableIdleTimeMillis(url.getParameter("min.evictable.idle.time.millis", 0));
final JedisPool jedisPool = new JedisPool(config, url.getHost(), url.getPort(DEFAULT_PORT),
- url.getParameter(Constants.TIMEOUT_KEY, Constants.DEFAULT_TIMEOUT));
+ url.getParameter(Constants.TIMEOUT_KEY, Constants.DEFAULT_TIMEOUT),
+ StringUtils.isBlank(url.getPassword()) ? null : url.getPassword(),
+ url.getParameter("db.index", 0));
final int expiry = url.getParameter("expiry", 0);
final String get = url.getParameter("get", "get");
final String set = url.getParameter("set", Map.class.equals(type) ? "put" : "set");
diff --git a/dubbo-rpc/dubbo-rpc-redis/src/test/java/com/alibaba/dubbo/rpc/protocol/redis/RedisProtocolTest.java b/dubbo-rpc/dubbo-rpc-redis/src/test/java/com/alibaba/dubbo/rpc/protocol/redis/RedisProtocolTest.java
index 312847841b..4bf78fb0e0 100644
--- a/dubbo-rpc/dubbo-rpc-redis/src/test/java/com/alibaba/dubbo/rpc/protocol/redis/RedisProtocolTest.java
+++ b/dubbo-rpc/dubbo-rpc-redis/src/test/java/com/alibaba/dubbo/rpc/protocol/redis/RedisProtocolTest.java
@@ -16,18 +16,32 @@
*/
package com.alibaba.dubbo.rpc.protocol.redis;
+import com.alibaba.dubbo.common.Constants;
import com.alibaba.dubbo.common.URL;
import com.alibaba.dubbo.common.extension.ExtensionLoader;
+import com.alibaba.dubbo.common.serialize.ObjectInput;
+import com.alibaba.dubbo.common.serialize.Serialization;
import com.alibaba.dubbo.common.utils.NetUtils;
import com.alibaba.dubbo.rpc.Invoker;
import com.alibaba.dubbo.rpc.Protocol;
import com.alibaba.dubbo.rpc.ProxyFactory;
import com.alibaba.dubbo.rpc.RpcException;
+
+import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
import org.junit.After;
+import org.junit.Assert;
import org.junit.Before;
+import org.junit.Rule;
import org.junit.Test;
+import org.junit.rules.TestName;
+import redis.clients.jedis.Jedis;
+import redis.clients.jedis.JedisPool;
+import redis.clients.jedis.exceptions.JedisConnectionException;
+import redis.clients.jedis.exceptions.JedisDataException;
import redis.embedded.RedisServer;
+import java.io.ByteArrayInputStream;
+
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.junit.Assert.assertThat;
@@ -38,12 +52,21 @@
private RedisServer redisServer;
private URL registryUrl;
+ @Rule
+ public TestName name = new TestName();
+
@Before
public void setUp() throws Exception {
int redisPort = NetUtils.getAvailablePort();
- this.redisServer = new RedisServer(redisPort);
+ if (name.getMethodName().equals("testAuthRedis") || name.getMethodName().equals("testWrongAuthRedis")) {
+ String password = "123456";
+ this.redisServer = RedisServer.builder().port(redisPort).setting("requirepass " + password).build();
+ this.registryUrl = URL.valueOf("redis://username:"+password+"@localhost:"+redisPort+"?db.index=0");
+ } else {
+ this.redisServer = RedisServer.builder().port(redisPort).build();
+ this.registryUrl = URL.valueOf("redis://localhost:" + redisPort);
+ }
this.redisServer.start();
- this.registryUrl = URL.valueOf("redis://localhost:" + redisPort);
}
@After
@@ -109,4 +132,90 @@ public void testWrongRedis() {
public void testExport() {
protocol.export(protocol.refer(IDemoService.class, registryUrl));
}
+
+ @Test
+ public void testAuthRedis() {
+ // default db.index=0
+ Invoker<IDemoService> refer = protocol.refer(IDemoService.class,
+ registryUrl
+ .addParameter("max.idle", 10)
+ .addParameter("max.active", 20));
+ IDemoService demoService = this.proxy.getProxy(refer);
+
+ String value = demoService.get("key");
+ assertThat(value, is(nullValue()));
+
+ demoService.set("key", "newValue");
+ value = demoService.get("key");
+ assertThat(value, is("newValue"));
+
+ demoService.delete("key");
+ value = demoService.get("key");
+ assertThat(value, is(nullValue()));
+
+ refer.destroy();
+
+ //change db.index=1
+ String password = "123456";
+ int database = 1;
+ this.registryUrl = this.registryUrl.setPassword(password).addParameter("db.index", database);
+ refer = protocol.refer(IDemoService.class,
+ registryUrl
+ .addParameter("max.idle", 10)
+ .addParameter("max.active", 20));
+ demoService = this.proxy.getProxy(refer);
+
+ demoService.set("key", "newValue");
+ value = demoService.get("key");
+ assertThat(value, is("newValue"));
+
+ // jedis gets the result comparison
+ JedisPool pool = new JedisPool(new GenericObjectPoolConfig(), "localhost", registryUrl.getPort(), 2000, password, database, (String)null);
+ Jedis jedis = null;
+ try {
+ jedis = pool.getResource();
+ byte[] valueByte = jedis.get("key".getBytes());
+ Serialization serialization = ExtensionLoader.getExtensionLoader(Serialization.class).getExtension(this.registryUrl.getParameter(Constants.SERIALIZATION_KEY, "java"));
+ ObjectInput oin = serialization.deserialize(this.registryUrl, new ByteArrayInputStream(valueByte));
+ String actual = (String) oin.readObject();
+ assertThat(value, is(actual));
+ } catch(Exception e) {
+ Assert.fail("jedis gets the result comparison is error!");
+ } finally {
+ if (jedis != null) {
+ jedis.close();
+ }
+ pool.destroy();
+ }
+
+ demoService.delete("key");
+ value = demoService.get("key");
+ assertThat(value, is(nullValue()));
+
+ refer.destroy();
+ }
+
+ @Test
+ public void testWrongAuthRedis() {
+ String password = "1234567";
+ this.registryUrl = this.registryUrl.setPassword(password);
+ Invoker<IDemoService> refer = protocol.refer(IDemoService.class,
+ registryUrl
+ .addParameter("max.idle", 10)
+ .addParameter("max.active", 20));
+ IDemoService demoService = this.proxy.getProxy(refer);
+
+ try {
+ String value = demoService.get("key");
+ assertThat(value, is(nullValue()));
+ } catch (RpcException e) {
+ if (e.getCause() instanceof JedisConnectionException && e.getCause().getCause() instanceof JedisDataException) {
+ Assert.assertEquals("ERR invalid password" , e.getCause().getCause().getMessage());
+ } else {
+ Assert.fail("no invalid password exception!");
+ }
+ }
+
+ refer.destroy();
+ }
}
\ No newline at end of file
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org