Disallow duplicate dynamic_templates names

Currently the same `dynamic_templates` name can be used more than once in the same mappings file. This most certainly is unintentional and can create issues like those reported in elastic#28988. Furthermore, when templates are matched later in `RootObjectMapper#findTemplate`, it looks like we simply pick the first matching one we see in the input array, so using the same template name twice sounds like an error prone idea anyway. This change checks for duplicate names on parsing and throws an error if we encounter the same name twice. Closes elastic#28988
cbuescher · Feb 3, 2020 · 7b759fa · 7b759fa
1 parent 10b7ffa
commit 7b759fa
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 0 deletions.
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java
@@ -32,9 +32,11 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeBooleanValue;
 import static org.elasticsearch.index.mapper.TypeParsers.parseDateTimeFormatter;
@@ -170,6 +172,7 @@ protected boolean processField(RootObjectMapper.Builder builder, String fieldNam
                 */
                 List<?> tmplNodes = (List<?>) fieldNode;
                 List<DynamicTemplate> templates = new ArrayList<>();
+                Set<String> templatesAlreadyDefinded = new HashSet<>();
                 for (Object tmplNode : tmplNodes) {
                     Map<String, Object> tmpl = (Map<String, Object>) tmplNode;
                     if (tmpl.size() != 1) {
@@ -180,7 +183,12 @@ protected boolean processField(RootObjectMapper.Builder builder, String fieldNam
                     Map<String, Object> templateParams = (Map<String, Object>) entry.getValue();
                     DynamicTemplate template = DynamicTemplate.parse(templateName, templateParams);
                     if (template != null) {
+                        if (templatesAlreadyDefinded.contains(templateName)) {
+                            throw new MapperParsingException("A dynamic template cannot be defined twice, but a template with name ["
+                                    + templateName + "] has already been defined.");
+                        }
                         templates.add(template);
+                        templatesAlreadyDefinded.add(template.name());
                     }
                 }
                 builder.dynamicTemplates(templates);

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java
@@ -160,6 +160,38 @@ public void testDynamicTemplates() throws Exception {
         assertEquals(mapping3, mapper.mappingSource().toString());
     }
 
+    public void testDynamicTemplatesDuplicationThrows() throws Exception {
+        String mapping = Strings.toString(XContentFactory.jsonBuilder()
+                .startObject()
+                    .startObject("type")
+                        .startArray("dynamic_templates")
+                            .startObject()
+                                .startObject("my_template")
+                                    .field("match_mapping_type", "string")
+                                    .startObject("mapping")
+                                        .field("type", "keyword")
+                                    .endObject()
+                                .endObject()
+                            .endObject()
+                            .startObject()
+                                .startObject("my_template")
+                                .field("match_mapping_type", "string")
+                                .startObject("mapping")
+                                    .field("type", "keyword")
+                                .endObject()
+                            .endObject()
+                            .endObject()
+                        .endArray()
+                    .endObject()
+                .endObject());
+        MapperService mapperService = createIndex("test").mapperService();
+        MapperParsingException ex = expectThrows(MapperParsingException.class,
+                () -> mapperService.merge("type", new CompressedXContent(mapping), MergeReason.MAPPING_UPDATE));
+        assertEquals("Failed to parse mapping: A dynamic template cannot be defined twice, but a template with name [my_template]"
+                + " has already been defined.", ex.getMessage());
+
+    }
+
     public void testIllegalFormatField() throws Exception {
         String dynamicMapping = Strings.toString(XContentFactory.jsonBuilder()
             .startObject()