Merge pull request #741 from emeroad/#740_backport_bindValue_array_in…

…dex_bug_fix #740 backport. bindValue array index bug fix.
pinpoint-apm · Jul 17, 2015 · c0a58f1 · c0a58f1
2 parents 9d13573 + 08d441c
commit c0a58f1
Show file tree

Hide file tree

Showing 3 changed files with 98 additions and 15 deletions.
diff --git a/...src/main/java/com/navercorp/pinpoint/profiler/modifier/db/interceptor/BindValueUtils.java b/...src/main/java/com/navercorp/pinpoint/profiler/modifier/db/interceptor/BindValueUtils.java
@@ -18,14 +18,50 @@
 
 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<Integer, String> 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<Integer, String> 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<Integer, String> 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 static String bindValueToString(String[] bindValueArray, int limit) {
                 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(", ");
             }
@@ -57,4 +94,4 @@ private static void appendLength(StringBuilder sb, int length) {
     public static String bindValueToString(String[] stringArray) {
         return bindValueToString(stringArray, Integer.MAX_VALUE);
     }
-}
+}
diff --git a/...p/pinpoint/profiler/modifier/db/interceptor/PreparedStatementExecuteQueryInterceptor.java b/...p/pinpoint/profiler/modifier/db/interceptor/PreparedStatementExecuteQueryInterceptor.java
@@ -106,17 +106,7 @@ private void clean(Object target) {
     }
 
     private String toBindVariable(Map<Integer, String> bindValue) {
-        final String[] temp = new String[bindValue.size()];
-        for (Map.Entry<Integer, String> 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/...test/java/com/navercorp/pinpoint/profiler/modifier/db/interceptor/BindValueUtilsTest.java b/...test/java/com/navercorp/pinpoint/profiler/modifier/db/interceptor/BindValueUtilsTest.java
@@ -22,6 +22,9 @@
 
 import com.navercorp.pinpoint.profiler.modifier.db.interceptor.BindValueUtils;
 
+import java.util.HashMap;
+import java.util.Map;
+
 public class BindValueUtilsTest {
 
     @Test
@@ -85,7 +88,7 @@ public void testBindValueToString_limit7() throws Exception {
 
     @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 +112,57 @@ public void testBindValueToString_twoLargeString() throws Exception {
         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<Integer, String> bindValue = new HashMap<Integer, String>();
+        bindValue.put(1, "1");
+        bindValue.put(2, "2");
+        // skip 3
+        bindValue.put(4, "4");
+
+        String bindValueToString = BindValueUtils.bindValueToString(bindValue, 100);
+        org.junit.Assert.assertEquals("1, 2, , 4", bindValueToString);
+    }
+
+    @Test
+    public void test_index_error_zero() throws Exception {
+        Map<Integer, String> bindValue = new HashMap<Integer, String>();
+        bindValue.put(0, "0");
+
+        String bindValueToString = BindValueUtils.bindValueToString(bindValue, 100);
+        org.junit.Assert.assertEquals("", bindValueToString);
+    }
+
+    @Test
+    public void test_index_error_negative() throws Exception {
+        Map<Integer, String> bindValue = new HashMap<Integer, String>();
+        bindValue.put(-2, "-2");
+
+        String bindValueToString = BindValueUtils.bindValueToString(bindValue, 100);
+        org.junit.Assert.assertEquals("", bindValueToString);
+    }
+
+    @Test
+    public void test_index_error_complex() throws Exception {
+        Map<Integer, String> bindValue = new HashMap<Integer, String>();
+        bindValue.put(-2, "-2");
+        bindValue.put(0, "0");
+        bindValue.put(1, "1");
+        bindValue.put(3, "3");
+
+        String bindValueToString = BindValueUtils.bindValueToString(bindValue, 100);
+        org.junit.Assert.assertEquals("1, , 3", bindValueToString);
+    }
+
+    @Test
+    public void test_NullElement() throws Exception {
+        String[] temp = {"1", null, "3"};
+        String bindValueToString = BindValueUtils.bindValueToString(temp, 100);
+        org.junit.Assert.assertEquals("1, , 3", bindValueToString);
+    }
+
+
 }