You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dubbo.apache.org by li...@apache.org on 2018/07/31 05:20:31 UTC

[incubator-dubbo] branch 2.6.x updated: Merge pull request #2146, fix redis auth problem for RedisProtocol.

This is an automated email from the ASF dual-hosted git repository.

liujun pushed a commit to branch 2.6.x
in repository https://gitbox.apache.org/repos/asf/incubator-dubbo.git


The following commit(s) were added to refs/heads/2.6.x by this push:
     new 79859aa  Merge pull request #2146, fix redis auth problem for RedisProtocol.
79859aa is described below

commit 79859aa20ce0c29bcb21d6d567a465db8f33ba2b
Author: 凝雨 <ni...@163.com>
AuthorDate: Tue Jul 31 13:20:29 2018 +0800

    Merge pull request #2146, fix redis auth problem for RedisProtocol.
    
    Fixes #2017
---
 .../dubbo/registry/redis/RedisRegistry.java        |  13 +--
 .../dubbo/rpc/protocol/redis/RedisProtocol.java    |   5 +-
 .../rpc/protocol/redis/RedisProtocolTest.java      | 113 ++++++++++++++++++++-
 3 files changed, 118 insertions(+), 13 deletions(-)

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 3c19f86..05f0102 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 class RedisRegistry extends FailbackRegistry {
             addresses.addAll(Arrays.asList(backups));
         }
 
-        String password = url.getPassword();
         for (String address : addresses) {
             int i = address.indexOf(':');
             String host;
@@ -132,15 +131,9 @@ public class RedisRegistry extends FailbackRegistry {
                 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 dba2ac1..ae856e5 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.extension.ExtensionLoader;
 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 @@ public class RedisProtocol extends AbstractProtocol {
             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 3128478..4bf78fb 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 @@ public class RedisProtocolTest {
     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 class RedisProtocolTest {
     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