Merge pull request #12068 from rjernst/fix/mapper-names-conflict

Mappings: Enforce field names do not contain dot

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 '.'"));
+        }
+    }
 }