From fc098586890ed147a31de1dc5924ee197c3f5d46 Mon Sep 17 00:00:00 2001 From: Now Date: Tue, 5 Aug 2025 15:50:53 +0900 Subject: [PATCH 1/3] Fix NullPointerException in LongComparator caused by null search_after values --- .../searchafter/SearchAfterBuilder.java | 11 +++- .../searchafter/SearchAfterBuilderTests.java | 57 +++++++++++++++++-- 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/searchafter/SearchAfterBuilder.java b/server/src/main/java/org/elasticsearch/search/searchafter/SearchAfterBuilder.java index 1ea702aa75e79..6b02d86b0571c 100644 --- a/server/src/main/java/org/elasticsearch/search/searchafter/SearchAfterBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/searchafter/SearchAfterBuilder.java @@ -68,7 +68,9 @@ public SearchAfterBuilder setSortValues(Object[] values) { throw new IllegalArgumentException("Values must contains at least one value."); } for (int i = 0; i < values.length; i++) { - if (values[i] == null) continue; + if (values[i] == null) { + throw new IllegalArgumentException("Values cannot contain null at position " + i + "."); + } if (values[i] instanceof String) continue; if (values[i] instanceof Text) continue; if (values[i] instanceof Long) continue; @@ -117,7 +119,7 @@ public static FieldDoc buildFieldDoc(SortAndFormats sort, Object[] values, @Null if (values[i] != null) { fieldValues[i] = convertValueFromSortField(values[i], sortField, format); } else { - fieldValues[i] = null; + throw new IllegalArgumentException("Values cannot contain null at position " + i + "."); } } /* @@ -250,7 +252,10 @@ public static SearchAfterBuilder fromXContent(XContentParser parser) throws IOEx } else if (token == XContentParser.Token.VALUE_BOOLEAN) { values.add(parser.booleanValue()); } else if (token == XContentParser.Token.VALUE_NULL) { - values.add(null); + throw new ParsingException( + parser.getTokenLocation(), + "Values cannot contain null." + ); } else { throw new ParsingException( parser.getTokenLocation(), diff --git a/server/src/test/java/org/elasticsearch/search/searchafter/SearchAfterBuilderTests.java b/server/src/test/java/org/elasticsearch/search/searchafter/SearchAfterBuilderTests.java index 031b891e49359..b3b33d69a92c9 100644 --- a/server/src/test/java/org/elasticsearch/search/searchafter/SearchAfterBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/searchafter/SearchAfterBuilderTests.java @@ -18,6 +18,7 @@ import org.apache.lucene.search.SortedNumericSortField; import org.apache.lucene.search.SortedSetSortField; import org.apache.lucene.util.BytesRef; +import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; @@ -59,7 +60,7 @@ public static SearchAfterBuilder randomSearchAfterBuilder() throws IOException { SearchAfterBuilder searchAfterBuilder = new SearchAfterBuilder(); Object[] values = new Object[numSearchFrom]; for (int i = 0; i < numSearchFrom; i++) { - int branch = randomInt(10); + int branch = randomInt(9); switch (branch) { case 0 -> values[i] = randomInt(); case 1 -> values[i] = randomFloat(); @@ -70,8 +71,7 @@ public static SearchAfterBuilder randomSearchAfterBuilder() throws IOException { case 6 -> values[i] = randomByte(); case 7 -> values[i] = randomShort(); case 8 -> values[i] = new Text(randomAlphaOfLengthBetween(5, 20)); - case 9 -> values[i] = null; - case 10 -> values[i] = randomBigInteger(); + case 9 -> values[i] = randomBigInteger(); } } searchAfterBuilder.setSortValues(values); @@ -91,7 +91,7 @@ public static SearchAfterBuilder randomJsonSearchFromBuilder(BiFunction jsonBuilder.value(randomInt()); case 1 -> jsonBuilder.value(randomFloat()); @@ -102,7 +102,6 @@ public static SearchAfterBuilder randomJsonSearchFromBuilder(BiFunction jsonBuilder.value(randomByte()); case 7 -> jsonBuilder.value(randomShort()); case 8 -> jsonBuilder.value(new Text(randomAlphaOfLengthBetween(5, 20))); - case 9 -> jsonBuilder.nullValue(); } } jsonBuilder.endArray(); @@ -280,4 +279,52 @@ public void testBuildFieldDocWithCollapse() { ); assertEquals(fieldDoc.toString(), new FieldDoc(Integer.MAX_VALUE, 0, new Object[] { new BytesRef("foo") }).toString()); } + + /** + * Test that null values are properly rejected in setSortValues + */ + public void testSetSortValuesRejectsNull() { + SearchAfterBuilder builder = new SearchAfterBuilder(); + + IllegalArgumentException e1 = expectThrows(IllegalArgumentException.class, () -> { + builder.setSortValues(new Object[]{ null }); + }); + assertThat(e1.getMessage(), equalTo("Values cannot contain null at position 0.")); + + IllegalArgumentException e2 = expectThrows(IllegalArgumentException.class, () -> { + builder.setSortValues(new Object[]{ 123L, null, "test" }); + }); + assertThat(e2.getMessage(), equalTo("Values cannot contain null at position 1.")); + } + + /** + * Test that null values are properly rejected in JSON parsing + */ + public void testFromXContentRejectsNull() throws Exception { + String jsonWithNull = "{\"search_after\": [123, null, \"test\"]}"; + try (XContentParser parser = createParser(JsonXContent.jsonXContent, jsonWithNull)) { + parser.nextToken(); + parser.nextToken(); + parser.nextToken(); + ParsingException e = expectThrows(ParsingException.class, () -> { + SearchAfterBuilder.fromXContent(parser); + }); + assertThat(e.getMessage(), equalTo("Values cannot contain null.")); + } + } + + /** + * Test that buildFieldDoc rejects null values + */ + public void testBuildFieldDocRejectsNull() { + SortField longSortField = new SortField("timestamp", SortField.Type.LONG, true); + Sort sort = new Sort(longSortField); + DocValueFormat[] formats = { DocValueFormat.RAW }; + SortAndFormats sortAndFormats = new SortAndFormats(sort, formats); + + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> { + SearchAfterBuilder.buildFieldDoc(sortAndFormats, new Object[]{ null }, null); + }); + assertThat(e.getMessage(), equalTo("Values cannot contain null at position 0.")); + } } From 462a529804c037baac75bc29f640ff0d5cf882c6 Mon Sep 17 00:00:00 2001 From: Now Date: Tue, 5 Aug 2025 16:33:02 +0900 Subject: [PATCH 2/3] Fix NullPointerException in LongComparator caused by null search_after values --- .../search/searchafter/SearchAfterBuilder.java | 5 +---- .../search/searchafter/SearchAfterBuilderTests.java | 12 ++++-------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/searchafter/SearchAfterBuilder.java b/server/src/main/java/org/elasticsearch/search/searchafter/SearchAfterBuilder.java index 6b02d86b0571c..a2f4a2d1e5f17 100644 --- a/server/src/main/java/org/elasticsearch/search/searchafter/SearchAfterBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/searchafter/SearchAfterBuilder.java @@ -252,10 +252,7 @@ public static SearchAfterBuilder fromXContent(XContentParser parser) throws IOEx } else if (token == XContentParser.Token.VALUE_BOOLEAN) { values.add(parser.booleanValue()); } else if (token == XContentParser.Token.VALUE_NULL) { - throw new ParsingException( - parser.getTokenLocation(), - "Values cannot contain null." - ); + throw new ParsingException(parser.getTokenLocation(), "Values cannot contain null."); } else { throw new ParsingException( parser.getTokenLocation(), diff --git a/server/src/test/java/org/elasticsearch/search/searchafter/SearchAfterBuilderTests.java b/server/src/test/java/org/elasticsearch/search/searchafter/SearchAfterBuilderTests.java index b3b33d69a92c9..548a91ab80873 100644 --- a/server/src/test/java/org/elasticsearch/search/searchafter/SearchAfterBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/searchafter/SearchAfterBuilderTests.java @@ -286,13 +286,11 @@ public void testBuildFieldDocWithCollapse() { public void testSetSortValuesRejectsNull() { SearchAfterBuilder builder = new SearchAfterBuilder(); - IllegalArgumentException e1 = expectThrows(IllegalArgumentException.class, () -> { - builder.setSortValues(new Object[]{ null }); - }); + IllegalArgumentException e1 = expectThrows(IllegalArgumentException.class, () -> { builder.setSortValues(new Object[] { null }); }); assertThat(e1.getMessage(), equalTo("Values cannot contain null at position 0.")); IllegalArgumentException e2 = expectThrows(IllegalArgumentException.class, () -> { - builder.setSortValues(new Object[]{ 123L, null, "test" }); + builder.setSortValues(new Object[] { 123L, null, "test" }); }); assertThat(e2.getMessage(), equalTo("Values cannot contain null at position 1.")); } @@ -306,9 +304,7 @@ public void testFromXContentRejectsNull() throws Exception { parser.nextToken(); parser.nextToken(); parser.nextToken(); - ParsingException e = expectThrows(ParsingException.class, () -> { - SearchAfterBuilder.fromXContent(parser); - }); + ParsingException e = expectThrows(ParsingException.class, () -> { SearchAfterBuilder.fromXContent(parser); }); assertThat(e.getMessage(), equalTo("Values cannot contain null.")); } } @@ -323,7 +319,7 @@ public void testBuildFieldDocRejectsNull() { SortAndFormats sortAndFormats = new SortAndFormats(sort, formats); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> { - SearchAfterBuilder.buildFieldDoc(sortAndFormats, new Object[]{ null }, null); + SearchAfterBuilder.buildFieldDoc(sortAndFormats, new Object[] { null }, null); }); assertThat(e.getMessage(), equalTo("Values cannot contain null at position 0.")); } From 2a099caa7f4457827beedac42d2db240ee3f12a9 Mon Sep 17 00:00:00 2001 From: Now Date: Tue, 19 Aug 2025 00:32:31 +0900 Subject: [PATCH 3/3] Make error message more specific for search_after null values --- .../elasticsearch/search/searchafter/SearchAfterBuilder.java | 2 +- .../search/searchafter/SearchAfterBuilderTests.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/searchafter/SearchAfterBuilder.java b/server/src/main/java/org/elasticsearch/search/searchafter/SearchAfterBuilder.java index a2f4a2d1e5f17..bee7ecb2a2dbe 100644 --- a/server/src/main/java/org/elasticsearch/search/searchafter/SearchAfterBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/searchafter/SearchAfterBuilder.java @@ -252,7 +252,7 @@ public static SearchAfterBuilder fromXContent(XContentParser parser) throws IOEx } else if (token == XContentParser.Token.VALUE_BOOLEAN) { values.add(parser.booleanValue()); } else if (token == XContentParser.Token.VALUE_NULL) { - throw new ParsingException(parser.getTokenLocation(), "Values cannot contain null."); + throw new ParsingException(parser.getTokenLocation(), "[search_after] values cannot contain null."); } else { throw new ParsingException( parser.getTokenLocation(), diff --git a/server/src/test/java/org/elasticsearch/search/searchafter/SearchAfterBuilderTests.java b/server/src/test/java/org/elasticsearch/search/searchafter/SearchAfterBuilderTests.java index 548a91ab80873..b2f99938ddfb2 100644 --- a/server/src/test/java/org/elasticsearch/search/searchafter/SearchAfterBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/searchafter/SearchAfterBuilderTests.java @@ -305,7 +305,7 @@ public void testFromXContentRejectsNull() throws Exception { parser.nextToken(); parser.nextToken(); ParsingException e = expectThrows(ParsingException.class, () -> { SearchAfterBuilder.fromXContent(parser); }); - assertThat(e.getMessage(), equalTo("Values cannot contain null.")); + assertThat(e.getMessage(), equalTo("[search_after] values cannot contain null.")); } }