Consolidate field name validation when parsing mappings and documents (#91328)

#91043 surfaced some inconsistencies in field names validation between mapping parsing and document parsing. This commit centralizes the validation of field names when parsing mappings to a single place, and attempts to address some of the inconsistencies.

 - field names that contain only whitespaces are no longer accepted in mappings. It was previously possible to map a field containing only whitespaces but a document containing such a field would be rejected. We start rejecting mappings with fields that contain only whitespaces for indices that are created from 8.6 on, just in case existing indices contain such fields. This is true also for dotted fields like top. .foo when subobjects are enabled.

 - A clear error message is thrown when mappings hold fields with names made of dots only. An ArrayIndexOutOfBoundsException was thrown before

 - The error thrown when a field name is empty is now unified with that thrown when an empty field name is provided as part of a document (field name cannot be an empty string)

 - When parsing documents (with subobjects set to false), distinguish between the error thrown when a field name is empty and that thrown when a field name is made of whitespaces only

 - When parsing documents (with subobjects set to false), accept field names that are made of dots only (these are already accepted in mappings), effectively reverts #90950

docs/changelog/91328.yaml

@@ -0,0 +1,5 @@
+pr: 91328
+summary: Consolidate field name validation when parsing mappings and documents
+area: Mapping
+type: bug
+issues: []

modules/legacy-geo/src/main/java/org/elasticsearch/legacygeo/mapper/LegacyGeoShapeFieldMapper.java

@@ -361,10 +361,6 @@ public class LegacyGeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<
 
         @Override
         public LegacyGeoShapeFieldMapper build(MapperBuilderContext context) {
-            if (name.isEmpty()) {
-                // Check for an empty name early so we can throw a consistent error message
-                throw new IllegalArgumentException("name cannot be empty string");
-            }
             LegacyGeoShapeParser parser = new LegacyGeoShapeParser();
             GeoShapeFieldType ft = buildFieldType(parser, context);
             return new LegacyGeoShapeFieldMapper(name, ft, multiFieldsBuilder.build(this, context), copyTo.build(), parser, this);

modules/legacy-geo/src/test/java/org/elasticsearch/legacygeo/mapper/LegacyGeoShapeFieldMapperTests.java

@@ -108,15 +108,8 @@ public class LegacyGeoShapeFieldMapperTests extends MapperTestCase {
     }
 
     @Override
-    protected MapperService createMapperService(XContentBuilder mappings) throws IOException {
-        Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0);
-        return createMapperService(version, mappings);
-    }
-
-    @Override
-    protected MapperService createMapperService(Version version, XContentBuilder mapping) throws IOException {
-        assumeFalse("LegacyGeoShapeFieldMapper can't be created in version " + version, version.onOrAfter(Version.V_8_0_0));
-        return super.createMapperService(version, mapping);
+    protected Version getVersion() {
+        return VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0);
     }
 
     public void testLegacySwitches() throws IOException {

modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java

@@ -112,6 +112,7 @@ import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 
+//TODO migrate tests that don't require a node to a unit test that subclasses MapperTestCase
 public class PercolatorFieldMapperTests extends ESSingleNodeTestCase {
 
     private String fieldName;
@@ -823,7 +824,7 @@ public class PercolatorFieldMapperTests extends ESSingleNodeTestCase {
             MapperParsingException.class,
             () -> mapperService.parseMapping("type1", new CompressedXContent(mapping))
         );
-        assertThat(e.getMessage(), containsString("name cannot be empty string"));
+        assertThat(e.getMessage(), containsString("field name cannot be an empty string"));
     }
 
     public void testImplicitlySetDefaultScriptLang() throws Exception {

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

@@ -310,12 +310,12 @@ public final class DocumentParser {
             switch (token) {
                 case FIELD_NAME:
                     currentFieldName = parser.currentName();
+                    if (currentFieldName.isEmpty()) {
+                        throw new IllegalArgumentException("Field name cannot be an empty string");
+                    }
                     if (currentFieldName.isBlank()) {
                         throwFieldNameBlank(context, currentFieldName);
                     }
-                    if (currentFieldName.replace(".", "").length() == 0) {
-                        throwFieldNameOnlyDots();
-                    }
                     break;
                 case START_OBJECT:
                     parseObject(context, mapper, currentFieldName);
@@ -342,10 +342,6 @@ public final class DocumentParser {
         );
     }
 
-    private static void throwFieldNameOnlyDots() {
-        throw new IllegalArgumentException("field name cannot contain only dots");
-    }
-
     private static void throwEOF(ObjectMapper mapper, DocumentParserContext context) throws IOException {
         throw new MapperParsingException(
             "object mapping for ["

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

@@ -99,9 +99,8 @@ public abstract class FieldMapper extends Mapper implements Cloneable {
         String onScriptError
     ) {
         super(simpleName);
-        if (mappedFieldType.name().isEmpty()) {
-            throw new IllegalArgumentException("name cannot be empty string");
-        }
+        // could be blank but not empty on indices created < 8.6.0
+        assert mappedFieldType.name().isEmpty() == false;
         this.mappedFieldType = mappedFieldType;
         this.multiFields = multiFields;
         this.copyTo = Objects.requireNonNull(copyTo);

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

@@ -90,6 +90,9 @@ public final class MappingParser {
         if (type == null) {
             throw new MapperParsingException("Failed to derive type");
         }
+        if (type.isEmpty()) {
+            throw new MapperParsingException("type cannot be an empty string");
+        }
         return parse(type, mapping);
     }
 

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

@@ -252,6 +252,7 @@ public class ObjectMapper extends Mapper implements Cloneable {
             while (iterator.hasNext()) {
                 Map.Entry<String, Object> entry = iterator.next();
                 String fieldName = entry.getKey();
+                validateFieldName(fieldName, parserContext.indexVersionCreated());
                 // 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();
@@ -305,10 +306,16 @@ public class ObjectMapper extends Mapper implements Cloneable {
                         fieldBuilder = typeParser.parse(fieldName, propNode, parserContext);
                     } else {
                         String[] fieldNameParts = fieldName.split("\\.");
+                        if (fieldNameParts.length == 0) {
+                            throw new IllegalArgumentException("field name cannot contain only dots");
+                        }
                         String realFieldName = fieldNameParts[fieldNameParts.length - 1];
+                        validateFieldName(realFieldName, parserContext.indexVersionCreated());
                         fieldBuilder = typeParser.parse(realFieldName, propNode, parserContext);
                         for (int i = fieldNameParts.length - 2; i >= 0; --i) {
-                            ObjectMapper.Builder intermediate = new ObjectMapper.Builder(fieldNameParts[i], Defaults.SUBOBJECTS);
+                            String intermediateObjectName = fieldNameParts[i];
+                            validateFieldName(intermediateObjectName, parserContext.indexVersionCreated());
+                            ObjectMapper.Builder intermediate = new ObjectMapper.Builder(intermediateObjectName, Defaults.SUBOBJECTS);
                             intermediate.add(fieldBuilder);
                             fieldBuilder = intermediate;
                         }
@@ -330,6 +337,16 @@ public class ObjectMapper extends Mapper implements Cloneable {
         }
     }
 
+    private static void validateFieldName(String fieldName, Version indexCreatedVersion) {
+        if (fieldName.isEmpty()) {
+            throw new IllegalArgumentException("field name cannot be an empty string");
+        }
+        if (fieldName.isBlank() & indexCreatedVersion.onOrAfter(Version.V_8_6_0)) {
+            // blank field names were previously accepted in mappings, but not in documents.
+            throw new IllegalArgumentException("field name cannot contain only whitespaces");
+        }
+    }
+
     private final String fullPath;
 
     protected Explicit<Boolean> enabled;
@@ -347,9 +364,8 @@ public class ObjectMapper extends Mapper implements Cloneable {
         Map<String, Mapper> mappers
     ) {
         super(name);
-        if (name.isEmpty()) {
-            throw new IllegalArgumentException("name cannot be empty string");
-        }
+        // could be blank but not empty on indices created < 8.6.0
+        assert name.isEmpty() == false;
         this.fullPath = internFieldName(fullPath);
         this.enabled = enabled;
         this.subobjects = subobjects;

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

@@ -1918,16 +1918,17 @@ public class DocumentParserTests extends MapperServiceTestCase {
         });
     }
 
-    public void testBlankFieldNamesSubobjectsFalse() throws Exception {
+    public void testEmptyFieldNameSubobjectsFalse() throws Exception {
         DocumentMapper mapper = createDocumentMapper(mappingNoSubobjects(b -> {}));
-        {
-            MapperParsingException err = expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> b.field("", "foo"))));
-            assertThat(err.getMessage(), containsString("Field name cannot contain only whitespace: []"));
-        }
-        {
-            MapperParsingException err = expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> b.field("  ", "foo"))));
-            assertThat(err.getMessage(), containsString("Field name cannot contain only whitespace: [  ]"));
-        }
+        MapperParsingException err = expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> b.field("", "foo"))));
+        assertThat(err.getCause(), instanceOf(IllegalArgumentException.class));
+        assertThat(err.getCause().getMessage(), containsString("Field name cannot be an empty string"));
+    }
+
+    public void testBlankFieldNameSubobjectsFalse() throws Exception {
+        DocumentMapper mapper = createDocumentMapper(mappingNoSubobjects(b -> {}));
+        MapperParsingException err = expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> b.field("  ", "foo"))));
+        assertThat(err.getMessage(), containsString("Field name cannot contain only whitespace: [  ]"));
     }
 
     public void testDotsOnlyFieldNames() throws Exception {
@@ -1944,25 +1945,21 @@ public class DocumentParserTests extends MapperServiceTestCase {
         });
     }
 
-    public void testDotsOnlyFieldNamesSubobjectsFalse() throws Exception {
-        DocumentMapper mapper = createDocumentMapper(mappingNoSubobjects(b -> {}));
-        String[] fieldNames = { ".", "..", "..." };
-        for (String fieldName : fieldNames) {
-            MapperParsingException err = expectThrows(
-                MapperParsingException.class,
-                () -> mapper.parse(source(b -> b.field(fieldName, "bar")))
-            );
-            assertThat(err.getCause(), notNullValue());
-            // TODO this is actually accepted in the mappings, shall we revert https://github.com/elastic/elasticsearch/pull/90950 ?
-            assertThat(err.getCause().getMessage(), containsString("field name cannot contain only dots"));
-        }
-    }
+    // these combinations are not accepted by default, but they are when subobjects are disabled
+    public static final String[] VALID_FIELD_NAMES_NO_SUBOBJECTS = new String[] {
+        ".foo",
+        "foo.",
+        "top..foo",
+        "top.foo.",
+        "top..foo.",
+        "top. .foo",
+        ".",
+        "..",
+        "..." };
 
     public void testDynamicFieldEdgeCaseNamesSubobjectsFalse() throws Exception {
-        // these combinations are not accepted by default, but they are when subobjects are disabled
         MapperService mapperService = createMapperService(mappingNoSubobjects(b -> {}));
-        String[] fieldNames = new String[] { ".foo", "foo.", "top..foo", "top.foo.", "top..foo.", "top. .foo" };
-        for (String fieldName : fieldNames) {
+        for (String fieldName : VALID_FIELD_NAMES_NO_SUBOBJECTS) {
             ParsedDocument doc = mapperService.documentMapper().parse(source("{\"" + fieldName + "\":1}"));
             merge(mapperService, dynamicMapping(doc.dynamicMappingsUpdate()));
             assertNotNull(mapperService.fieldType(fieldName));

server/src/test/java/org/elasticsearch/index/mapper/MappingParserTests.java

@@ -17,6 +17,7 @@ import org.elasticsearch.index.analysis.IndexAnalyzers;
 import org.elasticsearch.index.similarity.SimilarityService;
 import org.elasticsearch.indices.IndicesModule;
 import org.elasticsearch.script.ScriptService;
+import org.elasticsearch.test.VersionUtils;
 import org.elasticsearch.xcontent.XContentBuilder;
 import org.hamcrest.CoreMatchers;
 
@@ -29,8 +30,12 @@ import java.util.function.Supplier;
 public class MappingParserTests extends MapperServiceTestCase {
 
     private static MappingParser createMappingParser(Settings settings) {
+        return createMappingParser(settings, Version.CURRENT);
+    }
+
+    private static MappingParser createMappingParser(Settings settings, Version version) {
         ScriptService scriptService = new ScriptService(settings, Collections.emptyMap(), Collections.emptyMap(), () -> 1L);
-        IndexSettings indexSettings = createIndexSettings(Version.CURRENT, settings);
+        IndexSettings indexSettings = createIndexSettings(version, settings);
         IndexAnalyzers indexAnalyzers = createIndexAnalyzers();
         SimilarityService similarityService = new SimilarityService(indexSettings, scriptService, Collections.emptyMap());
         MapperRegistry mapperRegistry = new IndicesModule(Collections.emptyList()).getMapperRegistry();
@@ -207,18 +212,22 @@ public class MappingParserTests extends MapperServiceTestCase {
         assertEquals("obj.source.geo.location", geoPointFieldMapper.mappedFieldType.name());
     }
 
+    private static String randomFieldType() {
+        return randomBoolean() ? KeywordFieldMapper.CONTENT_TYPE : ObjectMapper.CONTENT_TYPE;
+    }
+
     public void testFieldStartingWithDot() throws Exception {
-        XContentBuilder builder = mapping(b -> b.startObject(".foo").field("type", "keyword").endObject());
+        XContentBuilder builder = mapping(b -> b.startObject(".foo").field("type", randomFieldType()).endObject());
         IllegalArgumentException iae = expectThrows(
             IllegalArgumentException.class,
             () -> createMappingParser(Settings.EMPTY).parse("_doc", new CompressedXContent(BytesReference.bytes(builder)))
         );
         // TODO isn't this error misleading?
-        assertEquals("name cannot be empty string", iae.getMessage());
+        assertEquals("field name cannot be an empty string", iae.getMessage());
     }
 
     public void testFieldEndingWithDot() throws Exception {
-        XContentBuilder builder = mapping(b -> b.startObject("foo.").field("type", "keyword").endObject());
+        XContentBuilder builder = mapping(b -> b.startObject("foo.").field("type", randomFieldType()).endObject());
         Mapping mapping = createMappingParser(Settings.EMPTY).parse("_doc", new CompressedXContent(BytesReference.bytes(builder)));
         // TODO this needs fixing as part of addressing https://github.com/elastic/elasticsearch/issues/28948
         assertNotNull(mapping.getRoot().mappers.get("foo"));
@@ -226,17 +235,17 @@ public class MappingParserTests extends MapperServiceTestCase {
     }
 
     public void testFieldTrailingDots() throws Exception {
-        XContentBuilder builder = mapping(b -> b.startObject("top..foo").field("type", "keyword").endObject());
+        XContentBuilder builder = mapping(b -> b.startObject("top..foo").field("type", randomFieldType()).endObject());
         IllegalArgumentException iae = expectThrows(
             IllegalArgumentException.class,
             () -> createMappingParser(Settings.EMPTY).parse("_doc", new CompressedXContent(BytesReference.bytes(builder)))
         );
         // TODO isn't this error misleading?
-        assertEquals("name cannot be empty string", iae.getMessage());
+        assertEquals("field name cannot be an empty string", iae.getMessage());
     }
 
     public void testDottedFieldEndingWithDot() throws Exception {
-        XContentBuilder builder = mapping(b -> b.startObject("foo.bar.").field("type", "keyword").endObject());
+        XContentBuilder builder = mapping(b -> b.startObject("foo.bar.").field("type", randomFieldType()).endObject());
         Mapping mapping = createMappingParser(Settings.EMPTY).parse("_doc", new CompressedXContent(BytesReference.bytes(builder)));
         // TODO this needs fixing as part of addressing https://github.com/elastic/elasticsearch/issues/28948
         assertNotNull(((ObjectMapper) mapping.getRoot().mappers.get("foo")).mappers.get("bar"));
@@ -244,53 +253,81 @@ public class MappingParserTests extends MapperServiceTestCase {
     }
 
     public void testFieldStartingAndEndingWithDot() throws Exception {
-        XContentBuilder builder = mapping(b -> b.startObject("foo..bar.").field("type", "keyword").endObject());
+        XContentBuilder builder = mapping(b -> b.startObject("foo..bar.").field("type", randomFieldType()).endObject());
         IllegalArgumentException iae = expectThrows(
             IllegalArgumentException.class,
             () -> createMappingParser(Settings.EMPTY).parse("_doc", new CompressedXContent(BytesReference.bytes(builder)))
         );
         // TODO isn't this error misleading?
-        assertEquals("name cannot be empty string", iae.getMessage());
+        assertEquals("field name cannot be an empty string", iae.getMessage());
     }
 
     public void testDottedFieldWithTrailingWhitespace() throws Exception {
-        XContentBuilder builder = mapping(b -> b.startObject("top. .foo").field("type", "keyword").endObject());
-        Mapping mapping = createMappingParser(Settings.EMPTY).parse("_doc", new CompressedXContent(BytesReference.bytes(builder)));
-        ObjectMapper top = (ObjectMapper) mapping.getRoot().mappers.get("top");
-        // TODO this needs fixing? This field name is not allowed in documents when subobjects are enabled.
-        ObjectMapper mapper = (ObjectMapper) top.getMapper(" ");
-        assertNotNull(mapper.getMapper("foo"));
+        XContentBuilder builder = mapping(b -> b.startObject("top. .foo").field("type", randomFieldType()).endObject());
+        IllegalArgumentException iae = expectThrows(
+            IllegalArgumentException.class,
+            () -> createMappingParser(Settings.EMPTY).parse("_doc", new CompressedXContent(BytesReference.bytes(builder)))
+        );
+        // TODO isn't this error misleading?
+        assertEquals("field name cannot contain only whitespaces", iae.getMessage());
     }
 
     public void testEmptyFieldName() throws Exception {
         {
-            XContentBuilder builder = mapping(b -> b.startObject("").field("type", "keyword").endObject());
+            XContentBuilder builder = mapping(b -> b.startObject("").field("type", randomFieldType()).endObject());
             IllegalArgumentException iae = expectThrows(
                 IllegalArgumentException.class,
                 () -> createMappingParser(Settings.EMPTY).parse("_doc", new CompressedXContent(BytesReference.bytes(builder)))
             );
-            assertEquals("name cannot be empty string", iae.getMessage());
+            assertEquals("field name cannot be an empty string", iae.getMessage());
         }
         {
-            XContentBuilder builder = mappingNoSubobjects(b -> b.startObject("").field("type", "keyword").endObject());
+            XContentBuilder builder = mappingNoSubobjects(b -> b.startObject("").field("type", randomFieldType()).endObject());
             IllegalArgumentException iae = expectThrows(
                 IllegalArgumentException.class,
                 () -> createMappingParser(Settings.EMPTY).parse("_doc", new CompressedXContent(BytesReference.bytes(builder)))
             );
-            assertEquals("name cannot be empty string", iae.getMessage());
+            assertEquals("field name cannot be an empty string", iae.getMessage());
         }
     }
 
     public void testBlankFieldName() throws Exception {
-        // TODO this needs fixing? This field name is never allowed in documents hence such a field can never be indexed?
         {
-            XContentBuilder builder = mapping(b -> b.startObject(" ").field("type", "keyword").endObject());
-            Mapping mapping = createMappingParser(Settings.EMPTY).parse("_doc", new CompressedXContent(BytesReference.bytes(builder)));
+            XContentBuilder builder = mapping(b -> b.startObject(" ").field("type", randomFieldType()).endObject());
+            IllegalArgumentException iae = expectThrows(
+                IllegalArgumentException.class,
+                () -> createMappingParser(Settings.EMPTY).parse("_doc", new CompressedXContent(BytesReference.bytes(builder)))
+            );
+            assertEquals("field name cannot contain only whitespaces", iae.getMessage());
+        }
+        {
+            XContentBuilder builder = mappingNoSubobjects(b -> b.startObject(" ").field("type", "keyword").endObject());
+            IllegalArgumentException iae = expectThrows(
+                IllegalArgumentException.class,
+                () -> createMappingParser(Settings.EMPTY).parse("_doc", new CompressedXContent(BytesReference.bytes(builder)))
+            );
+            assertEquals("field name cannot contain only whitespaces", iae.getMessage());
+        }
+    }
+
+    public void testBlankFieldNameBefore8_6_0() throws Exception {
+        Version version = VersionUtils.randomVersionBetween(random(), Version.CURRENT.minimumIndexCompatibilityVersion(), Version.V_8_5_0);
+        {
+            XContentBuilder builder = mapping(b -> b.startObject(" ").field("type", randomFieldType()).endObject());
+            MappingParser mappingParser = createMappingParser(Settings.EMPTY, version);
+            Mapping mapping = mappingParser.parse("_doc", new CompressedXContent(BytesReference.bytes(builder)));
             assertNotNull(mapping.getRoot().getMapper(" "));
         }
+        {
+            XContentBuilder builder = mapping(b -> b.startObject("top. .foo").field("type", randomFieldType()).endObject());
+            MappingParser mappingParser = createMappingParser(Settings.EMPTY, version);
+            Mapping mapping = mappingParser.parse("_doc", new CompressedXContent(BytesReference.bytes(builder)));
+            assertNotNull(((ObjectMapper) mapping.getRoot().getMapper("top")).getMapper(" "));
+        }
         {
             XContentBuilder builder = mappingNoSubobjects(b -> b.startObject(" ").field("type", "keyword").endObject());
-            Mapping mapping = createMappingParser(Settings.EMPTY).parse("_doc", new CompressedXContent(BytesReference.bytes(builder)));
+            MappingParser mappingParser = createMappingParser(Settings.EMPTY, version);
+            Mapping mapping = mappingParser.parse("_doc", new CompressedXContent(BytesReference.bytes(builder)));
             assertNotNull(mapping.getRoot().getMapper(" "));
         }
     }
@@ -298,31 +335,19 @@ public class MappingParserTests extends MapperServiceTestCase {
     public void testFieldNameDotsOnly() throws Exception {
         String[] fieldNames = { ".", "..", "..." };
         for (String fieldName : fieldNames) {
-            XContentBuilder builder = mapping(b -> b.startObject(fieldName).field("type", "keyword").endObject());
-            // TODO this should really throw a better error, relates to https://github.com/elastic/elasticsearch/issues/21862
-            expectThrows(
-                ArrayIndexOutOfBoundsException.class,
+            XContentBuilder builder = mapping(b -> b.startObject(fieldName).field("type", randomFieldType()).endObject());
+            IllegalArgumentException iae = expectThrows(
+                IllegalArgumentException.class,
                 () -> createMappingParser(Settings.EMPTY).parse("_doc", new CompressedXContent(BytesReference.bytes(builder)))
             );
-        }
-    }
-
-    public void testFieldNameDotsOnlySubobjectsFalse() throws Exception {
-        String[] fieldNames = { ".", "..", "..." };
-        for (String fieldName : fieldNames) {
-            XContentBuilder builder = mappingNoSubobjects(b -> b.startObject(fieldName).field("type", "keyword").endObject());
-
-            createMappingParser(Settings.EMPTY).parse("_doc", new CompressedXContent(BytesReference.bytes(builder)));
+            assertEquals("field name cannot contain only dots", iae.getMessage());
         }
     }
 
     public void testDynamicFieldEdgeCaseNamesSubobjectsFalse() throws Exception {
-        // these combinations are not accepted by default, but they are when subobjects are disabled
-        String[] fieldNames = new String[] { ".foo", "foo.", "top..foo", "top.foo.", "top..foo.", "top. .foo" };
         MappingParser mappingParser = createMappingParser(Settings.EMPTY);
-        for (String fieldName : fieldNames) {
+        for (String fieldName : DocumentParserTests.VALID_FIELD_NAMES_NO_SUBOBJECTS) {
             XContentBuilder builder = mappingNoSubobjects(b -> b.startObject(fieldName).field("type", "keyword").endObject());
-            // TODO this is not accepted in documents, shall we revert https://github.com/elastic/elasticsearch/pull/90950 ?
             assertNotNull(mappingParser.parse("_doc", new CompressedXContent(BytesReference.bytes(builder))));
         }
     }
@@ -330,9 +355,8 @@ public class MappingParserTests extends MapperServiceTestCase {
     public void testDynamicFieldEdgeCaseNamesRuntimeSection() throws Exception {
         // TODO these combinations are not accepted by default, but they are in the runtime section, though they are not accepted when
         // parsing documents with subobjects enabled
-        String[] fieldNames = new String[] { ".foo", "foo.", "top..foo", "top.foo.", "top..foo.", "top. .foo" };
         MappingParser mappingParser = createMappingParser(Settings.EMPTY);
-        for (String fieldName : fieldNames) {
+        for (String fieldName : DocumentParserTests.VALID_FIELD_NAMES_NO_SUBOBJECTS) {
             XContentBuilder builder = runtimeMapping(b -> b.startObject(fieldName).field("type", "keyword").endObject());
             mappingParser.parse("_doc", new CompressedXContent(BytesReference.bytes(builder)));
         }

server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperTests.java

@@ -312,25 +312,6 @@ public class ObjectMapperTests extends MapperServiceTestCase {
         assertThat(e.getMessage(), containsString("can't merge a non object mapping [object.field1] with an object mapping"));
     }
 
-    public void testEmptyName() throws Exception {
-        String mapping = Strings.toString(
-            XContentFactory.jsonBuilder()
-                .startObject()
-                .startObject("")
-                .startObject("properties")
-                .startObject("name")
-                .field("type", "text")
-                .endObject()
-                .endObject()
-                .endObject()
-                .endObject()
-        );
-
-        // Empty name not allowed in index created after 5.0
-        Exception e = expectThrows(MapperParsingException.class, () -> createMapperService(mapping));
-        assertThat(e.getMessage(), containsString("name cannot be empty string"));
-    }
-
     public void testUnknownLegacyFields() throws Exception {
         MapperService service = createMapperService(Version.fromString("5.0.0"), Settings.EMPTY, () -> false, mapping(b -> {
             b.startObject("name");

server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java

@@ -18,6 +18,7 @@ import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collections;
 
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.instanceOf;
 
 public class RootObjectMapperTests extends MapperServiceTestCase {
@@ -337,4 +338,24 @@ public class RootObjectMapperTests extends MapperServiceTestCase {
         MapperParsingException e = expectThrows(MapperParsingException.class, () -> createMapperService(mapping));
         assertEquals("Failed to parse mapping: unknown parameter [unsupported] on runtime field [field] of type [keyword]", e.getMessage());
     }
+
+    public void testEmptyType() throws Exception {
+        String mapping = Strings.toString(
+            XContentFactory.jsonBuilder()
+                .startObject()
+                .startObject("")
+                .startObject("properties")
+                .startObject("name")
+                .field("type", "text")
+                .endObject()
+                .endObject()
+                .endObject()
+                .endObject()
+        );
+
+        // Empty name not allowed in index created after 5.0
+        Exception e = expectThrows(MapperParsingException.class, () -> createMapperService(mapping));
+        assertThat(e.getMessage(), containsString("type cannot be an empty string"));
+    }
+
 }

test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java

@@ -151,16 +151,20 @@ public abstract class MapperServiceTestCase extends ESTestCase {
         return mapperService.documentMapper();
     }
 
-    protected MapperService createMapperService(XContentBuilder mappings) throws IOException {
-        return createMapperService(Version.CURRENT, mappings);
+    protected final MapperService createMapperService(XContentBuilder mappings) throws IOException {
+        return createMapperService(getVersion(), mappings);
     }
 
-    protected MapperService createMapperService(Settings settings, XContentBuilder mappings) throws IOException {
-        return createMapperService(Version.CURRENT, settings, () -> true, mappings);
+    protected Version getVersion() {
+        return Version.CURRENT;
     }
 
-    protected MapperService createMapperService(BooleanSupplier idFieldEnabled, XContentBuilder mappings) throws IOException {
-        return createMapperService(Version.CURRENT, getIndexSettings(), idFieldEnabled, mappings);
+    protected final MapperService createMapperService(Settings settings, XContentBuilder mappings) throws IOException {
+        return createMapperService(getVersion(), settings, () -> true, mappings);
+    }
+
+    protected final MapperService createMapperService(BooleanSupplier idFieldEnabled, XContentBuilder mappings) throws IOException {
+        return createMapperService(getVersion(), getIndexSettings(), idFieldEnabled, mappings);
     }
 
     protected final MapperService createMapperService(String mappings) throws IOException {
@@ -175,7 +179,7 @@ public abstract class MapperServiceTestCase extends ESTestCase {
         return mapperService;
     }
 
-    protected MapperService createMapperService(Version version, XContentBuilder mapping) throws IOException {
+    protected final MapperService createMapperService(Version version, XContentBuilder mapping) throws IOException {
         return createMapperService(version, getIndexSettings(), () -> true, mapping);
     }
 

test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java

@@ -364,8 +364,18 @@ public abstract class MapperTestCase extends MapperServiceTestCase {
             minimalMapping(b);
             b.endObject();
         })));
-        assertThat(e.getMessage(), containsString("name cannot be empty string"));
-        assertParseMinimalWarnings();
+        assertThat(e.getMessage(), containsString("field name cannot be an empty string"));
+    }
+
+    public final void testBlankName() {
+        Version version = getVersion();
+        assumeTrue("blank field names are rejected from 8.6.0 onwards", version.onOrAfter(Version.V_8_6_0));
+        MapperParsingException e = expectThrows(MapperParsingException.class, () -> createMapperService(version, mapping(b -> {
+            b.startObject("  ");
+            minimalMapping(b);
+            b.endObject();
+        })));
+        assertThat(e.getMessage(), containsString("field name cannot contain only whitespaces"));
     }
 
     public final void testMinimalSerializesToItself() throws IOException {