From e1965fae28116c87c62685044c29745ded0a9f0c Mon Sep 17 00:00:00 2001 From: Mark Zhang Date: Fri, 12 Jan 2024 20:50:53 +0800 Subject: [PATCH] fix: jedis wrapper call NPE (#375) --- .../java/io/arex/attacher/AgentAttacher.java | 6 +- .../matcher/SafeExtendsClassMatcherTest.java | 30 ++++++++++ .../context/RepeatedCollectManagerTest.java | 34 +++++++++++ .../io/arex/foundation/util/NetUtilsTest.java | 45 ++++++++++++++ .../io/arex/inst/jedis/v2/JedisWrapper.java | 5 +- .../arex/inst/jedis/v2/JedisWrapperTest.java | 30 +++++++++- .../io/arex/inst/jedis/v4/JedisWrapper.java | 5 +- .../arex/inst/jedis/v4/JedisWrapperTest.java | 33 +++++++++- .../inst/redis/common/RedisKeyUtilTest.java | 60 +++++++++++++++++++ .../inst/time/TimeMachineInterceptorTest.java | 41 +++++++++++++ pom.xml | 9 ++- 11 files changed, 287 insertions(+), 11 deletions(-) create mode 100644 arex-instrumentation-api/src/test/java/io/arex/inst/extension/matcher/SafeExtendsClassMatcherTest.java create mode 100644 arex-instrumentation-api/src/test/java/io/arex/inst/runtime/context/RepeatedCollectManagerTest.java create mode 100644 arex-instrumentation-foundation/src/test/java/io/arex/foundation/util/NetUtilsTest.java create mode 100644 arex-instrumentation/redis/arex-redis-common/src/test/java/io/arex/inst/redis/common/RedisKeyUtilTest.java create mode 100644 arex-instrumentation/time-machine/arex-time-machine/src/test/java/io/arex/inst/time/TimeMachineInterceptorTest.java diff --git a/arex-attacher/src/main/java/io/arex/attacher/AgentAttacher.java b/arex-attacher/src/main/java/io/arex/attacher/AgentAttacher.java index cfc0b1232..fc0687ec3 100644 --- a/arex-attacher/src/main/java/io/arex/attacher/AgentAttacher.java +++ b/arex-attacher/src/main/java/io/arex/attacher/AgentAttacher.java @@ -4,6 +4,7 @@ import com.sun.tools.attach.VirtualMachine; import java.io.IOException; +import java.util.Arrays; /** * agent attacher @@ -52,7 +53,8 @@ public static void main(String[] args) { virtualMachine.detach(); } catch (Throwable e) { // expected behavior, it will be returned as error stream to the caller, if any - e.printStackTrace(); + System.err.printf("agent attach failed, pid: %s, args: %s, error: %s%n", + pid, Arrays.toString(args), e.getMessage()); } } -} \ No newline at end of file +} diff --git a/arex-instrumentation-api/src/test/java/io/arex/inst/extension/matcher/SafeExtendsClassMatcherTest.java b/arex-instrumentation-api/src/test/java/io/arex/inst/extension/matcher/SafeExtendsClassMatcherTest.java new file mode 100644 index 000000000..e4403d781 --- /dev/null +++ b/arex-instrumentation-api/src/test/java/io/arex/inst/extension/matcher/SafeExtendsClassMatcherTest.java @@ -0,0 +1,30 @@ +package io.arex.inst.extension.matcher; + +import static org.junit.jupiter.api.Assertions.*; + +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; +import net.bytebuddy.matcher.ElementMatchers; +import org.junit.jupiter.api.Test; + +/** + * @since 2024/1/12 + */ +class SafeExtendsClassMatcherTest { + + @Test + void extendsClass() { + ElementMatcher.Junction matcher = SafeExtendsClassMatcher.extendsClass( + ElementMatchers.is(SafeExtendsClassMatcher.class)); + + assertTrue(matcher.matches(TypeDescription.ForLoadedType.of(SafeExtendsClassMatcher.class))); + } + + @Test + void testExtendsClass() { + ElementMatcher.Junction matcher = SafeExtendsClassMatcher.extendsClass( + ElementMatchers.is(ElementMatcher.Junction.AbstractBase.class), true); + + assertTrue(matcher.matches(TypeDescription.ForLoadedType.of(SafeExtendsClassMatcher.class))); + } +} diff --git a/arex-instrumentation-api/src/test/java/io/arex/inst/runtime/context/RepeatedCollectManagerTest.java b/arex-instrumentation-api/src/test/java/io/arex/inst/runtime/context/RepeatedCollectManagerTest.java new file mode 100644 index 000000000..66533c83f --- /dev/null +++ b/arex-instrumentation-api/src/test/java/io/arex/inst/runtime/context/RepeatedCollectManagerTest.java @@ -0,0 +1,34 @@ +package io.arex.inst.runtime.context; + +import static org.junit.jupiter.api.Assertions.*; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +/** + * @since 2024/1/12 + */ +class RepeatedCollectManagerTest { + @BeforeAll + static void setUp() { + Mockito.mockStatic(ContextManager.class); + } + @AfterAll + static void tearDown() { + Mockito.clearAllCaches(); + } + + @Test + void test() { + assertTrue(RepeatedCollectManager.validate()); + assertTrue(RepeatedCollectManager.exitAndValidate()); + + Mockito.when(ContextManager.needRecord()).thenReturn(true); + RepeatedCollectManager.enter(); + RepeatedCollectManager.enter(); + assertFalse(RepeatedCollectManager.exitAndValidate()); + assertTrue(RepeatedCollectManager.exitAndValidate()); + } +} diff --git a/arex-instrumentation-foundation/src/test/java/io/arex/foundation/util/NetUtilsTest.java b/arex-instrumentation-foundation/src/test/java/io/arex/foundation/util/NetUtilsTest.java new file mode 100644 index 000000000..f98468559 --- /dev/null +++ b/arex-instrumentation-foundation/src/test/java/io/arex/foundation/util/NetUtilsTest.java @@ -0,0 +1,45 @@ +package io.arex.foundation.util; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.mockStatic; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; + +/** + * @since 2024/1/12 + */ +class NetUtilsTest { + + @BeforeEach + void setUp() { + } + + @AfterEach + void tearDown() { + } + + @Test + void getIpAddress() { + try (MockedStatic netUtils = mockStatic(NetUtils.class)) { + netUtils.when(NetUtils::getIpAddress).thenReturn("127.0.0.1"); + assertEquals("127.0.0.1", NetUtils.getIpAddress()); + } + } + + @Test + void checkTcpPortAvailable() { + assertEquals(-1, NetUtils.checkTcpPortAvailable(1)); + + assertEquals(8080, NetUtils.checkTcpPortAvailable(8080)); + } + + @Test + void isTcpPortAvailable() { + assertFalse(NetUtils.isTcpPortAvailable(1)); + + assertTrue(NetUtils.isTcpPortAvailable(8080)); + } +} diff --git a/arex-instrumentation/redis/arex-jedis-v2/src/main/java/io/arex/inst/jedis/v2/JedisWrapper.java b/arex-instrumentation/redis/arex-jedis-v2/src/main/java/io/arex/inst/jedis/v2/JedisWrapper.java index 2aee9bec0..9a0dc4bc6 100644 --- a/arex-instrumentation/redis/arex-jedis-v2/src/main/java/io/arex/inst/jedis/v2/JedisWrapper.java +++ b/arex-instrumentation/redis/arex-jedis-v2/src/main/java/io/arex/inst/jedis/v2/JedisWrapper.java @@ -1,6 +1,7 @@ package io.arex.inst.jedis.v2; import io.arex.agent.bootstrap.model.MockResult; +import io.arex.agent.bootstrap.util.ArrayUtils; import io.arex.inst.runtime.context.ContextManager; import io.arex.inst.runtime.serializer.Serializer; import io.arex.inst.redis.common.RedisExtractor; @@ -527,8 +528,8 @@ public String ping(String message) { * mset/msetnx */ private U call(String command, String[] keysValues, Callable callable, U defaultValue) { - if (keysValues == null || keysValues.length == 0) { - return null; + if (ArrayUtils.isEmpty(keysValues)) { + return defaultValue; } if (keysValues.length == 2) { diff --git a/arex-instrumentation/redis/arex-jedis-v2/src/test/java/io/arex/inst/jedis/v2/JedisWrapperTest.java b/arex-instrumentation/redis/arex-jedis-v2/src/test/java/io/arex/inst/jedis/v2/JedisWrapperTest.java index 3ac648147..c1255b09e 100644 --- a/arex-instrumentation/redis/arex-jedis-v2/src/test/java/io/arex/inst/jedis/v2/JedisWrapperTest.java +++ b/arex-instrumentation/redis/arex-jedis-v2/src/test/java/io/arex/inst/jedis/v2/JedisWrapperTest.java @@ -5,6 +5,7 @@ import io.arex.inst.runtime.context.ContextManager; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -52,6 +53,33 @@ static void tearDown() { Mockito.clearAllCaches(); } + @Test + void callWithEmptyKeysValuesReturnsDefault() { + long result = target.msetnx( new String[]{}); + assertEquals(0, result); + } + + @Test + void callWithTwoKeysValuesReturnsCallableResult() { + Mockito.when(ContextManager.needRecord()).thenReturn(false); + Mockito.when(client.getIntegerReply()).thenReturn(1L); + try (MockedConstruction mocked = Mockito.mockConstruction(RedisExtractor.class, (mock, context) -> { + })) { + long result = target.msetnx("key", "value"); + assertEquals(1L, result); + + result = target.msetnx("key1", "value1", "key2", "value2", "key3", "value3"); + assertEquals(1L, result); + + result = target.exists("key1", "key2", "key3"); + assertEquals(1L, result); + } catch (Exception e) { + assertThrows(NullPointerException.class, () -> { + throw e; + }); + } + } + @ParameterizedTest @MethodSource("callCase") void call(Runnable mocker, Predicate predicate) { @@ -89,4 +117,4 @@ static Stream callCase() { arguments(mocker3, predicate2) ); } -} \ No newline at end of file +} diff --git a/arex-instrumentation/redis/arex-jedis-v4/src/main/java/io/arex/inst/jedis/v4/JedisWrapper.java b/arex-instrumentation/redis/arex-jedis-v4/src/main/java/io/arex/inst/jedis/v4/JedisWrapper.java index 12a5033ad..9c7a4afd3 100644 --- a/arex-instrumentation/redis/arex-jedis-v4/src/main/java/io/arex/inst/jedis/v4/JedisWrapper.java +++ b/arex-instrumentation/redis/arex-jedis-v4/src/main/java/io/arex/inst/jedis/v4/JedisWrapper.java @@ -1,5 +1,6 @@ package io.arex.inst.jedis.v4; +import io.arex.agent.bootstrap.util.ArrayUtils; import io.arex.inst.runtime.context.ContextManager; import io.arex.inst.runtime.serializer.Serializer; import io.arex.agent.bootstrap.model.MockResult; @@ -572,8 +573,8 @@ public String ping(String message) { * mset/msetnx */ private U call(String command, String[] keysValues, Callable callable, U defaultValue) { - if (keysValues == null || keysValues.length == 0) { - return null; + if (ArrayUtils.isEmpty(keysValues)) { + return defaultValue; } if (keysValues.length == 2) { diff --git a/arex-instrumentation/redis/arex-jedis-v4/src/test/java/io/arex/inst/jedis/v4/JedisWrapperTest.java b/arex-instrumentation/redis/arex-jedis-v4/src/test/java/io/arex/inst/jedis/v4/JedisWrapperTest.java index af8520917..e6ffb3544 100644 --- a/arex-instrumentation/redis/arex-jedis-v4/src/test/java/io/arex/inst/jedis/v4/JedisWrapperTest.java +++ b/arex-instrumentation/redis/arex-jedis-v4/src/test/java/io/arex/inst/jedis/v4/JedisWrapperTest.java @@ -5,8 +5,8 @@ import io.arex.inst.runtime.context.ContextManager; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import org.junit.jupiter.api.function.Executable; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -18,8 +18,8 @@ import java.util.function.Predicate; import java.util.stream.Stream; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertThrowsExactly; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.params.provider.Arguments.arguments; import static org.mockito.ArgumentMatchers.any; @@ -48,6 +48,33 @@ static void tearDown() { Mockito.clearAllCaches(); } + @Test + void callWithEmptyKeysValuesReturnsDefault() { + long result = target.msetnx( new String[]{}); + assertEquals(0, result); + } + + @Test + void callWithTwoKeysValuesReturnsCallableResult() { + Mockito.when(ContextManager.needRecord()).thenReturn(false); + Mockito.when(connection.executeCommand(any(CommandObject.class))).thenReturn(1L); + try (MockedConstruction mocked = Mockito.mockConstruction(RedisExtractor.class, (mock, context) -> { + })) { + long result = target.msetnx("key", "value"); + assertEquals(1L, result); + + result = target.msetnx("key1", "value1", "key2", "value2", "key3", "value3"); + assertEquals(1L, result); + + result = target.exists("key1", "key2", "key3"); + assertEquals(1L, result); + } catch (Exception e) { + assertThrows(NullPointerException.class, () -> { + throw e; + }); + } + } + @ParameterizedTest @MethodSource("callCase") void call(Runnable mocker, Predicate predicate) { @@ -85,4 +112,4 @@ static Stream callCase() { arguments(mocker3, predicate2) ); } -} \ No newline at end of file +} diff --git a/arex-instrumentation/redis/arex-redis-common/src/test/java/io/arex/inst/redis/common/RedisKeyUtilTest.java b/arex-instrumentation/redis/arex-redis-common/src/test/java/io/arex/inst/redis/common/RedisKeyUtilTest.java new file mode 100644 index 000000000..8df510330 --- /dev/null +++ b/arex-instrumentation/redis/arex-redis-common/src/test/java/io/arex/inst/redis/common/RedisKeyUtilTest.java @@ -0,0 +1,60 @@ +package io.arex.inst.redis.common; + +import static org.junit.jupiter.api.Assertions.*; + +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; +import org.junit.jupiter.api.Test; + +/** + * @since 2024/1/12 + */ +class RedisKeyUtilTest { + + @Test + void generateWithIterableKeys() { + String result = RedisKeyUtil.generate(Arrays.asList("key1", "key2", "key3")); + assertEquals("key1;key2;key3", result); + } + + @Test + void generateWithMapKeys() { + Map map = new HashMap<>(); + map.put("key1", "value1"); + map.put("key2", "value2"); + String result = RedisKeyUtil.generate(map); + assertTrue(result.contains("key1")); + assertTrue(result.contains("key2")); + } + + @Test + void generateWithVarargsKeys() { + String result = RedisKeyUtil.generate("key1", "key2", "key3"); + assertEquals("key1;key2;key3", result); + } + + @Test + void generateWithEmptyVarargsKeys() { + String result = RedisKeyUtil.generate(); + assertEquals("", result); + } + + @Test + void generateWithSingleVarargsKey() { + String result = RedisKeyUtil.generate("key1"); + assertEquals("key1", result); + } + + @Test + void generateWithByteValue() { + String result = RedisKeyUtil.generate(new byte[]{'k', 'e', 'y'}); + assertEquals("key", result); + } + + @Test + void generateWithCharValue() { + String result = RedisKeyUtil.generate(new char[]{'k', 'e', 'y'}); + assertEquals("key", result); + } +} diff --git a/arex-instrumentation/time-machine/arex-time-machine/src/test/java/io/arex/inst/time/TimeMachineInterceptorTest.java b/arex-instrumentation/time-machine/arex-time-machine/src/test/java/io/arex/inst/time/TimeMachineInterceptorTest.java new file mode 100644 index 000000000..6e9687aee --- /dev/null +++ b/arex-instrumentation/time-machine/arex-time-machine/src/test/java/io/arex/inst/time/TimeMachineInterceptorTest.java @@ -0,0 +1,41 @@ +package io.arex.inst.time; + +import static org.junit.jupiter.api.Assertions.*; + +import io.arex.agent.bootstrap.TraceContextManager; +import io.arex.agent.bootstrap.cache.TimeCache; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +/** + * @since 2024/1/12 + */ +class TimeMachineInterceptorTest { + @BeforeAll + static void setUp() { + Mockito.mockStatic(TraceContextManager.class); + } + + @AfterAll + static void tearDown() { + Mockito.clearAllCaches(); + } + + @Test + void onEnter() { + // traceId is not null + Mockito.when(TraceContextManager.get()).thenReturn("test"); + assertEquals(0, TimeCache.get()); + TimeCache.put(1L); + assertNotEquals(0, TimeMachineInterceptor.onEnter()); + } + + @Test + void onExit() { + long result = 456L; + TimeMachineInterceptor.onExit(123L, result); + assertEquals(456L, result); + } +} diff --git a/pom.xml b/pom.xml index 7fb94409c..b51b9c4a3 100644 --- a/pom.xml +++ b/pom.xml @@ -77,7 +77,9 @@ **/*Test.java, **/target/**, **/model/**, + **/constants/**, **/thirdparty/**, + **/integrationtest/**, **/*Instrumentation.java, **/JJWTGenerator.java @@ -88,12 +90,17 @@ **/test/**, **/*Test.java, + **/*Transformer.java, **/target/**, **/model/**, + **/constants/**, **/context/**, + **/bootstrap/internal/**, **/wrapper/**, **/thirdparty/**, - **/RedisCommandBuilderImpl.java + **/integrationtest/**, + **/RedisCommandBuilderImpl.java, + **/HttpResponseWrapper.java