From 408d9bb4a592a203e82ebddff161c5f7012f6dca Mon Sep 17 00:00:00 2001 From: Matthew Alp Date: Thu, 23 Oct 2025 11:26:41 -0400 Subject: [PATCH 1/6] Validate ancestry of paths provided to reverse nested aggs in _search --- .../ReverseNestedAggregationBuilder.java | 12 +++++++++ .../nested/ReverseNestedAggregatorTests.java | 25 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/nested/ReverseNestedAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/nested/ReverseNestedAggregationBuilder.java index 3d10e03ce9c0c..e631b9b1648f8 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/nested/ReverseNestedAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/nested/ReverseNestedAggregationBuilder.java @@ -84,6 +84,18 @@ protected AggregatorFactory doBuild(AggregationContext context, AggregatorFactor throw new IllegalArgumentException("Reverse nested aggregation [" + name + "] can only be used inside a [nested] aggregation"); } + if (path != null) { + NestedObjectMapper currentParent = context.nestedScope().getObjectMapper(); + if (currentParent != null) { + String parentPath = currentParent.fullPath(); + if (parentPath.equals(path) == false && parentPath.startsWith(path + ".") == false) { + throw new IllegalArgumentException( + "Reverse nested path [" + path + "] is not a parent of the current nested scope [" + parentPath + "]" + ); + } + } + } + NestedObjectMapper nestedMapper = null; if (path != null) { nestedMapper = context.nestedLookup().getNestedMappers().get(path); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/ReverseNestedAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/ReverseNestedAggregatorTests.java index 6eb88dbcf24cb..cd1b3f34b0e50 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/ReverseNestedAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/ReverseNestedAggregatorTests.java @@ -247,4 +247,29 @@ public void testNestedUnderTerms() throws IOException { protected List objectMappers() { return NestedAggregatorTests.MOCK_OBJECT_MAPPERS; } + + public void testErrorOnReverseNestedInReverseNestedWithPath() throws IOException { + AggregationBuilder aggregationBuilder = nested("n1", NESTED_OBJECT).subAggregation( + nested("n2", NESTED_OBJECT + ".child").subAggregation( + reverseNested("r1").path(NESTED_OBJECT).subAggregation(reverseNested("r2").path(NESTED_OBJECT + ".child")) + ) + ); + + try (Directory directory = newDirectory()) { + try (RandomIndexWriter iw = newRandomIndexWriterWithLogDocMergePolicy(directory)) { + // No docs needed + } + try (DirectoryReader indexReader = wrapInMockESDirectoryReader(DirectoryReader.open(directory))) { + IllegalArgumentException e = assertThrows( + IllegalArgumentException.class, + () -> searchAndReduce(indexReader, new AggTestConfig(aggregationBuilder)) + ); + assertThat( + e.getMessage(), + equalTo("Reverse nested path [nested_object.child] is not a parent of the current nested scope [nested_object]") + ); + + } + } + } } From e14576c31a8ab2e961ed9c3cf1d112bbb54371ac Mon Sep 17 00:00:00 2001 From: Matthew Alp Date: Thu, 23 Oct 2025 11:38:34 -0400 Subject: [PATCH 2/6] Change test name --- .../bucket/nested/ReverseNestedAggregatorTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/ReverseNestedAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/ReverseNestedAggregatorTests.java index cd1b3f34b0e50..497637eda76a9 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/ReverseNestedAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/ReverseNestedAggregatorTests.java @@ -248,7 +248,7 @@ protected List objectMappers() { return NestedAggregatorTests.MOCK_OBJECT_MAPPERS; } - public void testErrorOnReverseNestedInReverseNestedWithPath() throws IOException { + public void testErrorOnInvalidReverseNestedWithPath() throws IOException { AggregationBuilder aggregationBuilder = nested("n1", NESTED_OBJECT).subAggregation( nested("n2", NESTED_OBJECT + ".child").subAggregation( reverseNested("r1").path(NESTED_OBJECT).subAggregation(reverseNested("r2").path(NESTED_OBJECT + ".child")) From 7a4aebba1978d71ab11d2a22a9f7c485a2858c31 Mon Sep 17 00:00:00 2001 From: Matt Date: Thu, 23 Oct 2025 11:41:28 -0400 Subject: [PATCH 3/6] Update docs/changelog/137047.yaml --- docs/changelog/137047.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/137047.yaml diff --git a/docs/changelog/137047.yaml b/docs/changelog/137047.yaml new file mode 100644 index 0000000000000..e696bf0da3721 --- /dev/null +++ b/docs/changelog/137047.yaml @@ -0,0 +1,5 @@ +pr: 137047 +summary: Reject invalid `reverse_nested` aggs +area: Aggregations +type: bug +issues: [] From 66baa3a1ca877210afe8688fd8eac5b824efdbe5 Mon Sep 17 00:00:00 2001 From: Matthew Alp Date: Fri, 24 Oct 2025 10:44:15 -0400 Subject: [PATCH 4/6] Add YAML test for correct handling of invalid reverse-nested aggs --- .../test/aggregations/nested.yml | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/modules/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/nested.yml b/modules/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/nested.yml index 1372908760d65..b59ae7ceed177 100644 --- a/modules/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/nested.yml +++ b/modules/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/nested.yml @@ -194,3 +194,26 @@ setup: - match: { aggregations.courses.highpass_filter.unnest.department.buckets.0.doc_count: 1 } - match: { aggregations.courses.highpass_filter.unnest.department.buckets.1.key: math } - match: { aggregations.courses.highpass_filter.unnest.department.buckets.1.doc_count: 1 } +--- +"Illegal reverse nested aggregation to a child nested object": + - do: + catch: /Reverse nested path \[courses.sessions\] is not a parent of the current nested scope \[courses\]/ + search: + index: test + body: + { + "aggs": { + "parent_nested": { + "nested": { + "path": "courses" + }, + "aggs": { + "invalid_reverse_nested": { + "reverse_nested": { + "path": "courses.sessions" + } + } + } + } + } + } From 49ce5bb0bf0ff98540be6aeaf1f88b5a50faf493 Mon Sep 17 00:00:00 2001 From: Matthew Alp Date: Fri, 24 Oct 2025 13:30:38 -0400 Subject: [PATCH 5/6] Add capability for rejecting invalid nesting aggs --- .../resources/rest-api-spec/test/aggregations/nested.yml | 7 +++++++ .../rest/action/search/SearchCapabilities.java | 2 ++ 2 files changed, 9 insertions(+) diff --git a/modules/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/nested.yml b/modules/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/nested.yml index b59ae7ceed177..a793315ab956a 100644 --- a/modules/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/nested.yml +++ b/modules/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/nested.yml @@ -196,6 +196,13 @@ setup: - match: { aggregations.courses.highpass_filter.unnest.department.buckets.1.doc_count: 1 } --- "Illegal reverse nested aggregation to a child nested object": + - requires: + capabilities: + - method: POST + path: /_search + capabilities: [ reject_invalid_reverse_nesting ] + test_runner_features: [ capabilities ] + reason: "search does not yet reject invalid reverse nesting paths" - do: catch: /Reverse nested path \[courses.sessions\] is not a parent of the current nested scope \[courses\]/ search: diff --git a/server/src/main/java/org/elasticsearch/rest/action/search/SearchCapabilities.java b/server/src/main/java/org/elasticsearch/rest/action/search/SearchCapabilities.java index f35ecb0b369fb..379588e024490 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/search/SearchCapabilities.java +++ b/server/src/main/java/org/elasticsearch/rest/action/search/SearchCapabilities.java @@ -60,6 +60,7 @@ private SearchCapabilities() {} private static final String BUCKET_SCRIPT_PARENT_MULTI_BUCKET_ERROR = "bucket_script_parent_multi_bucket_error"; private static final String EXCLUDE_SOURCE_VECTORS_SETTING = "exclude_source_vectors_setting"; private static final String CLUSTER_STATS_EXTENDED_USAGE = "extended-search-usage-stats"; + private static final String REJECT_INVALID_REVERSE_NESTING = "reject_invalid_reverse_nesting"; public static final Set CAPABILITIES; static { @@ -90,6 +91,7 @@ private SearchCapabilities() {} capabilities.add(BUCKET_SCRIPT_PARENT_MULTI_BUCKET_ERROR); capabilities.add(EXCLUDE_SOURCE_VECTORS_SETTING); capabilities.add(CLUSTER_STATS_EXTENDED_USAGE); + capabilities.add(REJECT_INVALID_REVERSE_NESTING); CAPABILITIES = Set.copyOf(capabilities); } } From 050097ce98be426495a3a154bbd2a6a7efd0c21e Mon Sep 17 00:00:00 2001 From: Matthew Alp Date: Tue, 28 Oct 2025 15:36:16 -0400 Subject: [PATCH 6/6] Update test --- .../search/aggregations/bucket/ReverseNestedIT.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/ReverseNestedIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/ReverseNestedIT.java index 33b2c74153885..2fe5464230ed4 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/ReverseNestedIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/ReverseNestedIT.java @@ -470,15 +470,19 @@ public void testReverseNestedAggWithoutNestedAgg() { public void testNonExistingNestedField() throws Exception { assertNoFailuresAndResponse( prepareSearch("idx2").setQuery(matchAllQuery()) - .addAggregation(nested("nested2", "nested1.nested2").subAggregation(reverseNested("incorrect").path("nested3"))), + .addAggregation(nested("nested2", "nested1.nested2")) + .addAggregation(nested("incorrect", "nested1.incorrect")), response -> { SingleBucketAggregation nested = response.getAggregations().get("nested2"); assertThat(nested, notNullValue()); assertThat(nested.getName(), equalTo("nested2")); + assertThat(nested.getDocCount(), is(27L)); - SingleBucketAggregation reverseNested = nested.getAggregations().get("incorrect"); - assertThat(reverseNested.getDocCount(), is(0L)); + SingleBucketAggregation incorrect = response.getAggregations().get("incorrect"); + assertThat(incorrect, notNullValue()); + assertThat(incorrect.getName(), equalTo("incorrect")); + assertThat(incorrect.getDocCount(), is(0L)); } );