From 71158daeb98f31136393e53cfd20edfa17492c2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=87=9D=E9=9B=A8?= Date: Tue, 31 Jul 2018 13:19:58 +0800 Subject: [PATCH] Merge pull request #2018, fix redis auth problem for RedisProtocol. Fixes #2017 --- .../dubbo/registry/redis/RedisRegistry.java | 13 +- .../rpc/protocol/redis/RedisProtocol.java | 5 +- .../rpc/protocol/redis/RedisProtocolTest.java | 114 +++++++++++++++++- 3 files changed, 119 insertions(+), 13 deletions(-) diff --git a/dubbo-registry/dubbo-registry-redis/src/main/java/org/apache/dubbo/registry/redis/RedisRegistry.java b/dubbo-registry/dubbo-registry-redis/src/main/java/org/apache/dubbo/registry/redis/RedisRegistry.java index 3c396bfbafd..98b9de176f1 100644 --- a/dubbo-registry/dubbo-registry-redis/src/main/java/org/apache/dubbo/registry/redis/RedisRegistry.java +++ b/dubbo-registry/dubbo-registry-redis/src/main/java/org/apache/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/org/apache/dubbo/rpc/protocol/redis/RedisProtocol.java b/dubbo-rpc/dubbo-rpc-redis/src/main/java/org/apache/dubbo/rpc/protocol/redis/RedisProtocol.java index 5c12a1037f3..bd1ea8bcd21 100644 --- a/dubbo-rpc/dubbo-rpc-redis/src/main/java/org/apache/dubbo/rpc/protocol/redis/RedisProtocol.java +++ b/dubbo-rpc/dubbo-rpc-redis/src/main/java/org/apache/dubbo/rpc/protocol/redis/RedisProtocol.java @@ -22,6 +22,7 @@ import org.apache.dubbo.common.serialize.ObjectInput; import org.apache.dubbo.common.serialize.ObjectOutput; import org.apache.dubbo.common.serialize.Serialization; +import org.apache.dubbo.common.utils.StringUtils; import org.apache.dubbo.rpc.Exporter; import org.apache.dubbo.rpc.Invocation; import org.apache.dubbo.rpc.Invoker; @@ -90,7 +91,9 @@ public Invoker refer(final Class type, final URL url) throws RpcExcept 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/org/apache/dubbo/rpc/protocol/redis/RedisProtocolTest.java b/dubbo-rpc/dubbo-rpc-redis/src/test/java/org/apache/dubbo/rpc/protocol/redis/RedisProtocolTest.java index 0ef53dda140..8c69c38767c 100644 --- a/dubbo-rpc/dubbo-rpc-redis/src/test/java/org/apache/dubbo/rpc/protocol/redis/RedisProtocolTest.java +++ b/dubbo-rpc/dubbo-rpc-redis/src/test/java/org/apache/dubbo/rpc/protocol/redis/RedisProtocolTest.java @@ -16,18 +16,33 @@ */ package org.apache.dubbo.rpc.protocol.redis; +import org.apache.commons.pool2.impl.GenericObjectPoolConfig; +import org.apache.dubbo.common.Constants; import org.apache.dubbo.common.URL; import org.apache.dubbo.common.extension.ExtensionLoader; +import org.apache.dubbo.common.serialize.ObjectInput; +import org.apache.dubbo.common.serialize.Serialization; import org.apache.dubbo.common.utils.NetUtils; import org.apache.dubbo.rpc.Invoker; import org.apache.dubbo.rpc.Protocol; import org.apache.dubbo.rpc.ProxyFactory; import org.apache.dubbo.rpc.RpcException; +import org.apache.dubbo.rpc.RpcResult; 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 java.io.IOException; + import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.nullValue; import static org.junit.Assert.assertThat; @@ -38,12 +53,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 +133,90 @@ public void testWrongRedis() { public void testExport() { protocol.export(protocol.refer(IDemoService.class, registryUrl)); } + + @Test + public void testAuthRedis() { + // default db.index=0 + Invoker 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 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