From dd90cf1bbb86ae27c682298abde7020edd0fb44e Mon Sep 17 00:00:00 2001
From: Lee Hinman <dakrone@users.noreply.github.com>
Date: Fri, 8 Sep 2017 13:30:17 -0600
Subject: [PATCH] Throw a better error message for empty field names (#26543)

* Throw a better error message for empty field names

When a document is parsed with a `""` for a field name, we currently throw a
confusing error about `.` being present in the field. This changes the error
message to be clearer about what's causing the problem.

Resolves #23348

* Fix exception message in test
---
 .../index/mapper/DocumentParser.java          | 19 ++++++++++-----
 .../index/mapper/DocumentParserTests.java     | 23 +++++++++++++++++++
 .../elasticsearch/indexing/IndexActionIT.java |  2 +-
 3 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java
index b37367d992f..cdf4bd07ceb 100644
--- a/core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java
+++ b/core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java
@@ -175,14 +175,21 @@ final class DocumentParser {
     }
 
     private static String[] splitAndValidatePath(String fullFieldPath) {
-        String[] parts = fullFieldPath.split("\\.");
-        for (String part : parts) {
-            if (Strings.hasText(part) == false) {
-                throw new IllegalArgumentException(
-                    "object field starting or ending with a [.] makes object resolution ambiguous: [" + fullFieldPath + "]");
+        if (fullFieldPath.contains(".")) {
+            String[] parts = fullFieldPath.split("\\.");
+            for (String part : parts) {
+                if (Strings.hasText(part) == false) {
+                    throw new IllegalArgumentException(
+                            "object field starting or ending with a [.] makes object resolution ambiguous: [" + fullFieldPath + "]");
+                }
             }
+            return parts;
+        } else {
+            if (Strings.isEmpty(fullFieldPath)) {
+                throw new IllegalArgumentException("field name cannot be an empty string");
+            }
+            return new String[] {fullFieldPath};
         }
-        return parts;
     }
 
     /** Creates a Mapping containing any dynamically added fields, or returns null if there were no dynamic mappings. */
diff --git a/core/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java b/core/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java
index 4d83cc99846..cbf890ef476 100644
--- a/core/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java
+++ b/core/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.index.mapper;
 
+import org.elasticsearch.ExceptionsHelper;
 import org.elasticsearch.Version;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.bytes.BytesArray;
@@ -1375,4 +1376,26 @@ public class DocumentParserTests extends ESSingleNodeTestCase {
                     containsString("object field starting or ending with a [.] makes object resolution ambiguous: [top..foo..bar]"));
         }
     }
+
+    public void testBlankFieldNames() throws Exception {
+        final BytesReference bytes = XContentFactory.jsonBuilder()
+                .startObject()
+                .field("", "foo")
+                .endObject().bytes();
+
+        MapperParsingException err = expectThrows(MapperParsingException.class, () ->
+                client().prepareIndex("idx", "type").setSource(bytes, XContentType.JSON).get());
+        assertThat(ExceptionsHelper.detailedMessage(err), containsString("field name cannot be an empty string"));
+
+        final BytesReference bytes2 = XContentFactory.jsonBuilder()
+                .startObject()
+                .startObject("foo")
+                .field("", "bar")
+                .endObject()
+                .endObject().bytes();
+
+        err = expectThrows(MapperParsingException.class, () ->
+                client().prepareIndex("idx", "type").setSource(bytes2, XContentType.JSON).get());
+        assertThat(ExceptionsHelper.detailedMessage(err), containsString("field name cannot be an empty string"));
+    }
 }
diff --git a/core/src/test/java/org/elasticsearch/indexing/IndexActionIT.java b/core/src/test/java/org/elasticsearch/indexing/IndexActionIT.java
index 63f889179a2..7abb603b8eb 100644
--- a/core/src/test/java/org/elasticsearch/indexing/IndexActionIT.java
+++ b/core/src/test/java/org/elasticsearch/indexing/IndexActionIT.java
@@ -250,6 +250,6 @@ public class IndexActionIT extends ESIntegTestCase {
         );
         assertThat(e.getMessage(), containsString("failed to parse"));
         assertThat(e.getRootCause().getMessage(),
-                containsString("object field starting or ending with a [.] makes object resolution ambiguous: []"));
+                containsString("field name cannot be an empty string"));
     }
 }