Make sure to reject mappings with type _doc when include_type_name is false. (#38484)

`CreateIndexRequest#source(Map<String, Object>, ... )`, which is used when
deserializing index creation requests, accidentally accepts mappings that are
nested twice under the type key (as described in the bug report #38266).

This in turn causes us to be too lenient in parsing typeless mappings. In
particular, we accept the following index creation request, even though it
should not contain the type key `_doc`:

```
PUT index?include_type_name=false
{
  "mappings": {
    "_doc": {
      "properties": { ... }
    }
  }
}
```

There is a similar issue for both 'put templates' and 'put mappings' requests
as well.

This PR makes the minimal changes to detect and reject these typed mappings in
requests. It does not address #38266 generally, or attempt a larger refactor
around types in these server-side requests, as I think this should be done at a
later time.

Author: jtibshirani

client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java

@@ -313,15 +313,13 @@ public void testCreateIndex() throws IOException {
             {
                 request = new CreateIndexRequest("twitter2");
                 //tag::create-index-mappings-map
-                Map<String, Object> jsonMap = new HashMap<>();
                 Map<String, Object> message = new HashMap<>();
                 message.put("type", "text");
                 Map<String, Object> properties = new HashMap<>();
                 properties.put("message", message);
                 Map<String, Object> mapping = new HashMap<>();
                 mapping.put("properties", properties);
-                jsonMap.put("_doc", mapping);
-                request.mapping(jsonMap); // <1>
+                request.mapping(mapping); // <1>
                 //end::create-index-mappings-map
                 CreateIndexResponse createIndexResponse = client.indices().create(request, RequestOptions.DEFAULT);
                 assertTrue(createIndexResponse.isAcknowledged());
@@ -332,15 +330,11 @@ public void testCreateIndex() throws IOException {
                 XContentBuilder builder = XContentFactory.jsonBuilder();
                 builder.startObject();
                 {
-                    builder.startObject("_doc");
+                    builder.startObject("properties");
                     {
-                        builder.startObject("properties");
+                        builder.startObject("message");
                         {
-                            builder.startObject("message");
-                            {
-                                builder.field("type", "text");
-                            }
-                            builder.endObject();
+                            builder.field("type", "text");
                         }
                         builder.endObject();
                     }
@@ -381,10 +375,8 @@ public void testCreateIndex() throws IOException {
                     "        \"number_of_replicas\" : 0\n" +
                     "    },\n" +
                     "    \"mappings\" : {\n" +
-                    "        \"_doc\" : {\n" +
-                    "            \"properties\" : {\n" +
-                    "                \"message\" : { \"type\" : \"text\" }\n" +
-                    "            }\n" +
+                    "        \"properties\" : {\n" +
+                    "            \"message\" : { \"type\" : \"text\" }\n" +
                     "        }\n" +
                     "    },\n" +
                     "    \"aliases\" : {\n" +

rest-api-spec/src/main/resources/rest-api-spec/test/indices.create/10_basic.yml

@@ -113,3 +113,23 @@
             properties:
               "":
                 type:     keyword
+
+---
+"Create index with explicit _doc type":
+  - skip:
+      version: " - 6.6.99"
+      reason: include_type_name was introduced in 6.7.0
+  - do:
+      catch: bad_request
+      indices.create:
+        include_type_name: false
+        index: test_index
+        body:
+          mappings:
+            _doc:
+              properties:
+                field:
+                  type: keyword
+
+  - match: { error.type: "illegal_argument_exception" }
+  - match: { error.reason: "The mapping definition cannot be nested under a type [_doc] unless include_type_name is set to true." }

rest-api-spec/src/main/resources/rest-api-spec/test/indices.put_mapping/10_basic.yml

@@ -74,3 +74,28 @@
           properties:
             "":
               type:     keyword
+
+---
+"Put mappings with explicit _doc type":
+  - skip:
+      version: " - 6.6.99"
+      reason: include_type_name was introduced in 6.7.0
+
+  - do:
+      indices.create:
+        include_type_name: false
+        index: test_index
+
+  - do:
+      catch: bad_request
+      indices.put_mapping:
+        include_type_name: false
+        index: test_index
+        body:
+          _doc:
+            properties:
+              field:
+                type: keyword
+
+  - match: { error.type: "illegal_argument_exception" }
+  - match: { error.reason: "Types cannot be provided in put mapping requests, unless the include_type_name parameter is set to true." }

rest-api-spec/src/main/resources/rest-api-spec/test/indices.put_template/10_basic.yml

@@ -260,3 +260,25 @@
       indices.put_template:
         name: test
         body: {}
+
+---
+"Put template with explicit _doc type":
+  - skip:
+      version: " - 6.6.99"
+      reason: include_type_name was introduced in 6.7.0
+
+  - do:
+      catch: bad_request
+      indices.put_template:
+        include_type_name: false
+        name: test
+        body:
+          index_patterns: test-*
+          mappings:
+            _doc:
+              properties:
+                field:
+                  type: keyword
+
+  - match: { error.type: "illegal_argument_exception" }
+  - match: { error.reason: "The mapping definition cannot be nested under a type [_doc] unless include_type_name is set to true." }

rest-api-spec/src/main/resources/rest-api-spec/test/indices.rollover/40_mapping.yml

@@ -45,3 +45,33 @@
 
   - match: { conditions: { "[max_docs: 2]": true } }
   - match: { rolled_over: true }
+
+---
+"Mappings with explicit _doc type":
+  - skip:
+      version: " - 6.6.99"
+      reason: include_type_name was introduced in 6.7.0
+
+  - do:
+      indices.create:
+        index: logs-1
+        body:
+          aliases:
+            logs_search: {}
+
+  - do:
+      catch: bad_request
+      indices.rollover:
+        include_type_name: false
+        alias: "logs_search"
+        body:
+          conditions:
+            max_docs: 2
+          mappings:
+            _doc:
+              properties:
+                field:
+                  type: keyword
+
+  - match: { error.caused_by.type: "illegal_argument_exception" }
+  - match: { error.caused_by.reason: "The mapping definition cannot be nested under a type [_doc] unless include_type_name is set to true." }

server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverRequest.java

@@ -70,14 +70,17 @@ public class RolloverRequest extends AcknowledgedRequest<RolloverRequest> implem
             CONDITIONS, ObjectParser.ValueType.OBJECT);
         PARSER.declareField((parser, request, context) -> request.createIndexRequest.settings(parser.map()),
             CreateIndexRequest.SETTINGS, ObjectParser.ValueType.OBJECT);
-        PARSER.declareField((parser, request, isTypeIncluded) -> {
-            if (isTypeIncluded) {
+        PARSER.declareField((parser, request, includeTypeName) -> {
+            if (includeTypeName) {
                 for (Map.Entry<String, Object> mappingsEntry : parser.map().entrySet()) {
                     request.createIndexRequest.mapping(mappingsEntry.getKey(), (Map<String, Object>) mappingsEntry.getValue());
                 }
             } else {
-                // a type is not included, add a dummy _doc type
-                request.createIndexRequest.mapping(MapperService.SINGLE_MAPPING_NAME, parser.map());
+                Map<String, Object> mappings = parser.map();
+                if (MapperService.isMappingSourceTyped(MapperService.SINGLE_MAPPING_NAME, mappings)) {
+                    throw new IllegalArgumentException("The mapping definition cannot be nested under a type " +
+                        "[" + MapperService.SINGLE_MAPPING_NAME + "] unless include_type_name is set to true.");
+                }
             }
         }, CreateIndexRequest.MAPPINGS, ObjectParser.ValueType.OBJECT);
         PARSER.declareField((parser, request, context) -> request.createIndexRequest.aliases(parser.map()),

server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java

@@ -38,8 +38,6 @@
 import org.elasticsearch.common.compress.CompressedXContent;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.unit.TimeValue;
-import org.elasticsearch.common.xcontent.XContentHelper;
-import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.core.internal.io.IOUtils;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.IndexService;
@@ -57,6 +55,7 @@
 import java.util.List;
 import java.util.Map;
 
+import static org.elasticsearch.index.mapper.MapperService.isMappingSourceTyped;
 import static org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason.NO_LONGER_ASSIGNED;
 
 /**
@@ -276,7 +275,7 @@ private ClusterState applyRequest(ClusterState currentState, PutMappingClusterSt
                 // try and parse it (no need to add it here) so we can bail early in case of parsing exception
                 DocumentMapper newMapper;
                 DocumentMapper existingMapper = mapperService.documentMapper(mappingType);
-                if (existingMapper == null && isMappingSourceTyped(mapperService, mappingUpdateSource, request.type()) == false) {
+                if (existingMapper == null && isMappingSourceTyped(request.type(), mappingUpdateSource) == false) {
                     existingMapper = getMapperForUpdate(mapperService, mappingType);
                 }
                 String typeForUpdate = existingMapper == null ? mappingType : existingMapper.type();
@@ -337,7 +336,7 @@ private ClusterState applyRequest(ClusterState currentState, PutMappingClusterSt
                 String typeForUpdate = mappingType;
                 CompressedXContent existingSource = null;
                 DocumentMapper existingMapper = mapperService.documentMapper(mappingType);
-                if (existingMapper == null && isMappingSourceTyped(mapperService, mappingUpdateSource, request.type()) == false) {
+                if (existingMapper == null && isMappingSourceTyped(request.type(), mappingUpdateSource) == false) {
                     existingMapper = getMapperForUpdate(mapperService, mappingType);
                 }
                 if (existingMapper != null) {
@@ -400,15 +399,6 @@ public String describeTasks(List<PutMappingClusterStateUpdateRequest> tasks) {
         }
     }
 
-    /**
-     * Returns {@code true} if the given {@code mappingSource} includes a type
-     * as a top-level object.
-     */
-    private static boolean isMappingSourceTyped(MapperService mapperService, CompressedXContent mappingSource, String type) {
-        Map<String, Object> root = XContentHelper.convertToMap(mappingSource.compressedReference(), true, XContentType.JSON).v2();
-        return root.size() == 1 && root.keySet().iterator().next().equals(type);
-    }
-
     public void putMapping(final PutMappingClusterStateUpdateRequest request, final ActionListener<ClusterStateUpdateResponse> listener) {
         clusterService.submitStateUpdateTask("put-mapping",
                 request,

server/src/main/java/org/elasticsearch/index/mapper/MapperService.java

@@ -40,6 +40,7 @@
 import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.XContentFactory;
+import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.index.AbstractIndexComponent;
@@ -725,6 +726,19 @@ public DocumentMapperForType documentMapperWithAutoCreate(String type) {
         return new DocumentMapperForType(mapper, mapper.mapping());
     }
 
+    /**
+     * Returns {@code true} if the given {@code mappingSource} includes a type
+     * as a top-level object.
+     */
+    public static boolean isMappingSourceTyped(String type, Map<String, Object> mapping) {
+        return mapping.size() == 1 && mapping.keySet().iterator().next().equals(type);
+    }
+
+    public static boolean isMappingSourceTyped(String type, CompressedXContent mappingSource) {
+        Map<String, Object> root = XContentHelper.convertToMap(mappingSource.compressedReference(), true, XContentType.JSON).v2();
+        return isMappingSourceTyped(type, root);
+    }
+
     /**
      * Returns the {@link MappedFieldType} for the give fullName.
      *

server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestCreateIndexAction.java

@@ -57,22 +57,16 @@ public String getName() {
 
     @Override
     public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
-        final boolean includeTypeName = request.paramAsBoolean(INCLUDE_TYPE_NAME_PARAMETER,
-            DEFAULT_INCLUDE_TYPE_NAME_POLICY);
-
         CreateIndexRequest createIndexRequest = new CreateIndexRequest(request.param("index"));
+
+        boolean includeTypeName = request.paramAsBoolean(INCLUDE_TYPE_NAME_PARAMETER,
+            DEFAULT_INCLUDE_TYPE_NAME_POLICY);
         if (request.hasContent()) {
             Map<String, Object> sourceAsMap = XContentHelper.convertToMap(request.content(), false, request.getXContentType()).v2();
-            if (sourceAsMap.containsKey("mappings")) {
-                if (includeTypeName == false) {
-                    Map<String, Object> newSourceAsMap = new HashMap<>(sourceAsMap);
-                    newSourceAsMap.put("mappings", Collections.singletonMap(
-                        MapperService.SINGLE_MAPPING_NAME, sourceAsMap.get("mappings")));
-                    sourceAsMap = newSourceAsMap;
-                } else {
-                    deprecationLogger.deprecatedAndMaybeLog("create_index_with_types", TYPES_DEPRECATION_MESSAGE);
-                }
+            if (includeTypeName && sourceAsMap.containsKey("mappings")) {
+                deprecationLogger.deprecatedAndMaybeLog("create_index_with_types", TYPES_DEPRECATION_MESSAGE);
             }
+            sourceAsMap = prepareMappings(sourceAsMap, includeTypeName);
             createIndexRequest.source(sourceAsMap, LoggingDeprecationHandler.INSTANCE);
         }
 
@@ -85,4 +79,25 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
         createIndexRequest.waitForActiveShards(ActiveShardCount.parseString(request.param("wait_for_active_shards")));
         return channel -> client.admin().indices().create(createIndexRequest, new RestToXContentListener<>(channel));
     }
+
+    static Map<String, Object> prepareMappings(Map<String, Object> source, boolean includeTypeName) {
+        if (includeTypeName
+            || source.containsKey("mappings") == false
+            || (source.get("mappings") instanceof Map) == false) {
+            return source;
+        }
+
+        Map<String, Object> newSource = new HashMap<>(source);
+
+        @SuppressWarnings("unchecked")
+        Map<String, Object> mappings = (Map<String, Object>) source.get("mappings");
+
+        if (MapperService.isMappingSourceTyped(MapperService.SINGLE_MAPPING_NAME, mappings)) {
+            throw new IllegalArgumentException("The mapping definition cannot be nested under a type " +
+                "[" + MapperService.SINGLE_MAPPING_NAME + "] unless include_type_name is set to true.");
+        }
+
+        newSource.put("mappings", Collections.singletonMap(MapperService.SINGLE_MAPPING_NAME, mappings));
+        return newSource;
+    }
 }

server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutIndexTemplateAction.java

@@ -26,7 +26,6 @@
 import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentHelper;
-import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.rest.BaseRestHandler;
 import org.elasticsearch.rest.RestController;
 import org.elasticsearch.rest.RestRequest;
@@ -35,7 +34,6 @@
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.Map;
 
 public class RestPutIndexTemplateAction extends BaseRestHandler {
@@ -72,24 +70,14 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
         putRequest.cause(request.param("cause", ""));
 
         boolean includeTypeName = request.paramAsBoolean(INCLUDE_TYPE_NAME_PARAMETER, DEFAULT_INCLUDE_TYPE_NAME_POLICY);
-        Map<String, Object> sourceAsMap = prepareRequestSource(request, includeTypeName);
-        putRequest.source(sourceAsMap);
-
-        return channel -> client.admin().indices().putTemplate(putRequest, new RestToXContentListener<>(channel));
-    }
-
-    Map<String, Object> prepareRequestSource(RestRequest request, boolean includeTypeName) {
         Map<String, Object> sourceAsMap = XContentHelper.convertToMap(request.requiredContent(), false,
             request.getXContentType()).v2();
-        if (includeTypeName == false && sourceAsMap.containsKey("mappings")) {
-            Map<String, Object> newSourceAsMap = new HashMap<>(sourceAsMap);
-            newSourceAsMap.put("mappings", Collections.singletonMap(MapperService.SINGLE_MAPPING_NAME, sourceAsMap.get("mappings")));
-            return newSourceAsMap;
-        } else {
-            if(includeTypeName && sourceAsMap.containsKey("mappings") ) {
-                DEPRECATION_LOGGER.deprecatedAndMaybeLog("put_index_template_with_types", TYPES_DEPRECATION_MESSAGE);                
-            }
-            return sourceAsMap;
+        if (includeTypeName && sourceAsMap.containsKey("mappings")) {
+            DEPRECATION_LOGGER.deprecatedAndMaybeLog("put_index_template_with_types", TYPES_DEPRECATION_MESSAGE);
         }
+        sourceAsMap = RestCreateIndexAction.prepareMappings(sourceAsMap, includeTypeName);
+        putRequest.source(sourceAsMap);
+
+        return channel -> client.admin().indices().putTemplate(putRequest, new RestToXContentListener<>(channel));
     }
 }