Skip to content

Commit

Permalink
JSON parse failures should be 4xx codes (elastic#112703)
Browse files Browse the repository at this point in the history
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: elastic#112296
  • Loading branch information
benwtrent committed Sep 11, 2024
1 parent e1e9e83 commit 75ff390
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 13 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/112703.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 112703
summary: JSON parse failures should be 4xx codes
area: Infra/Core
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand All @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ public void testParseFailsWithTermsArray() {
"message1" : ["term1", "term2"]
}
}""";
expectThrows(IllegalStateException.class, () -> parseQuery(json2));
expectThrows(IllegalArgumentException.class, () -> parseQuery(json2));
}

public void testExceptionUsingAnalyzerOnNumericField() {
Expand Down

0 comments on commit 75ff390

Please sign in to comment.