Disallow introducing illegal object mappings (double '..')

This disallows object mappings that would accidentally create something like
`foo..bar`, which is then unparsable for the `bar` field as it does not know
what its parent is.

Resolves #22794

core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java

@@ -22,6 +22,7 @@ package org.elasticsearch.index.mapper;
 import org.apache.lucene.document.Field;
 import org.apache.lucene.index.IndexableField;
 import org.elasticsearch.Version;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.joda.FormatDateTimeFormatter;
 import org.elasticsearch.common.xcontent.XContentHelper;
@@ -172,6 +173,17 @@ final class DocumentParser {
         return new MapperParsingException("failed to parse", e);
     }
 
+    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 + "]");
+            }
+        }
+        return parts;
+    }
+
     /** Creates a Mapping containing any dynamically added fields, or returns null if there were no dynamic mappings. */
     static Mapping createDynamicUpdate(Mapping mapping, DocumentMapper docMapper, List<Mapper> dynamicMappers) {
         if (dynamicMappers.isEmpty()) {
@@ -184,7 +196,7 @@ final class DocumentParser {
         Iterator<Mapper> dynamicMapperItr = dynamicMappers.iterator();
         List<ObjectMapper> parentMappers = new ArrayList<>();
         Mapper firstUpdate = dynamicMapperItr.next();
-        parentMappers.add(createUpdate(mapping.root(), firstUpdate.name().split("\\."), 0, firstUpdate));
+        parentMappers.add(createUpdate(mapping.root(), splitAndValidatePath(firstUpdate.name()), 0, firstUpdate));
         Mapper previousMapper = null;
         while (dynamicMapperItr.hasNext()) {
             Mapper newMapper = dynamicMapperItr.next();
@@ -196,7 +208,7 @@ final class DocumentParser {
                 continue;
             }
             previousMapper = newMapper;
-            String[] nameParts = newMapper.name().split("\\.");
+            String[] nameParts = splitAndValidatePath(newMapper.name());
 
             // We first need the stack to only contain mappers in common with the previously processed mapper
             // For example, if the first mapper processed was a.b.c, and we now have a.d, the stack will contain
@@ -453,7 +465,7 @@ final class DocumentParser {
             context.path().remove();
         } else {
 
-            final String[] paths = currentFieldName.split("\\.");
+            final String[] paths = splitAndValidatePath(currentFieldName);
             currentFieldName = paths[paths.length - 1];
             Tuple<Integer, ObjectMapper> parentMapperTuple = getDynamicParentMapper(context, paths, mapper);
             ObjectMapper parentMapper = parentMapperTuple.v2();
@@ -497,7 +509,7 @@ final class DocumentParser {
             }
         } else {
 
-            final String[] paths = arrayFieldName.split("\\.");
+            final String[] paths = splitAndValidatePath(arrayFieldName);
             arrayFieldName = paths[paths.length - 1];
             lastFieldName = arrayFieldName;
             Tuple<Integer, ObjectMapper> parentMapperTuple = getDynamicParentMapper(context, paths, parentMapper);
@@ -561,7 +573,7 @@ final class DocumentParser {
             parseObjectOrField(context, mapper);
         } else {
 
-            final String[] paths = currentFieldName.split("\\.");
+            final String[] paths = splitAndValidatePath(currentFieldName);
             currentFieldName = paths[paths.length - 1];
             Tuple<Integer, ObjectMapper> parentMapperTuple = getDynamicParentMapper(context, paths, parentMapper);
             parentMapper = parentMapperTuple.v2();
@@ -813,7 +825,7 @@ final class DocumentParser {
             // The path of the dest field might be completely different from the current one so we need to reset it
             context = context.overridePath(new ContentPath(0));
 
-            final String[] paths = field.split("\\.");
+            final String[] paths = splitAndValidatePath(field);
             final String fieldName = paths[paths.length-1];
             Tuple<Integer, ObjectMapper> parentMapperTuple = getDynamicParentMapper(context, paths, null);
             ObjectMapper mapper = parentMapperTuple.v2();
@@ -897,7 +909,7 @@ final class DocumentParser {
 
     // looks up a child mapper, but takes into account field names that expand to objects
     static Mapper getMapper(ObjectMapper objectMapper, String fieldName) {
-        String[] subfields = fieldName.split("\\.");
+        String[] subfields = splitAndValidatePath(fieldName);
         for (int i = 0; i < subfields.length - 1; ++i) {
             Mapper mapper = objectMapper.getMapper(subfields[i]);
             if (mapper == null || (mapper instanceof ObjectMapper) == false) {

core/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java

@@ -47,6 +47,7 @@ import org.elasticsearch.test.InternalSettingsPlugin;
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.elasticsearch.test.StreamsUtils.copyToBytesFromClasspath;
 import static org.elasticsearch.test.StreamsUtils.copyToStringFromClasspath;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.not;
@@ -1262,4 +1263,36 @@ public class DocumentParserTests extends ESSingleNodeTestCase {
         assertNotNull(dateMapper);
         assertThat(dateMapper, instanceOf(DateFieldMapper.class));
     }
+
+    public void testDynamicFieldsStartingAndEndingWithDot() throws Exception {
+        BytesReference bytes = XContentFactory.jsonBuilder().startObject().startArray("top.")
+                .startObject().startArray("foo.")
+                .startObject()
+                .field("thing", "bah")
+                .endObject().endArray()
+                .endObject().endArray()
+                .endObject().bytes();
+
+        client().prepareIndex("idx", "type").setSource(bytes).get();
+
+        bytes = XContentFactory.jsonBuilder().startObject().startArray("top.")
+                .startObject().startArray("foo.")
+                .startObject()
+                .startObject("bar.")
+                .startObject("aoeu")
+                .field("a", 1).field("b", 2)
+                .endObject()
+                .endObject()
+                .endObject()
+                .endArray().endObject().endArray()
+                .endObject().bytes();
+
+        try {
+            client().prepareIndex("idx", "type").setSource(bytes).get();
+            fail("should have failed to dynamically introduce a double-dot field");
+        } catch (IllegalArgumentException e) {
+            assertThat(e.getMessage(),
+                    containsString("object field starting or ending with a [.] makes object resolution ambiguous: [top..foo..bar]"));
+        }
+    }
 }

core/src/test/java/org/elasticsearch/indexing/IndexActionIT.java

@@ -257,21 +257,7 @@ public class IndexActionIT extends ESIntegTestCase {
             }
         );
         assertThat(e.getMessage(), containsString("failed to parse"));
-        assertThat(e.getRootCause().getMessage(), containsString("name cannot be empty string"));
+        assertThat(e.getRootCause().getMessage(),
-    }
+                containsString("object field starting or ending with a [.] makes object resolution ambiguous: []"));
-
-    @Override
-    protected Collection<Class<? extends Plugin>> nodePlugins() {
-        return Collections.singleton(InternalSettingsPlugin.class); // uses index.version.created
-    }
-
-    public void testDocumentWithBlankFieldName2x() {
-        Version version = VersionUtils.randomVersionBetween(random(), Version.V_2_0_0, Version.V_2_3_4);
-        Settings settings = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, version).build();
-        assertAcked(prepareCreate("test1").setSettings(settings));
-        ensureGreen();
-
-        IndexResponse indexResponse = client().prepareIndex("test1", "type", "1").setSource("", "value1_2").execute().actionGet();
-        assertEquals(DocWriteResponse.Result.CREATED, indexResponse.getResult());
     }
 }