From 75ff390c6f8267ea7db3f1b6e676be532bd1c31b Mon Sep 17 00:00:00 2001 From: Benjamin Trent Date: Wed, 11 Sep 2024 10:15:56 -0400 Subject: [PATCH] JSON parse failures should be 4xx codes (#112703) It seemed if there wasn't any text to parse, this is not an internal issue but instead an argument issue. I simply changed the exception thrown. If we don't agree with this, I can adjust `query` parsing directly, but this seemed like the better choice. closes: https://github.com/elastic/elasticsearch/issues/112296 --- docs/changelog/112703.yaml | 5 +++++ .../xcontent/provider/json/JsonXContentParser.java | 2 +- .../elasticsearch/index/mapper/IpFieldMapper.java | 12 ++---------- .../org/elasticsearch/common/ReferenceDocsTests.java | 2 +- .../index/query/MatchQueryBuilderTests.java | 2 +- 5 files changed, 10 insertions(+), 13 deletions(-) create mode 100644 docs/changelog/112703.yaml diff --git a/docs/changelog/112703.yaml b/docs/changelog/112703.yaml new file mode 100644 index 0000000000000..a428e8c4e2339 --- /dev/null +++ b/docs/changelog/112703.yaml @@ -0,0 +1,5 @@ +pr: 112703 +summary: JSON parse failures should be 4xx codes +area: Infra/Core +type: bug +issues: [] diff --git a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentParser.java b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentParser.java index c59f003d9cb04..63191084ca837 100644 --- a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentParser.java +++ b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentParser.java @@ -111,7 +111,7 @@ public String text() throws IOException { } private void throwOnNoText() { - throw new IllegalStateException("Can't get text on a " + currentToken() + " at " + getTokenLocation()); + throw new IllegalArgumentException("Expected text at " + getTokenLocation() + " but found " + currentToken()); } @Override diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java index d5819f5eb4338..6fe1c70b66e5e 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java @@ -42,7 +42,6 @@ import org.elasticsearch.search.aggregations.support.CoreValuesSourceType; import org.elasticsearch.search.lookup.FieldValues; import org.elasticsearch.search.lookup.SearchLookup; -import org.elasticsearch.xcontent.XContentParser; import java.io.IOException; import java.net.InetAddress; @@ -533,8 +532,9 @@ protected String contentType() { @Override protected void parseCreateField(DocumentParserContext context) throws IOException { InetAddress address; + String value = context.parser().textOrNull(); try { - address = value(context.parser(), nullValue); + address = value == null ? nullValue : InetAddresses.forString(value); } catch (IllegalArgumentException e) { if (ignoreMalformed) { context.addIgnoredField(fieldType().name()); @@ -552,14 +552,6 @@ protected void parseCreateField(DocumentParserContext context) throws IOExceptio } } - private static InetAddress value(XContentParser parser, InetAddress nullValue) throws IOException { - String value = parser.textOrNull(); - if (value == null) { - return nullValue; - } - return InetAddresses.forString(value); - } - private void indexValue(DocumentParserContext context, InetAddress address) { if (dimension) { context.getDimensions().addIp(fieldType().name(), address).validate(context.indexSettings()); diff --git a/server/src/test/java/org/elasticsearch/common/ReferenceDocsTests.java b/server/src/test/java/org/elasticsearch/common/ReferenceDocsTests.java index 0fabf78017304..49208f2341701 100644 --- a/server/src/test/java/org/elasticsearch/common/ReferenceDocsTests.java +++ b/server/src/test/java/org/elasticsearch/common/ReferenceDocsTests.java @@ -66,7 +66,7 @@ public void testResourceValidation() throws Exception { builder.startObject("UNEXPECTED").endObject().endObject(); try (var stream = BytesReference.bytes(builder).streamInput()) { - expectThrows(IllegalStateException.class, () -> ReferenceDocs.readLinksBySymbol(stream)); + expectThrows(IllegalArgumentException.class, () -> ReferenceDocs.readLinksBySymbol(stream)); } } diff --git a/server/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java index 278d4ae505bdc..48e8f0ef11676 100644 --- a/server/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java @@ -373,7 +373,7 @@ public void testParseFailsWithTermsArray() { "message1" : ["term1", "term2"] } }"""; - expectThrows(IllegalStateException.class, () -> parseQuery(json2)); + expectThrows(IllegalArgumentException.class, () -> parseQuery(json2)); } public void testExceptionUsingAnalyzerOnNumericField() {