From a041224c55374dc069db46a1fb967c9e19b6325f Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Wed, 9 Jul 2025 15:34:56 +0200 Subject: [PATCH 1/3] Check field data type before casting when applying geo distance sort If a user tries to apply geo distance sorting to a field of the wrong type, they'll get a 500 error that causes shard failures, because the field data impl gets casted to the expected type without first checking that the field type is the one expected. This commit addresses that by returning a 400 error instead. Closes #129500 --- .../search/sort/GeoDistanceSortBuilder.java | 8 +++++++- .../sort/GeoDistanceSortBuilderTests.java | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java index 61d20fa72e262..b0c7c03179b21 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java @@ -38,6 +38,7 @@ import org.elasticsearch.index.fielddata.NumericDoubleValues; import org.elasticsearch.index.fielddata.SortedNumericDoubleValues; import org.elasticsearch.index.fielddata.plain.LatLonPointIndexFieldData; +import org.elasticsearch.index.mapper.GeoPointScriptFieldType; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.query.GeoValidationMethod; import org.elasticsearch.index.query.QueryBuilder; @@ -599,7 +600,12 @@ private IndexGeoPointFieldData fieldData(SearchExecutionContext context) { throw new IllegalArgumentException("failed to find mapper for [" + fieldName + "] for geo distance based sort"); } } - return context.getForField(fieldType, MappedFieldType.FielddataOperation.SEARCH); + IndexFieldData indexFieldData = context.getForField(fieldType, MappedFieldType.FielddataOperation.SEARCH); + if (indexFieldData instanceof IndexGeoPointFieldData) { + return (IndexGeoPointFieldData) indexFieldData; + } + throw new IllegalArgumentException("unable to apply geo distance sort to field [" + fieldName + + "] of type [" + fieldType.typeName() + "]"); } private Nested nested(SearchExecutionContext context) throws IOException { diff --git a/server/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java b/server/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java index 17a9fb5974176..be798b74f3c37 100644 --- a/server/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java @@ -18,11 +18,16 @@ import org.elasticsearch.common.geo.GeoDistance; import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.unit.DistanceUnit; +import org.elasticsearch.index.IndexMode; +import org.elasticsearch.index.IndexVersion; import org.elasticsearch.index.fielddata.IndexFieldData.XFieldComparatorSource; import org.elasticsearch.index.fielddata.IndexFieldData.XFieldComparatorSource.Nested; import org.elasticsearch.index.mapper.GeoPointFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; +import org.elasticsearch.index.mapper.Mapper; +import org.elasticsearch.index.mapper.MapperBuilderContext; import org.elasticsearch.index.mapper.NestedPathFieldMapper; +import org.elasticsearch.index.mapper.NumberFieldMapper; import org.elasticsearch.index.query.GeoValidationMethod; import org.elasticsearch.index.query.MatchAllQueryBuilder; import org.elasticsearch.index.query.MatchNoneQueryBuilder; @@ -30,6 +35,7 @@ import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.index.query.RangeQueryBuilder; import org.elasticsearch.index.query.SearchExecutionContext; +import org.elasticsearch.script.ScriptCompiler; import org.elasticsearch.search.DocValueFormat; import org.elasticsearch.search.MultiValueMode; import org.elasticsearch.test.geo.RandomGeoGenerator; @@ -99,6 +105,9 @@ public static GeoDistanceSortBuilder randomGeoDistanceSortBuilder() { @Override protected MappedFieldType provideMappedFieldType(String name) { + if (name.equals("double")) { + return new NumberFieldMapper.NumberFieldType(name, NumberFieldMapper.NumberType.DOUBLE); + } return new GeoPointFieldMapper.GeoPointFieldType(name); } @@ -531,6 +540,15 @@ public void testBuildInvalidPoints() throws IOException { ); assertEquals("illegal longitude value [-360.0] for [GeoDistanceSort] for field [fieldName].", ex.getMessage()); } + { + GeoDistanceSortBuilder sortBuilder = new GeoDistanceSortBuilder("double", 0.0, 180.0); + sortBuilder.validation(GeoValidationMethod.STRICT); + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> sortBuilder.build(searchExecutionContext) + ); + assertEquals("unable to apply geo distance sort to field [double] of type [double]", ex.getMessage()); + } } /** From d7ae391126e93102a7d323903fa07cfc28a70a1c Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Wed, 9 Jul 2025 15:38:15 +0200 Subject: [PATCH 2/3] Update docs/changelog/130924.yaml --- docs/changelog/130924.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 docs/changelog/130924.yaml diff --git a/docs/changelog/130924.yaml b/docs/changelog/130924.yaml new file mode 100644 index 0000000000000..09b0f3b90533c --- /dev/null +++ b/docs/changelog/130924.yaml @@ -0,0 +1,6 @@ +pr: 130924 +summary: Check field data type before casting when applying geo distance sort +area: Search +type: bug +issues: + - 129500 From a48559802a56f7dfa5df63c5b5381beb5f023a31 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Wed, 9 Jul 2025 13:51:38 +0000 Subject: [PATCH 3/3] [CI] Auto commit changes from spotless --- .../search/sort/GeoDistanceSortBuilder.java | 6 +++--- .../search/sort/GeoDistanceSortBuilderTests.java | 10 +--------- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java index b0c7c03179b21..3c5033355b826 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java @@ -38,7 +38,6 @@ import org.elasticsearch.index.fielddata.NumericDoubleValues; import org.elasticsearch.index.fielddata.SortedNumericDoubleValues; import org.elasticsearch.index.fielddata.plain.LatLonPointIndexFieldData; -import org.elasticsearch.index.mapper.GeoPointScriptFieldType; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.query.GeoValidationMethod; import org.elasticsearch.index.query.QueryBuilder; @@ -604,8 +603,9 @@ private IndexGeoPointFieldData fieldData(SearchExecutionContext context) { if (indexFieldData instanceof IndexGeoPointFieldData) { return (IndexGeoPointFieldData) indexFieldData; } - throw new IllegalArgumentException("unable to apply geo distance sort to field [" + fieldName + - "] of type [" + fieldType.typeName() + "]"); + throw new IllegalArgumentException( + "unable to apply geo distance sort to field [" + fieldName + "] of type [" + fieldType.typeName() + "]" + ); } private Nested nested(SearchExecutionContext context) throws IOException { diff --git a/server/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java b/server/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java index be798b74f3c37..5bef1f4769cff 100644 --- a/server/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java @@ -18,14 +18,10 @@ import org.elasticsearch.common.geo.GeoDistance; import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.unit.DistanceUnit; -import org.elasticsearch.index.IndexMode; -import org.elasticsearch.index.IndexVersion; import org.elasticsearch.index.fielddata.IndexFieldData.XFieldComparatorSource; import org.elasticsearch.index.fielddata.IndexFieldData.XFieldComparatorSource.Nested; import org.elasticsearch.index.mapper.GeoPointFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; -import org.elasticsearch.index.mapper.Mapper; -import org.elasticsearch.index.mapper.MapperBuilderContext; import org.elasticsearch.index.mapper.NestedPathFieldMapper; import org.elasticsearch.index.mapper.NumberFieldMapper; import org.elasticsearch.index.query.GeoValidationMethod; @@ -35,7 +31,6 @@ import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.index.query.RangeQueryBuilder; import org.elasticsearch.index.query.SearchExecutionContext; -import org.elasticsearch.script.ScriptCompiler; import org.elasticsearch.search.DocValueFormat; import org.elasticsearch.search.MultiValueMode; import org.elasticsearch.test.geo.RandomGeoGenerator; @@ -543,10 +538,7 @@ public void testBuildInvalidPoints() throws IOException { { GeoDistanceSortBuilder sortBuilder = new GeoDistanceSortBuilder("double", 0.0, 180.0); sortBuilder.validation(GeoValidationMethod.STRICT); - IllegalArgumentException ex = expectThrows( - IllegalArgumentException.class, - () -> sortBuilder.build(searchExecutionContext) - ); + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> sortBuilder.build(searchExecutionContext)); assertEquals("unable to apply geo distance sort to field [double] of type [double]", ex.getMessage()); } }