From 9c8c6bdcb8b32b81cbc93c8bc5efff39d2cf60b7 Mon Sep 17 00:00:00 2001 From: Woonduk Kang Date: Fri, 17 Jul 2015 14:34:40 +0900 Subject: [PATCH] #737 bindValue bug fix --- .../jdbc/common/bindvalue/BindValueUtils.java | 42 +++++++++++++- ...paredStatementExecuteQueryInterceptor.java | 12 +--- .../db/interceptor/BindValueUtils.java | 41 ++++++++++++- ...paredStatementExecuteQueryInterceptor.java | 12 +--- .../db/interceptor/BindValueUtilsTest.java | 58 ++++++++++++++++++- 5 files changed, 136 insertions(+), 29 deletions(-) diff --git a/plugins/jdbc-driver/src/main/java/com/navercorp/pinpoint/plugin/jdbc/common/bindvalue/BindValueUtils.java b/plugins/jdbc-driver/src/main/java/com/navercorp/pinpoint/plugin/jdbc/common/bindvalue/BindValueUtils.java index 5cc7ed292..72ccd7954 100644 --- a/plugins/jdbc-driver/src/main/java/com/navercorp/pinpoint/plugin/jdbc/common/bindvalue/BindValueUtils.java +++ b/plugins/jdbc-driver/src/main/java/com/navercorp/pinpoint/plugin/jdbc/common/bindvalue/BindValueUtils.java @@ -18,14 +18,51 @@ package com.navercorp.pinpoint.plugin.jdbc.common.bindvalue; import com.navercorp.pinpoint.bootstrap.util.StringUtils; +import java.util.Map; + /** + * duplicate : com.navercorp.pinpoint.profiler.modifier.db.interceptor.BindValueUtils * @author emeroad */ -public class BindValueUtils { +public final class BindValueUtils { private BindValueUtils() { } + public static String bindValueToString(final Map bindValueMap, int limit) { + if (bindValueMap == null) { + return ""; + } + if (bindValueMap.isEmpty()) { + return ""; + } + final int maxParameterIndex = getMaxParameterIndex(bindValueMap); + if (maxParameterIndex <= 0) { + return ""; + } + final String[] temp = new String[maxParameterIndex]; + for (Map.Entry entry : bindValueMap.entrySet()) { + final int parameterIndex = entry.getKey() - 1; + if (parameterIndex < 0) { + // invalid index. PreparedStatement first parameterIndex is 1 + continue; + } + if (temp.length <= parameterIndex) { + continue; + } + temp[parameterIndex] = entry.getValue(); + } + return bindValueToString(temp, limit); + } + + private static int getMaxParameterIndex(Map bindValueMap) { + int maxIndex = 0; + for (Integer idx : bindValueMap.keySet()) { + maxIndex = Math.max(maxIndex, idx); + } + return maxIndex; + } + public static String bindValueToString(String[] bindValueArray, int limit) { if (bindValueArray == null) { return ""; @@ -39,7 +76,8 @@ public class BindValueUtils { appendLength(sb, length); break; } - StringUtils.appendDrop(sb, bindValueArray[i], limit); + final String bindValue = StringUtils.defaultString(bindValueArray[i], ""); + StringUtils.appendDrop(sb, bindValue, limit); if (i < end) { sb.append(", "); } diff --git a/plugins/jdbc-driver/src/main/java/com/navercorp/pinpoint/plugin/jdbc/common/interceptor/PreparedStatementExecuteQueryInterceptor.java b/plugins/jdbc-driver/src/main/java/com/navercorp/pinpoint/plugin/jdbc/common/interceptor/PreparedStatementExecuteQueryInterceptor.java index 815734ba5..ee7c6bcb7 100644 --- a/plugins/jdbc-driver/src/main/java/com/navercorp/pinpoint/plugin/jdbc/common/interceptor/PreparedStatementExecuteQueryInterceptor.java +++ b/plugins/jdbc-driver/src/main/java/com/navercorp/pinpoint/plugin/jdbc/common/interceptor/PreparedStatementExecuteQueryInterceptor.java @@ -130,17 +130,7 @@ public class PreparedStatementExecuteQueryInterceptor implements SimpleAroundInt } private String toBindVariable(Map bindValue) { - final String[] temp = new String[bindValue.size()]; - for (Map.Entry entry : bindValue.entrySet()) { - Integer key = entry.getKey() - 1; - if (temp.length <= key) { - continue; - } - temp[key] = entry.getValue(); - } - - return BindValueUtils.bindValueToString(temp, maxSqlBindValueLength); - + return BindValueUtils.bindValueToString(bindValue, maxSqlBindValueLength); } @Override diff --git a/profiler/src/main/java/com/navercorp/pinpoint/profiler/modifier/db/interceptor/BindValueUtils.java b/profiler/src/main/java/com/navercorp/pinpoint/profiler/modifier/db/interceptor/BindValueUtils.java index 815f613b2..50c78810e 100644 --- a/profiler/src/main/java/com/navercorp/pinpoint/profiler/modifier/db/interceptor/BindValueUtils.java +++ b/profiler/src/main/java/com/navercorp/pinpoint/profiler/modifier/db/interceptor/BindValueUtils.java @@ -18,14 +18,50 @@ package com.navercorp.pinpoint.profiler.modifier.db.interceptor; import com.navercorp.pinpoint.bootstrap.util.StringUtils; +import java.util.Map; + /** * @author emeroad */ -public class BindValueUtils { +public final class BindValueUtils { private BindValueUtils() { } + public static String bindValueToString(final Map bindValueMap, int limit) { + if (bindValueMap == null) { + return ""; + } + if (bindValueMap.isEmpty()) { + return ""; + } + final int maxParameterIndex = getMaxParameterIndex(bindValueMap); + if (maxParameterIndex <= 0) { + return ""; + } + final String[] temp = new String[maxParameterIndex]; + for (Map.Entry entry : bindValueMap.entrySet()) { + final int parameterIndex = entry.getKey() - 1; + if (parameterIndex < 0) { + // invalid index. PreparedStatement first parameterIndex is 1 + continue; + } + if (temp.length <= parameterIndex) { + continue; + } + temp[parameterIndex] = entry.getValue(); + } + return bindValueToString(temp, limit); + } + + private static int getMaxParameterIndex(Map bindValueMap) { + int maxIndex = 0; + for (Integer idx : bindValueMap.keySet()) { + maxIndex = Math.max(maxIndex, idx); + } + return maxIndex; + } + public static String bindValueToString(String[] bindValueArray, int limit) { if (bindValueArray == null) { return ""; @@ -39,7 +75,8 @@ public class BindValueUtils { appendLength(sb, length); break; } - StringUtils.appendDrop(sb, bindValueArray[i], limit); + final String bindValue = StringUtils.defaultString(bindValueArray[i], ""); + StringUtils.appendDrop(sb, bindValue, limit); if (i < end) { sb.append(", "); } diff --git a/profiler/src/main/java/com/navercorp/pinpoint/profiler/modifier/db/interceptor/PreparedStatementExecuteQueryInterceptor.java b/profiler/src/main/java/com/navercorp/pinpoint/profiler/modifier/db/interceptor/PreparedStatementExecuteQueryInterceptor.java index 0b1f9da3d..4ea56ffcf 100644 --- a/profiler/src/main/java/com/navercorp/pinpoint/profiler/modifier/db/interceptor/PreparedStatementExecuteQueryInterceptor.java +++ b/profiler/src/main/java/com/navercorp/pinpoint/profiler/modifier/db/interceptor/PreparedStatementExecuteQueryInterceptor.java @@ -106,17 +106,7 @@ public class PreparedStatementExecuteQueryInterceptor implements SimpleAroundInt } private String toBindVariable(Map bindValue) { - final String[] temp = new String[bindValue.size()]; - for (Map.Entry entry : bindValue.entrySet()) { - Integer key = entry.getKey() - 1; - if (temp.length < key) { - continue; - } - temp[key] = entry.getValue(); - } - - return BindValueUtils.bindValueToString(temp, maxSqlBindValueLength); - + return BindValueUtils.bindValueToString(bindValue, maxSqlBindValueLength); } @Override diff --git a/profiler/src/test/java/com/navercorp/pinpoint/profiler/modifier/db/interceptor/BindValueUtilsTest.java b/profiler/src/test/java/com/navercorp/pinpoint/profiler/modifier/db/interceptor/BindValueUtilsTest.java index a903ed829..765e628c2 100644 --- a/profiler/src/test/java/com/navercorp/pinpoint/profiler/modifier/db/interceptor/BindValueUtilsTest.java +++ b/profiler/src/test/java/com/navercorp/pinpoint/profiler/modifier/db/interceptor/BindValueUtilsTest.java @@ -17,10 +17,10 @@ package com.navercorp.pinpoint.profiler.modifier.db.interceptor; import org.junit.Assert; - import org.junit.Test; -import com.navercorp.pinpoint.profiler.modifier.db.interceptor.BindValueUtils; +import java.util.HashMap; +import java.util.Map; public class BindValueUtilsTest { @@ -85,7 +85,7 @@ public class BindValueUtilsTest { @Test public void testBindValueToString_null() throws Exception { - String result = BindValueUtils.bindValueToString(null, 10); + String result = BindValueUtils.bindValueToString((String[])null, 10); Assert.assertEquals("", result); } @@ -109,4 +109,56 @@ public class BindValueUtilsTest { String result = BindValueUtils.bindValueToString(bindValue, 5); Assert.assertEquals("12345...(6), ...(2)", result); } + + // #737 https://github.com/naver/pinpoint/issues/737 + @Test + public void test_734_bug_regression() throws Exception { + Map bindValue = new HashMap(); + bindValue.put(1, "1"); + bindValue.put(2, "2"); + // skip 3 + bindValue.put(4, "4"); + + String bindValueToString = BindValueUtils.bindValueToString(bindValue, 100); + Assert.assertEquals("1, 2, , 4", bindValueToString); + } + + @Test + public void test_index_error_zero() throws Exception { + Map bindValue = new HashMap(); + bindValue.put(0, "0"); + + String bindValueToString = BindValueUtils.bindValueToString(bindValue, 100); + Assert.assertEquals("", bindValueToString); + } + + @Test + public void test_index_error_negative() throws Exception { + Map bindValue = new HashMap(); + bindValue.put(-2, "-2"); + + String bindValueToString = BindValueUtils.bindValueToString(bindValue, 100); + Assert.assertEquals("", bindValueToString); + } + + @Test + public void test_index_error_complex() throws Exception { + Map bindValue = new HashMap(); + bindValue.put(-2, "-2"); + bindValue.put(0, "0"); + bindValue.put(1, "1"); + bindValue.put(3, "3"); + + String bindValueToString = BindValueUtils.bindValueToString(bindValue, 100); + Assert.assertEquals("1, , 3", bindValueToString); + } + + @Test + public void test_NullElement() throws Exception { + String[] temp = {"1", null, "3"}; + String bindValueToString = BindValueUtils.bindValueToString(temp, 100); + Assert.assertEquals("1, , 3", bindValueToString); + } + + } \ No newline at end of file