Mappings: Enforce field names do not contain dot

Field names containing dots can cause problems. For example, @jpountz
made this recreation which cause no error, but can result in a
serialization exception if the type already exists:
https://gist.github.com/jpountz/8c66817e00a322b81f85

But this is not just a potential conflict. It also has larger problems,
since only the leaf mapper is created. The intermediate "foo" object
field would not exist if only "foo.bar" was in the mappings.

This change forbids the use of dots in field names. It also
fixes an issue with passing through the update_all_types setting,
which was always set to true whenever a type already existed (!).

I do not think we should worry about backwards compatibility here. This
should be a hard break (and added to the migration plugin).

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

@@ -399,10 +399,10 @@ public class DocumentMapper implements ToXContent {
         return mapperService.getParentTypes().contains(type);
     }
 
-    private void addMappers(Collection<ObjectMapper> objectMappers, Collection<FieldMapper> fieldMappers) {
+    private void addMappers(Collection<ObjectMapper> objectMappers, Collection<FieldMapper> fieldMappers, boolean updateAllTypes) {
         assert mappingLock.isWriteLockedByCurrentThread();
         // first ensure we don't have any incompatible new fields
-        mapperService.checkNewMappersCompatibility(objectMappers, fieldMappers, true);
+        mapperService.checkNewMappersCompatibility(objectMappers, fieldMappers, updateAllTypes);
 
         // update mappers for this document type
         MapBuilder<String, ObjectMapper> builder = MapBuilder.newMapBuilder(this.objectMappers);
@@ -424,7 +424,7 @@ public class DocumentMapper implements ToXContent {
             final MergeResult mergeResult = new MergeResult(simulate, updateAllTypes);
             this.mapping.merge(mapping, mergeResult);
             if (simulate == false) {
-                addMappers(mergeResult.getNewObjectMappers(), mergeResult.getNewFieldMappers());
+                addMappers(mergeResult.getNewObjectMappers(), mergeResult.getNewFieldMappers(), updateAllTypes);
                 refreshSource();
             }
             return mergeResult;

core/src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java

@@ -278,7 +278,10 @@ public class ObjectMapper extends Mapper implements AllFieldMapper.IncludeInAll,
             Iterator<Map.Entry<String, Object>> iterator = propsNode.entrySet().iterator();
             while (iterator.hasNext()) {
                 Map.Entry<String, Object> entry = iterator.next();
-                String propName = entry.getKey();
+                String fieldName = entry.getKey();
+                if (fieldName.contains(".")) {
+                    throw new MapperParsingException("Field name [" + fieldName + "] cannot contain '.'");
+                }
                 // Should accept empty arrays, as a work around for when the
                 // user can't provide an empty Map. (PHP for example)
                 boolean isEmptyList = entry.getValue() instanceof List && ((List<?>) entry.getValue()).isEmpty();
@@ -301,23 +304,23 @@ public class ObjectMapper extends Mapper implements AllFieldMapper.IncludeInAll,
                             // any type, including core values, which
                             type = ObjectMapper.CONTENT_TYPE;
                         } else {
-                            throw new MapperParsingException("No type specified for property [" + propName + "]");
+                            throw new MapperParsingException("No type specified for field [" + fieldName + "]");
                         }
                     }
 
                     Mapper.TypeParser typeParser = parserContext.typeParser(type);
                     if (typeParser == null) {
-                        throw new MapperParsingException("No handler for type [" + type + "] declared on field [" + propName + "]");
+                        throw new MapperParsingException("No handler for type [" + type + "] declared on field [" + fieldName + "]");
                     }
-                    objBuilder.add(typeParser.parse(propName, propNode, parserContext));
+                    objBuilder.add(typeParser.parse(fieldName, propNode, parserContext));
                     propNode.remove("type");
-                    DocumentMapperParser.checkNoRemainingFields(propName, propNode, parserContext.indexVersionCreated());
+                    DocumentMapperParser.checkNoRemainingFields(fieldName, propNode, parserContext.indexVersionCreated());
                     iterator.remove();
                 } else if (isEmptyList) {
                     iterator.remove();
                 } else {
-                    throw new MapperParsingException("Expected map for property [fields] on field [" + propName + "] but got a "
-                            + propName.getClass());
+                    throw new MapperParsingException("Expected map for property [fields] on field [" + fieldName + "] but got a "
+                            + fieldName.getClass());
                 }
             }
 

core/src/test/java/org/elasticsearch/index/mapper/simple/SimpleMapperTests.java

@@ -23,6 +23,7 @@ import com.google.common.base.Charsets;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.index.mapper.*;
 import org.elasticsearch.index.mapper.ParseContext.Document;
 import org.elasticsearch.index.IndexService;
@@ -135,4 +136,18 @@ public class SimpleMapperTests extends ElasticsearchSingleNodeTest {
             assertThat(e.getMessage(), equalTo("failed to parse, document is empty"));
         }
     }
+
+    public void testHazardousFieldNames() throws Exception {
+        IndexService indexService = createIndex("test");
+        DocumentMapperParser mapperParser = indexService.mapperService().documentMapperParser();
+        String mapping = XContentFactory.jsonBuilder().startObject().startObject("type").startObject("properties")
+            .startObject("foo.bar").field("type", "string").endObject()
+            .endObject().endObject().string();
+        try {
+            mapperParser.parse(mapping);
+            fail("Mapping parse should have failed");
+        } catch (MapperParsingException e) {
+            assertTrue(e.getMessage(), e.getMessage().contains("cannot contain '.'"));
+        }
+    }
 }