Fix NPE when logging null values in JSON (#53715)

Slow log's routing value when null was causing ESLogMessage.asJson method to throw Null Pointer Exception. This should be fixed in ESLogMessage as well as prevent passing that key at all. This only happens in 8 because of previous refactoring #46702
elastic · Mar 19, 2020 · 1dac8df · 1dac8df
1 parent a2b428f
commit 1dac8df
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 3 deletions.
diff --git a/server/src/main/java/org/elasticsearch/common/logging/ESLogMessage.java b/server/src/main/java/org/elasticsearch/common/logging/ESLogMessage.java
@@ -79,7 +79,7 @@ protected void asJson(StringBuilder sb) {
             StringBuilders.escapeJson(sb, start);
             sb.append(Chars.DQUOTE).append(':').append(Chars.DQUOTE);
             start = sb.length();
-            sb.append(getIndexedReadOnlyStringMap().getValueAt(i).toString());
+            sb.append((Object) getIndexedReadOnlyStringMap().getValueAt(i));
             StringBuilders.escapeJson(sb, start);
             sb.append(Chars.DQUOTE);
         }

diff --git a/server/src/main/java/org/elasticsearch/index/IndexingSlowLog.java b/server/src/main/java/org/elasticsearch/index/IndexingSlowLog.java
@@ -24,8 +24,8 @@
 import org.apache.logging.log4j.util.StringBuilders;
 import org.elasticsearch.common.Booleans;
 import org.elasticsearch.common.Strings;
-import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.common.logging.ESLogMessage;
+import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Setting.Property;
 import org.elasticsearch.common.unit.TimeValue;
@@ -180,7 +180,9 @@ private static Map<String, Object> prepareMap(Index index, ParsedDocument doc, l
             map.put("took", TimeValue.timeValueNanos(tookInNanos));
             map.put("took_millis", ""+TimeUnit.NANOSECONDS.toMillis(tookInNanos));
             map.put("id", doc.id());
-            map.put("routing", doc.routing());
+            if (doc.routing() != null) {
+                map.put("routing", doc.routing());
+            }
 
             if (maxSourceCharsToLog == 0 || doc.source() == null || doc.source().length() == 0) {
                 return map;

diff --git a/server/src/test/java/org/elasticsearch/index/IndexingSlowLogTests.java b/server/src/test/java/org/elasticsearch/index/IndexingSlowLogTests.java
@@ -46,6 +46,7 @@
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.nullValue;
 import static org.hamcrest.Matchers.startsWith;
 
 public class IndexingSlowLogTests extends ESTestCase {
@@ -72,6 +73,20 @@ public void testSlowLogMessageHasJsonFields() throws IOException {
         assertThat(p.get("source"), containsString("{\\\"foo\\\":\\\"bar\\\"}"));
     }
 
+    public void testEmptyRoutingField() throws IOException {
+        BytesReference source = BytesReference.bytes(JsonXContent.contentBuilder()
+                                                                 .startObject().field("foo", "bar").endObject());
+        ParsedDocument pd = new ParsedDocument(new NumericDocValuesField("version", 1),
+            SeqNoFieldMapper.SequenceIDFields.emptySeqID(), "id",
+            null, null, source, XContentType.JSON, null);
+        Index index = new Index("foo", "123");
+
+        ESLogMessage p = IndexingSlowLogMessage.of(index, pd, 10, true, 0);
+        assertThat(p.get("routing"), nullValue());
+
+        assertThat(p.asString(), containsString("routing[]"));
+    }
+
     public void testSlowLogParsedDocumentPrinterSourceToLog() throws IOException {
         BytesReference source = BytesReference.bytes(JsonXContent.contentBuilder()
             .startObject().field("foo", "bar").endObject());