Version-guard checking for lossy params in _source (#108460)

elastic · May 9, 2024 · 4dcbc3b · 4dcbc3b
1 parent e178684
commit 4dcbc3b
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 6 deletions.
diff --git a/server/src/main/java/org/elasticsearch/index/IndexVersions.java b/server/src/main/java/org/elasticsearch/index/IndexVersions.java
@@ -105,6 +105,7 @@ private static IndexVersion def(int id, Version luceneVersion) {
     public static final IndexVersion TIME_SERIES_ROUTING_HASH_IN_ID = def(8_504_00_0, Version.LUCENE_9_10_0);
     public static final IndexVersion DEFAULT_DENSE_VECTOR_TO_INT8_HNSW = def(8_505_00_0, Version.LUCENE_9_10_0);
     public static final IndexVersion DOC_VALUES_FOR_IGNORED_META_FIELD = def(8_505_00_1, Version.LUCENE_9_10_0);
+    public static final IndexVersion SOURCE_MAPPER_LOSSY_PARAMS_CHECK = def(8_506_00_0, Version.LUCENE_9_10_0);
 
     /*
      * STOP! READ THIS FIRST! No, really,

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java
@@ -134,10 +134,11 @@ public static class Builder extends MetadataFieldMapper.Builder {
 
         private final boolean supportsNonDefaultParameterValues;
 
-        public Builder(IndexMode indexMode, final Settings settings) {
+        public Builder(IndexMode indexMode, final Settings settings, boolean supportsCheckForNonDefaultParams) {
             super(Defaults.NAME);
             this.indexMode = indexMode;
-            this.supportsNonDefaultParameterValues = settings.getAsBoolean(LOSSY_PARAMETERS_ALLOWED_SETTING_NAME, true);
+            this.supportsNonDefaultParameterValues = supportsCheckForNonDefaultParams == false
+                || settings.getAsBoolean(LOSSY_PARAMETERS_ALLOWED_SETTING_NAME, true);
         }
 
         public Builder setSynthetic() {
@@ -212,7 +213,11 @@ public SourceFieldMapper build() {
         c -> c.getIndexSettings().getMode() == IndexMode.TIME_SERIES
             ? c.getIndexSettings().getIndexVersionCreated().onOrAfter(IndexVersions.V_8_7_0) ? TSDB_DEFAULT : TSDB_LEGACY_DEFAULT
             : DEFAULT,
-        c -> new Builder(c.getIndexSettings().getMode(), c.getSettings())
+        c -> new Builder(
+            c.getIndexSettings().getMode(),
+            c.getSettings(),
+            c.indexVersionCreated().onOrAfter(IndexVersions.SOURCE_MAPPER_LOSSY_PARAMS_CHECK)
+        )
     );
 
     static final class SourceFieldType extends MappedFieldType {
@@ -347,7 +352,7 @@ protected String contentType() {
 
     @Override
     public FieldMapper.Builder getMergeBuilder() {
-        return new Builder(indexMode, Settings.EMPTY).init(this);
+        return new Builder(indexMode, Settings.EMPTY, false).init(this);
     }
 
     /**

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DynamicFieldsBuilderTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DynamicFieldsBuilderTests.java
@@ -68,7 +68,7 @@ public void testCreateDynamicStringFieldAsKeywordForDimension() throws IOExcepti
         XContentParser parser = createParser(JsonXContent.jsonXContent, source);
         SourceToParse sourceToParse = new SourceToParse("test", new BytesArray(source), XContentType.JSON);
 
-        SourceFieldMapper sourceMapper = new SourceFieldMapper.Builder(null, Settings.EMPTY).setSynthetic().build();
+        SourceFieldMapper sourceMapper = new SourceFieldMapper.Builder(null, Settings.EMPTY, false).setSynthetic().build();
         RootObjectMapper root = new RootObjectMapper.Builder("_doc", Explicit.IMPLICIT_TRUE).add(
             new PassThroughObjectMapper.Builder("labels").setContainsDimensions().dynamic(ObjectMapper.Dynamic.TRUE)
         ).build(MapperBuilderContext.root(false, false));

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java
@@ -13,6 +13,8 @@
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentHelper;
+import org.elasticsearch.index.IndexVersions;
+import org.elasticsearch.test.index.IndexVersionUtils;
 import org.elasticsearch.xcontent.XContentBuilder;
 import org.elasticsearch.xcontent.XContentFactory;
 import org.elasticsearch.xcontent.XContentParser;
@@ -298,4 +300,44 @@ public void testSupportsNonDefaultParameterValues() throws IOException {
         );
         assertThat(e.getMessage(), containsString("Parameters [enabled,includes,excludes] are not allowed in source"));
     }
+
+    public void testBypassCheckForNonDefaultParameterValuesInEarlierVersions() throws IOException {
+        Settings settings = Settings.builder().put(SourceFieldMapper.LOSSY_PARAMETERS_ALLOWED_SETTING_NAME, false).build();
+        {
+            var sourceFieldMapper = createMapperService(
+                IndexVersionUtils.getPreviousVersion(IndexVersions.SOURCE_MAPPER_LOSSY_PARAMS_CHECK),
+                settings,
+                () -> true,
+                topMapping(b -> b.startObject("_source").field("enabled", false).endObject())
+            ).documentMapper().sourceMapper();
+            assertThat(sourceFieldMapper, notNullValue());
+        }
+        {
+            var sourceFieldMapper = createMapperService(
+                IndexVersionUtils.getPreviousVersion(IndexVersions.SOURCE_MAPPER_LOSSY_PARAMS_CHECK),
+                settings,
+                () -> true,
+                topMapping(b -> b.startObject("_source").array("includes", "foo").endObject())
+            ).documentMapper().sourceMapper();
+            assertThat(sourceFieldMapper, notNullValue());
+        }
+        {
+            var sourceFieldMapper = createMapperService(
+                IndexVersionUtils.getPreviousVersion(IndexVersions.SOURCE_MAPPER_LOSSY_PARAMS_CHECK),
+                settings,
+                () -> true,
+                topMapping(b -> b.startObject("_source").array("excludes", "foo").endObject())
+            ).documentMapper().sourceMapper();
+            assertThat(sourceFieldMapper, notNullValue());
+        }
+        {
+            var sourceFieldMapper = createMapperService(
+                IndexVersionUtils.getPreviousVersion(IndexVersions.SOURCE_MAPPER_LOSSY_PARAMS_CHECK),
+                settings,
+                () -> true,
+                topMapping(b -> b.startObject("_source").field("mode", "disabled").endObject())
+            ).documentMapper().sourceMapper();
+            assertThat(sourceFieldMapper, notNullValue());
+        }
+    }
 }
diff --git a/server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java b/server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java
@@ -382,7 +382,7 @@ public void testSearchRequestRuntimeFieldsAndMultifieldDetection() {
 
     public void testSyntheticSourceSearchLookup() throws IOException {
         // Build a mapping using synthetic source
-        SourceFieldMapper sourceMapper = new SourceFieldMapper.Builder(null, Settings.EMPTY).setSynthetic().build();
+        SourceFieldMapper sourceMapper = new SourceFieldMapper.Builder(null, Settings.EMPTY, false).setSynthetic().build();
         RootObjectMapper root = new RootObjectMapper.Builder("_doc", Explicit.IMPLICIT_TRUE).add(
             new KeywordFieldMapper.Builder("cat", IndexVersion.current()).ignoreAbove(100)
         ).build(MapperBuilderContext.root(true, false));