Skip to content

Commit fc09858

Browse files
committed
Fix NullPointerException in LongComparator caused by null search_after values
1 parent 68eff34 commit fc09858

File tree

2 files changed

+60
-8
lines changed

2 files changed

+60
-8
lines changed

server/src/main/java/org/elasticsearch/search/searchafter/SearchAfterBuilder.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,9 @@ public SearchAfterBuilder setSortValues(Object[] values) {
6868
throw new IllegalArgumentException("Values must contains at least one value.");
6969
}
7070
for (int i = 0; i < values.length; i++) {
71-
if (values[i] == null) continue;
71+
if (values[i] == null) {
72+
throw new IllegalArgumentException("Values cannot contain null at position " + i + ".");
73+
}
7274
if (values[i] instanceof String) continue;
7375
if (values[i] instanceof Text) continue;
7476
if (values[i] instanceof Long) continue;
@@ -117,7 +119,7 @@ public static FieldDoc buildFieldDoc(SortAndFormats sort, Object[] values, @Null
117119
if (values[i] != null) {
118120
fieldValues[i] = convertValueFromSortField(values[i], sortField, format);
119121
} else {
120-
fieldValues[i] = null;
122+
throw new IllegalArgumentException("Values cannot contain null at position " + i + ".");
121123
}
122124
}
123125
/*
@@ -250,7 +252,10 @@ public static SearchAfterBuilder fromXContent(XContentParser parser) throws IOEx
250252
} else if (token == XContentParser.Token.VALUE_BOOLEAN) {
251253
values.add(parser.booleanValue());
252254
} else if (token == XContentParser.Token.VALUE_NULL) {
253-
values.add(null);
255+
throw new ParsingException(
256+
parser.getTokenLocation(),
257+
"Values cannot contain null."
258+
);
254259
} else {
255260
throw new ParsingException(
256261
parser.getTokenLocation(),

server/src/test/java/org/elasticsearch/search/searchafter/SearchAfterBuilderTests.java

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import org.apache.lucene.search.SortedNumericSortField;
1919
import org.apache.lucene.search.SortedSetSortField;
2020
import org.apache.lucene.util.BytesRef;
21+
import org.elasticsearch.common.ParsingException;
2122
import org.elasticsearch.common.bytes.BytesReference;
2223
import org.elasticsearch.common.geo.GeoPoint;
2324
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
@@ -59,7 +60,7 @@ public static SearchAfterBuilder randomSearchAfterBuilder() throws IOException {
5960
SearchAfterBuilder searchAfterBuilder = new SearchAfterBuilder();
6061
Object[] values = new Object[numSearchFrom];
6162
for (int i = 0; i < numSearchFrom; i++) {
62-
int branch = randomInt(10);
63+
int branch = randomInt(9);
6364
switch (branch) {
6465
case 0 -> values[i] = randomInt();
6566
case 1 -> values[i] = randomFloat();
@@ -70,8 +71,7 @@ public static SearchAfterBuilder randomSearchAfterBuilder() throws IOException {
7071
case 6 -> values[i] = randomByte();
7172
case 7 -> values[i] = randomShort();
7273
case 8 -> values[i] = new Text(randomAlphaOfLengthBetween(5, 20));
73-
case 9 -> values[i] = null;
74-
case 10 -> values[i] = randomBigInteger();
74+
case 9 -> values[i] = randomBigInteger();
7575
}
7676
}
7777
searchAfterBuilder.setSortValues(values);
@@ -91,7 +91,7 @@ public static SearchAfterBuilder randomJsonSearchFromBuilder(BiFunction<XContent
9191
jsonBuilder.startObject();
9292
jsonBuilder.startArray("search_after");
9393
for (int i = 0; i < numSearchAfter; i++) {
94-
int branch = randomInt(9);
94+
int branch = randomInt(8);
9595
switch (branch) {
9696
case 0 -> jsonBuilder.value(randomInt());
9797
case 1 -> jsonBuilder.value(randomFloat());
@@ -102,7 +102,6 @@ public static SearchAfterBuilder randomJsonSearchFromBuilder(BiFunction<XContent
102102
case 6 -> jsonBuilder.value(randomByte());
103103
case 7 -> jsonBuilder.value(randomShort());
104104
case 8 -> jsonBuilder.value(new Text(randomAlphaOfLengthBetween(5, 20)));
105-
case 9 -> jsonBuilder.nullValue();
106105
}
107106
}
108107
jsonBuilder.endArray();
@@ -280,4 +279,52 @@ public void testBuildFieldDocWithCollapse() {
280279
);
281280
assertEquals(fieldDoc.toString(), new FieldDoc(Integer.MAX_VALUE, 0, new Object[] { new BytesRef("foo") }).toString());
282281
}
282+
283+
/**
284+
* Test that null values are properly rejected in setSortValues
285+
*/
286+
public void testSetSortValuesRejectsNull() {
287+
SearchAfterBuilder builder = new SearchAfterBuilder();
288+
289+
IllegalArgumentException e1 = expectThrows(IllegalArgumentException.class, () -> {
290+
builder.setSortValues(new Object[]{ null });
291+
});
292+
assertThat(e1.getMessage(), equalTo("Values cannot contain null at position 0."));
293+
294+
IllegalArgumentException e2 = expectThrows(IllegalArgumentException.class, () -> {
295+
builder.setSortValues(new Object[]{ 123L, null, "test" });
296+
});
297+
assertThat(e2.getMessage(), equalTo("Values cannot contain null at position 1."));
298+
}
299+
300+
/**
301+
* Test that null values are properly rejected in JSON parsing
302+
*/
303+
public void testFromXContentRejectsNull() throws Exception {
304+
String jsonWithNull = "{\"search_after\": [123, null, \"test\"]}";
305+
try (XContentParser parser = createParser(JsonXContent.jsonXContent, jsonWithNull)) {
306+
parser.nextToken();
307+
parser.nextToken();
308+
parser.nextToken();
309+
ParsingException e = expectThrows(ParsingException.class, () -> {
310+
SearchAfterBuilder.fromXContent(parser);
311+
});
312+
assertThat(e.getMessage(), equalTo("Values cannot contain null."));
313+
}
314+
}
315+
316+
/**
317+
* Test that buildFieldDoc rejects null values
318+
*/
319+
public void testBuildFieldDocRejectsNull() {
320+
SortField longSortField = new SortField("timestamp", SortField.Type.LONG, true);
321+
Sort sort = new Sort(longSortField);
322+
DocValueFormat[] formats = { DocValueFormat.RAW };
323+
SortAndFormats sortAndFormats = new SortAndFormats(sort, formats);
324+
325+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> {
326+
SearchAfterBuilder.buildFieldDoc(sortAndFormats, new Object[]{ null }, null);
327+
});
328+
assertThat(e.getMessage(), equalTo("Values cannot contain null at position 0."));
329+
}
283330
}

0 commit comments

Comments
 (0)