From df83eb983c5f5cdc50c3d2c3d080b4c56bc571f4 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Wed, 23 Oct 2019 20:01:42 +0200 Subject: [PATCH] Do not throw errors on unknown types in SearchAfterBuilder (#48147) * Do not throw errors on unknown types in SearchAfterBuilder The support for BigInteger and BigDecimal was added for XContent in https://github.com/elastic/elasticsearch/pull/32888. However the SearchAfterBuilder xcontent parser doesn't expect them to be present so it throws an AssertionError. This change fixes this discrepancy by changing the AssertionError into an IllegalArgumentException that will not cause the node to die when thrown. Closes #48074 --- .../searchafter/SearchAfterBuilder.java | 3 +- .../searchafter/SearchAfterBuilderTests.java | 41 +++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) 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 bdd44c3438dd6..304a639a8981a 100644 --- a/server/src/main/java/org/elasticsearch/search/searchafter/SearchAfterBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/searchafter/SearchAfterBuilder.java @@ -243,7 +243,8 @@ public static SearchAfterBuilder fromXContent(XContentParser parser) throws IOEx break; default: - throw new AssertionError("Unknown number type []" + parser.numberType()); + throw new IllegalArgumentException("[search_after] does not accept numbers of type [" + + parser.numberType() + "], got " + parser.text()); } } else if (token == XContentParser.Token.VALUE_STRING) { values.add(parser.text()); 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 f7457d965744a..a0e4c6bbbea6e 100644 --- a/server/src/test/java/org/elasticsearch/search/searchafter/SearchAfterBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/searchafter/SearchAfterBuilderTests.java @@ -38,10 +38,13 @@ import org.elasticsearch.test.ESTestCase; import java.io.IOException; +import java.math.BigDecimal; +import java.math.BigInteger; import java.util.Collections; import static org.elasticsearch.search.searchafter.SearchAfterBuilder.extractSortType; import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; public class SearchAfterBuilderTests extends ESTestCase { @@ -187,6 +190,44 @@ public void testFromXContent() throws Exception { } } + public void testFromXContentIllegalType() throws Exception { + for (XContentType type : XContentType.values()) { + // BIG_INTEGER + XContentBuilder xContent = XContentFactory.contentBuilder(type); + xContent.startObject() + .startArray("search_after") + .value(new BigInteger("9223372036854776000")) + .endArray() + .endObject(); + try (XContentParser parser = createParser(xContent)) { + parser.nextToken(); + parser.nextToken(); + parser.nextToken(); + IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, () -> SearchAfterBuilder.fromXContent(parser)); + assertThat(exc.getMessage(), containsString("BIG_INTEGER")); + } + + // BIG_DECIMAL + // ignore json and yaml, they parse floating point numbers as floats/doubles + if (type == XContentType.JSON || type == XContentType.YAML) { + continue; + } + xContent = XContentFactory.contentBuilder(type); + xContent.startObject() + .startArray("search_after") + .value(new BigDecimal("9223372036854776003.3")) + .endArray() + .endObject(); + try (XContentParser parser = createParser(xContent)) { + parser.nextToken(); + parser.nextToken(); + parser.nextToken(); + IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, () -> SearchAfterBuilder.fromXContent(parser)); + assertThat(exc.getMessage(), containsString("BIG_DECIMAL")); + } + } + } + public void testWithNullArray() throws Exception { SearchAfterBuilder builder = new SearchAfterBuilder(); try {