Make header recording null safe (#159)

See https://discuss.elastic.co/t/unable-to-instrument-weblogic-server-with-apm-java-agent/143312

Author: felixbarny

apm-agent-core/src/main/java/co/elastic/apm/impl/context/Request.java

@@ -112,8 +112,10 @@ public PotentiallyMultiValuedMap getFormUrlEncodedParameters() {
      * @param headerValue The value of the header.
      * @return {@code this}, for fluent method chaining
      */
-    public Request addHeader(String headerName, String headerValue) {
-        headers.add(headerName, headerValue);
+    public Request addHeader(String headerName, @Nullable String headerValue) {
+        if (headerValue != null) {
+            headers.add(headerName, headerValue);
+        }
         return this;
     }
 

apm-agent-core/src/main/java/co/elastic/apm/report/serialize/DslJsonSerializer.java

@@ -587,11 +587,11 @@ private void writeField(final String fieldName, final PotentiallyMultiValuedMap
         }
     }
 
-    private void serializePotentiallyMultiValuedEntry(String key, Object value) {
+    private void serializePotentiallyMultiValuedEntry(String key, @Nullable Object value) {
         jw.writeString(key);
         jw.writeByte(JsonWriter.SEMI);
         if (value instanceof String) {
-            StringConverter.serializeNullable((String) value, jw);
+            StringConverter.serialize((String) value, jw);
         } else if (value instanceof List) {
             jw.writeByte(ARRAY_START);
             final List<String> values = (List<String>) value;
@@ -601,6 +601,8 @@ private void serializePotentiallyMultiValuedEntry(String key, Object value) {
                 jw.writeString(values.get(i));
             }
             jw.writeByte(ARRAY_END);
+        } else if (value == null) {
+            jw.writeNull();
         }
     }
 

apm-agent-core/src/test/java/co/elastic/apm/report/serialize/DslJsonSerializerTest.java

@@ -20,6 +20,8 @@
 package co.elastic.apm.report.serialize;
 
 import co.elastic.apm.configuration.CoreConfiguration;
+import co.elastic.apm.impl.ElasticApmTracer;
+import co.elastic.apm.impl.transaction.Transaction;
 import com.dslplatform.json.JsonWriter;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.JsonNode;
@@ -32,6 +34,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.SoftAssertions.assertSoftly;
+import static org.mockito.Mockito.mock;
 
 
 class DslJsonSerializerTest {
@@ -74,6 +77,19 @@ void testLimitStringValueLength() throws IOException {
         assertThat(jsonNode.get("lastString").textValue()).hasSize(DslJsonSerializer.MAX_VALUE_LENGTH).endsWith("…");
     }
 
+    @Test
+    void testNullHeaders() throws IOException {
+        Transaction transaction = new Transaction();
+        transaction.getContext().getRequest().addHeader("foo", null);
+        transaction.getContext().getRequest().getHeaders().add("bar", null);
+        JsonNode jsonNode = objectMapper.readTree(serializer.toJsonString(transaction));
+        System.out.println(jsonNode);
+        // calling addHeader with a null value ignores the header
+        assertThat(jsonNode.get("context").get("request").get("headers").get("foo")).isNull();
+        // should a null value sneak in, it should not break
+        assertThat(jsonNode.get("context").get("request").get("headers").get("bar").isNull()).isTrue();
+    }
+
     private String toJson(Map<String, String> map) {
         try {
             return objectMapper.writeValueAsString(map);

apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/servlet/ServletApiAdvice.java

@@ -76,9 +76,11 @@ public static Transaction onEnterServletService(@Advice.Argument(0) ServletReque
                 }
             }
             final Enumeration headerNames = request.getHeaderNames();
-            while (headerNames.hasMoreElements()) {
-                final String headerName = (String) headerNames.nextElement();
-                req.addHeader(headerName, request.getHeader(headerName));
+            if (headerNames != null) {
+                while (headerNames.hasMoreElements()) {
+                    final String headerName = (String) headerNames.nextElement();
+                    req.addHeader(headerName, request.getHeader(headerName));
+                }
             }
 
             final Principal userPrincipal = request.getUserPrincipal();