From 566fef0cde01b8ce6576b21135c32f34133e33ac Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 7 Oct 2015 12:52:13 -0700 Subject: [PATCH] Mappings: Enforce metadata fields are not passed in documents We previously removed the ability to specify metadata fields inside documents in #11074, but the backcompat left leniency that allowed this to still occur. This change locks down parsing so any metadata field found while parsing a document results in an exception. This only affects 2.0+ indexes; backcompat is maintained. closes #13740 --- .../index/mapper/DocumentParser.java | 10 +++++++--- .../index/mapper/DynamicMappingTests.java | 2 +- .../mapper/all/SimpleAllMapperTests.java | 14 +++++++++++++ .../index/mapper/id/IdMappingTests.java | 13 ++++++++++++ .../mapper/parent/ParentMappingTests.java | 20 +++++++++---------- .../routing/RoutingTypeMapperTests.java | 16 ++++++++++++++- .../timestamp/TimestampMappingTests.java | 15 ++++++++++++++ .../index/mapper/ttl/TTLMappingTests.java | 15 ++++++++++++++ 8 files changed, 89 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java index db2919e0217..97435e039e1 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java @@ -122,7 +122,7 @@ class DocumentParser implements Closeable { // entire type is disabled parser.skipChildren(); } else if (emptyDoc == false) { - Mapper update = parseObject(context, mapping.root); + Mapper update = parseObject(context, mapping.root, true); if (update != null) { context.addDynamicMappingsUpdate(update); } @@ -194,7 +194,7 @@ class DocumentParser implements Closeable { return doc; } - static ObjectMapper parseObject(ParseContext context, ObjectMapper mapper) throws IOException { + static ObjectMapper parseObject(ParseContext context, ObjectMapper mapper, boolean atRoot) throws IOException { if (mapper.isEnabled() == false) { context.parser().skipChildren(); return null; @@ -202,6 +202,10 @@ class DocumentParser implements Closeable { XContentParser parser = context.parser(); String currentFieldName = parser.currentName(); + if (atRoot && MapperService.isMetadataField(currentFieldName) && + Version.indexCreated(context.indexSettings()).onOrAfter(Version.V_2_0_0_beta1)) { + throw new MapperParsingException("Field [" + currentFieldName + "] is a metadata field and cannot be added inside a document. Use the index API request parameters."); + } XContentParser.Token token = parser.currentToken(); if (token == XContentParser.Token.VALUE_NULL) { // the object is null ("obj1" : null), simply bail @@ -302,7 +306,7 @@ class DocumentParser implements Closeable { private static Mapper parseObjectOrField(ParseContext context, Mapper mapper) throws IOException { if (mapper instanceof ObjectMapper) { - return parseObject(context, (ObjectMapper) mapper); + return parseObject(context, (ObjectMapper) mapper, false); } else { FieldMapper fieldMapper = (FieldMapper)mapper; Mapper update = fieldMapper.parse(context); diff --git a/core/src/test/java/org/elasticsearch/index/mapper/DynamicMappingTests.java b/core/src/test/java/org/elasticsearch/index/mapper/DynamicMappingTests.java index f989467c0f2..ce71635492f 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/DynamicMappingTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/DynamicMappingTests.java @@ -197,7 +197,7 @@ public class DynamicMappingTests extends ESSingleNodeTestCase { ctx.reset(XContentHelper.createParser(source.source()), new ParseContext.Document(), source); assertEquals(XContentParser.Token.START_OBJECT, ctx.parser().nextToken()); ctx.parser().nextToken(); - return DocumentParser.parseObject(ctx, mapper.root()); + return DocumentParser.parseObject(ctx, mapper.root(), true); } public void testDynamicMappingsNotNeeded() throws Exception { diff --git a/core/src/test/java/org/elasticsearch/index/mapper/all/SimpleAllMapperTests.java b/core/src/test/java/org/elasticsearch/index/mapper/all/SimpleAllMapperTests.java index a7314c2d27f..0e3a04aa699 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/all/SimpleAllMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/all/SimpleAllMapperTests.java @@ -43,6 +43,7 @@ import org.elasticsearch.index.mapper.DocumentMapperParser; import org.elasticsearch.index.mapper.MapperParsingException; import org.elasticsearch.index.mapper.ParseContext.Document; import org.elasticsearch.index.mapper.ParsedDocument; +import org.elasticsearch.index.mapper.SourceToParse; import org.elasticsearch.index.mapper.internal.AllFieldMapper; import org.elasticsearch.test.ESSingleNodeTestCase; import org.elasticsearch.index.mapper.internal.TimestampFieldMapper; @@ -453,4 +454,17 @@ public class SimpleAllMapperTests extends ESSingleNodeTestCase { // the backcompat behavior is actually ignoring directly specifying _all assertFalse(field.getAllEntries().fields().iterator().hasNext()); } + + public void testIncludeInObjectNotAllowed() throws Exception { + String mapping = XContentFactory.jsonBuilder().startObject().startObject("type").endObject().endObject().string(); + DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping); + + try { + docMapper.parse("test", "type", "1", XContentFactory.jsonBuilder() + .startObject().field("_all", "foo").endObject().bytes()); + fail("Expected failure to parse metadata field"); + } catch (MapperParsingException e) { + assertTrue(e.getMessage(), e.getMessage().contains("Field [_all] is a metadata field and cannot be added inside a document")); + } + } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/id/IdMappingTests.java b/core/src/test/java/org/elasticsearch/index/mapper/id/IdMappingTests.java index 2688674f859..679b49e7be5 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/id/IdMappingTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/id/IdMappingTests.java @@ -114,4 +114,17 @@ public class IdMappingTests extends ESSingleNodeTestCase { // _id is not indexed so we need to check _uid assertEquals(Uid.createUid("type", "1"), doc.rootDoc().get(UidFieldMapper.NAME)); } + + public void testIncludeInObjectNotAllowed() throws Exception { + String mapping = XContentFactory.jsonBuilder().startObject().startObject("type").endObject().endObject().string(); + DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping); + + try { + docMapper.parse(SourceToParse.source(XContentFactory.jsonBuilder() + .startObject().field("_id", "1").endObject().bytes()).type("type")); + fail("Expected failure to parse metadata field"); + } catch (MapperParsingException e) { + assertTrue(e.getMessage(), e.getMessage().contains("Field [_id] is a metadata field and cannot be added inside a document")); + } + } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/parent/ParentMappingTests.java b/core/src/test/java/org/elasticsearch/index/mapper/parent/ParentMappingTests.java index bdfb0e475b4..3719500669c 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/parent/ParentMappingTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/parent/ParentMappingTests.java @@ -23,6 +23,7 @@ import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.index.mapper.DocumentMapper; +import org.elasticsearch.index.mapper.MapperParsingException; import org.elasticsearch.index.mapper.ParsedDocument; import org.elasticsearch.index.mapper.SourceToParse; import org.elasticsearch.index.mapper.Uid; @@ -32,21 +33,18 @@ import static org.hamcrest.Matchers.nullValue; public class ParentMappingTests extends ESSingleNodeTestCase { - public void testParentNotSet() throws Exception { + public void testParentSetInDocNotAllowed() throws Exception { String mapping = XContentFactory.jsonBuilder().startObject().startObject("type") .endObject().endObject().string(); DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping); - ParsedDocument doc = docMapper.parse(SourceToParse.source(XContentFactory.jsonBuilder() - .startObject() - .field("_parent", "1122") - .field("x_field", "x_value") - .endObject() - .bytes()).type("type").id("1")); - - // no _parent mapping, dynamically used as a string field - assertNull(doc.parent()); - assertNotNull(doc.rootDoc().get("_parent")); + try { + docMapper.parse(SourceToParse.source(XContentFactory.jsonBuilder() + .startObject().field("_parent", "1122").endObject().bytes()).type("type").id("1")); + fail("Expected failure to parse metadata field"); + } catch (MapperParsingException e) { + assertTrue(e.getMessage(), e.getMessage().contains("Field [_parent] is a metadata field and cannot be added inside a document")); + } } public void testParentSetInDocBackcompat() throws Exception { diff --git a/core/src/test/java/org/elasticsearch/index/mapper/routing/RoutingTypeMapperTests.java b/core/src/test/java/org/elasticsearch/index/mapper/routing/RoutingTypeMapperTests.java index 30fcb5f3d6d..7d0afdb0724 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/routing/RoutingTypeMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/routing/RoutingTypeMapperTests.java @@ -32,6 +32,7 @@ import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.index.mapper.DocumentMapper; +import org.elasticsearch.index.mapper.MapperParsingException; import org.elasticsearch.index.mapper.ParsedDocument; import org.elasticsearch.index.mapper.SourceToParse; import org.elasticsearch.test.ESSingleNodeTestCase; @@ -113,7 +114,7 @@ public class RoutingTypeMapperTests extends ESSingleNodeTestCase { Settings settings = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.V_1_4_2.id).build(); DocumentMapper docMapper = createIndex("test", settings).mapperService().documentMapperParser().parse(mapping); - XContentBuilder doc = XContentFactory.jsonBuilder().startObject().field("_timestamp", 2000000).endObject(); + XContentBuilder doc = XContentFactory.jsonBuilder().startObject().field("_routing", "foo").endObject(); MappingMetaData mappingMetaData = new MappingMetaData(docMapper); IndexRequest request = new IndexRequest("test", "type", "1").source(doc); request.process(MetaData.builder().build(), mappingMetaData, true, "test"); @@ -122,4 +123,17 @@ public class RoutingTypeMapperTests extends ESSingleNodeTestCase { assertNull(request.routing()); assertNull(docMapper.parse("test", "type", "1", doc.bytes()).rootDoc().get("_routing")); } + + public void testIncludeInObjectNotAllowed() throws Exception { + String mapping = XContentFactory.jsonBuilder().startObject().startObject("type").endObject().endObject().string(); + DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping); + + try { + docMapper.parse("test", "type", "1", XContentFactory.jsonBuilder() + .startObject().field("_routing", "foo").endObject().bytes()); + fail("Expected failure to parse metadata field"); + } catch (MapperParsingException e) { + assertTrue(e.getMessage(), e.getMessage().contains("Field [_routing] is a metadata field and cannot be added inside a document")); + } + } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/timestamp/TimestampMappingTests.java b/core/src/test/java/org/elasticsearch/index/mapper/timestamp/TimestampMappingTests.java index 057dc41f0f9..e51b6a61d50 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/timestamp/TimestampMappingTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/timestamp/TimestampMappingTests.java @@ -769,6 +769,21 @@ public class TimestampMappingTests extends ESSingleNodeTestCase { assertNull(docMapper.parse("test", "type", "1", doc.bytes()).rootDoc().get("_timestamp")); } + public void testIncludeInObjectNotAllowed() throws Exception { + String mapping = XContentFactory.jsonBuilder().startObject().startObject("type") + .startObject("_timestamp").field("enabled", true).field("default", "1970").field("format", "YYYY").endObject() + .endObject().endObject().string(); + DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping); + + try { + docMapper.parse("test", "type", "1", XContentFactory.jsonBuilder() + .startObject().field("_timestamp", 2000000).endObject().bytes()); + fail("Expected failure to parse metadata field"); + } catch (MapperParsingException e) { + assertTrue(e.getMessage(), e.getMessage().contains("Field [_timestamp] is a metadata field and cannot be added inside a document")); + } + } + public void testThatEpochCanBeIgnoredWithCustomFormat() throws Exception { String mapping = XContentFactory.jsonBuilder().startObject().startObject("type") .startObject("_timestamp").field("enabled", true).field("format", "yyyyMMddHH").endObject() diff --git a/core/src/test/java/org/elasticsearch/index/mapper/ttl/TTLMappingTests.java b/core/src/test/java/org/elasticsearch/index/mapper/ttl/TTLMappingTests.java index c9b6131900f..b9f7a988788 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/ttl/TTLMappingTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/ttl/TTLMappingTests.java @@ -310,6 +310,21 @@ public class TTLMappingTests extends ESSingleNodeTestCase { assertNull(docMapper.parse("test", "type", "1", doc.bytes()).rootDoc().get("_ttl")); } + public void testIncludeInObjectNotAllowed() throws Exception { + String mapping = XContentFactory.jsonBuilder().startObject().startObject("type") + .startObject("_ttl").field("enabled", true).endObject() + .endObject().endObject().string(); + DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping); + + try { + docMapper.parse("test", "type", "1", XContentFactory.jsonBuilder() + .startObject().field("_ttl", "2d").endObject().bytes()); + fail("Expected failure to parse metadata field"); + } catch (MapperParsingException e) { + assertTrue(e.getMessage(), e.getMessage().contains("Field [_ttl] is a metadata field and cannot be added inside a document")); + } + } + private org.elasticsearch.common.xcontent.XContentBuilder getMappingWithTtlEnabled() throws IOException { return getMappingWithTtlEnabled(null); }