From: Geneviève Bastien <gbastien+lttng@versatic.net>
Date: Fri, 24 Mar 2017 13:58:57 +0000 (-0400)
Subject: common: Fix NPE in log arguments
X-Git-Url: http://drtracing.org/?a=commitdiff_plain;h=9c9523fdad85d9f98c5c39c4827f1ff36114c6f8;p=deliverable%2Ftracecompass.git

common: Fix NPE in log arguments

Loggers should NEVER throw exceptions

Change-Id: I39f7ffd18735c6ee2919b76afaec1e1616702c30
Signed-off-by: Geneviève Bastien <gbastien+lttng@versatic.net>
Reviewed-on: https://git.eclipse.org/r/93809
Reviewed-by: Hudson CI
Reviewed-by: Matthew Khouzam <matthew.khouzam@ericsson.com>
Tested-by: Matthew Khouzam <matthew.khouzam@ericsson.com>
---

diff --git a/common/org.eclipse.tracecompass.common.core.tests/src/org/eclipse/tracecompass/common/core/tests/log/LoggerTest.java b/common/org.eclipse.tracecompass.common.core.tests/src/org/eclipse/tracecompass/common/core/tests/log/LoggerTest.java
index 1ac78d2c7e..c8a3a5394d 100644
--- a/common/org.eclipse.tracecompass.common.core.tests/src/org/eclipse/tracecompass/common/core/tests/log/LoggerTest.java
+++ b/common/org.eclipse.tracecompass.common.core.tests/src/org/eclipse/tracecompass/common/core/tests/log/LoggerTest.java
@@ -516,4 +516,19 @@ public class LoggerTest {
         assertEquals("FINE: {\"ts\":0,\"ph\":\"e\",\"tid\":1,\"name\":\"network connect\",\"cat\":\"net\",\"id\":\"0x1234\",\"args\":{\"msg\":\"OK\"}}", fLog.getMessages().get(6));
     }
 
+    /**
+     * Test that null values in arguments are properly handled
+     */
+    @Test
+    public void testNullArguments() {
+        Logger logger = fLogger;
+        assertNotNull(logger);
+        TraceCompassLogUtils.traceInstant(logger, Level.INFO, "test null value", "nullvalue", null);
+        TraceCompassLogUtils.traceInstant(logger, Level.INFO, "test null key", null, "value");
+
+        fStreamHandler.flush();
+        assertEquals("INFO: {\"ts\":0,\"ph\":\"i\",\"tid\":1,\"name\":\"test null value\",\"args\":{\"nullvalue\":\"null\"}}", fLog.getMessages().get(0));
+        assertEquals("INFO: {\"ts\":0,\"ph\":\"i\",\"tid\":1,\"name\":\"test null key\",\"args\":{\"null\":\"value\"}}", fLog.getMessages().get(1));
+    }
+
 }
diff --git a/common/org.eclipse.tracecompass.common.core/src/org/eclipse/tracecompass/common/core/log/TraceCompassLogUtils.java b/common/org.eclipse.tracecompass.common.core/src/org/eclipse/tracecompass/common/core/log/TraceCompassLogUtils.java
index 77ee8534cf..79cdbfa2c8 100644
--- a/common/org.eclipse.tracecompass.common.core/src/org/eclipse/tracecompass/common/core/log/TraceCompassLogUtils.java
+++ b/common/org.eclipse.tracecompass.common.core/src/org/eclipse/tracecompass/common/core/log/TraceCompassLogUtils.java
@@ -21,7 +21,6 @@ import java.util.logging.Level;
 import java.util.logging.Logger;
 
 import org.eclipse.jdt.annotation.Nullable;
-import org.eclipse.tracecompass.common.core.NonNullUtils;
 
 /**
  * Logger helper
@@ -742,19 +741,19 @@ public final class TraceCompassLogUtils {
                 if (i > 0) {
                     appendTo.append(',');
                 }
-                writeObject(appendTo, keyVal, NonNullUtils.checkNotNull(value));
+                writeObject(appendTo, keyVal, value);
             }
         }
 
         return appendTo.append('}');
     }
 
-    private static StringBuilder writeObject(StringBuilder appendTo, Object key, Object value) {
+    private static StringBuilder writeObject(StringBuilder appendTo, Object key, @Nullable Object value) {
         appendTo.append('"').append(key).append('"').append(':');
         if (value instanceof Number) {
             appendTo.append(value);
         } else {
-            appendTo.append('"').append(value).append('"');
+            appendTo.append('"').append(String.valueOf(value)).append('"');
         }
         return appendTo;
     }