From c9d4bb78cca0786a4682e6d9955811eefba5df45 Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Tue, 25 Feb 2025 17:42:55 -0700 Subject: [PATCH 1/7] Improve exception handling for JsonXContentParser --- .../provider/json/JsonXContentParser.java | 133 +++++++----------- 1 file changed, 53 insertions(+), 80 deletions(-) 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 38ef8bc2e4ef0..c5e09ba45a09b 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 @@ -15,8 +15,10 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.JsonToken; import com.fasterxml.jackson.core.exc.InputCoercionException; +import com.fasterxml.jackson.core.exc.StreamConstraintsException; import com.fasterxml.jackson.core.io.JsonEOFException; +import org.elasticsearch.core.CheckedSupplier; import org.elasticsearch.core.IOUtils; import org.elasticsearch.xcontent.XContentEOFException; import org.elasticsearch.xcontent.XContentLocation; @@ -26,6 +28,7 @@ import org.elasticsearch.xcontent.provider.XContentParserConfigurationImpl; import org.elasticsearch.xcontent.support.AbstractXContentParser; +import java.io.CharConversionException; import java.io.IOException; import java.nio.CharBuffer; @@ -48,30 +51,52 @@ public void allowDuplicateKeys(boolean allowDuplicateKeys) { parser.configure(JsonParser.Feature.STRICT_DUPLICATE_DETECTION, allowDuplicateKeys == false); } - private static XContentParseException newXContentParseException(JsonProcessingException e) { + private static XContentLocation getLocation(JsonProcessingException e) { JsonLocation loc = e.getLocation(); - throw new XContentParseException(new XContentLocation(loc.getLineNr(), loc.getColumnNr()), e.getMessage(), e); + if (loc != null) { + return new XContentLocation(loc.getLineNr(), loc.getColumnNr()); + } else { + return null; + } } - @Override - public Token nextToken() throws IOException { + private static XContentParseException newXContentParseException(JsonProcessingException e) { + return new XContentParseException(getLocation(e), e.getMessage(), e); + } + + /** + * Handle parser exception depending on type. + * This converts known exceptions to XContentParseException and rethrows them. + */ + private static IOException handleParserException(IOException e) throws IOException { + switch (e) { + case JsonEOFException eof -> throw new XContentEOFException(getLocation(eof), "Unexpected end of file", e); + case JsonParseException pe -> throw newXContentParseException(pe); + case InputCoercionException ice -> throw newXContentParseException(ice); + case CharConversionException cce -> throw new XContentParseException(null, cce.getMessage(), cce); + case StreamConstraintsException sce -> throw newXContentParseException(sce); + default -> { + return e; + } + } + } + + private static T safeParse(CheckedSupplier runnable) throws IOException { try { - return convertToken(parser.nextToken()); - } catch (JsonEOFException e) { - JsonLocation location = e.getLocation(); - throw new XContentEOFException(new XContentLocation(location.getLineNr(), location.getColumnNr()), "Unexpected end of file", e); - } catch (JsonParseException e) { - throw newXContentParseException(e); + return runnable.get(); + } catch (IOException e) { + throw handleParserException(e); } } + @Override + public Token nextToken() throws IOException { + return safeParse(() -> convertToken(parser.nextToken())); + } + @Override public String nextFieldName() throws IOException { - try { - return parser.nextFieldName(); - } catch (JsonParseException e) { - throw newXContentParseException(e); - } + return safeParse(parser::nextFieldName); } @Override @@ -96,11 +121,7 @@ public String currentName() throws IOException { @Override protected boolean doBooleanValue() throws IOException { - try { - return parser.getBooleanValue(); - } catch (JsonParseException e) { - throw newXContentParseException(e); - } + return safeParse(parser::getBooleanValue); } @Override @@ -108,11 +129,7 @@ public String text() throws IOException { if (currentToken().isValue() == false) { throwOnNoText(); } - try { - return parser.getText(); - } catch (JsonParseException e) { - throw newXContentParseException(e); - } + return safeParse(parser::getText); } private void throwOnNoText() { @@ -121,11 +138,7 @@ private void throwOnNoText() { @Override public CharBuffer charBuffer() throws IOException { - try { - return CharBuffer.wrap(parser.getTextCharacters(), parser.getTextOffset(), parser.getTextLength()); - } catch (JsonParseException e) { - throw newXContentParseException(e); - } + return safeParse(() -> CharBuffer.wrap(parser.getTextCharacters(), parser.getTextOffset(), parser.getTextLength())); } @Override @@ -171,92 +184,52 @@ public boolean hasTextCharacters() { @Override public char[] textCharacters() throws IOException { - try { - return parser.getTextCharacters(); - } catch (JsonParseException e) { - throw newXContentParseException(e); - } + return safeParse(parser::getTextCharacters); } @Override public int textLength() throws IOException { - try { - return parser.getTextLength(); - } catch (JsonParseException e) { - throw newXContentParseException(e); - } + return safeParse(parser::getTextLength); } @Override public int textOffset() throws IOException { - try { - return parser.getTextOffset(); - } catch (JsonParseException e) { - throw newXContentParseException(e); - } + return safeParse(parser::getTextOffset); } @Override public Number numberValue() throws IOException { - try { - return parser.getNumberValue(); - } catch (InputCoercionException | JsonParseException e) { - throw newXContentParseException(e); - } + return safeParse(parser::getNumberValue); } @Override public short doShortValue() throws IOException { - try { - return parser.getShortValue(); - } catch (InputCoercionException | JsonParseException e) { - throw newXContentParseException(e); - } + return safeParse(parser::getShortValue); } @Override public int doIntValue() throws IOException { - try { - return parser.getIntValue(); - } catch (InputCoercionException | JsonParseException e) { - throw newXContentParseException(e); - } + return safeParse(parser::getIntValue); } @Override public long doLongValue() throws IOException { - try { - return parser.getLongValue(); - } catch (InputCoercionException | JsonParseException e) { - throw newXContentParseException(e); - } + return safeParse(parser::getLongValue); } @Override public float doFloatValue() throws IOException { - try { - return parser.getFloatValue(); - } catch (InputCoercionException | JsonParseException e) { - throw newXContentParseException(e); - } + return safeParse(parser::getFloatValue); } @Override public double doDoubleValue() throws IOException { - try { - return parser.getDoubleValue(); - } catch (InputCoercionException | JsonParseException e) { - throw newXContentParseException(e); - } + return safeParse(parser::getDoubleValue); } @Override public byte[] binaryValue() throws IOException { - try { - return parser.getBinaryValue(); - } catch (JsonParseException e) { - throw newXContentParseException(e); - } + return safeParse(parser::getBinaryValue); } @Override From 617f12a2a564c5c2ce5d520b67b2e56414d68b5d Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Wed, 26 Feb 2025 13:41:39 -0700 Subject: [PATCH 2/7] Fix tests --- .../xcontent/provider/json/JsonXContentParser.java | 2 +- .../org/elasticsearch/action/bulk/BulkRequestParserTests.java | 3 ++- .../java/org/elasticsearch/action/bulk/BulkRequestTests.java | 2 +- .../xpack/inference/external/response/XContentUtilsTests.java | 2 +- .../huggingface/HuggingFaceElserResponseEntityTests.java | 2 +- 5 files changed, 6 insertions(+), 5 deletions(-) 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 c5e09ba45a09b..ec08d2f1b5c1d 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 @@ -70,7 +70,7 @@ private static XContentParseException newXContentParseException(JsonProcessingEx */ private static IOException handleParserException(IOException e) throws IOException { switch (e) { - case JsonEOFException eof -> throw new XContentEOFException(getLocation(eof), "Unexpected end of file", e); + case JsonEOFException eof -> throw new XContentEOFException(getLocation(eof), eof.getMessage(), e); case JsonParseException pe -> throw newXContentParseException(pe); case InputCoercionException ice -> throw newXContentParseException(ice); case CharConversionException cce -> throw new XContentParseException(null, cce.getMessage(), cce); diff --git a/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestParserTests.java b/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestParserTests.java index de2dc8f9ea0b5..e2e37ffaf611c 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestParserTests.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestParserTests.java @@ -24,6 +24,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Stream; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; public class BulkRequestParserTests extends ESTestCase { @@ -394,7 +395,7 @@ public void testFailMissingCloseBrace() { req -> fail("expected failure before we got this far") ) ); - assertEquals("[1:14] Unexpected end of file", ex.getMessage()); + assertThat(ex.getMessage(), containsString("[1:14] Unexpected end-of-input")); } public void testFailExtraKeys() { diff --git a/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java b/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java index b36b3af1ddb86..a085826e511e1 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java @@ -432,7 +432,7 @@ public void testBulkActionWithoutCurlyBrace() throws Exception { () -> bulkRequest.add(bulkAction.getBytes(StandardCharsets.UTF_8), 0, bulkAction.length(), null, XContentType.JSON) ); - assertThat(ex.getMessage(), containsString("Unexpected end of file")); + assertThat(ex.getMessage(), containsString("Unexpected end-of-input")); } public void testBulkActionWithAdditionalKeys() throws Exception { diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/XContentUtilsTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/XContentUtilsTests.java index 360936373a010..20dc6810f1553 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/XContentUtilsTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/XContentUtilsTests.java @@ -103,7 +103,7 @@ public void testPositionParserAtTokenAfterField_ThrowsWithMalformedJSON() throws XContentEOFException.class, () -> XContentUtils.positionParserAtTokenAfterField(parser, missingField, errorFormat) ); - assertThat(exception.getMessage(), containsString("[4:1] Unexpected end of file")); + assertThat(exception.getMessage(), containsString("[4:1] Unexpected end-of-input")); } } diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/huggingface/HuggingFaceElserResponseEntityTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/huggingface/HuggingFaceElserResponseEntityTests.java index e28d4f9608ae5..ca5862d71f8d5 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/huggingface/HuggingFaceElserResponseEntityTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/huggingface/HuggingFaceElserResponseEntityTests.java @@ -309,7 +309,7 @@ public void testFails_ResponseIsInvalidJson_MissingSquareBracket() { new HttpResult(mock(HttpResponse.class), responseJson.getBytes(StandardCharsets.UTF_8)) ) ); - assertThat(thrownException.getMessage(), containsString("[5:1] Unexpected end of file")); + assertThat(thrownException.getMessage(), containsString("[5:1] Unexpected end-of-input")); assertThat(thrownException.getCause().getMessage(), containsString("expected close marker for Array (start marker at")); } From d37e09c68c621d8d84744deacf5103121bdcb3e2 Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Wed, 26 Feb 2025 14:45:03 -0700 Subject: [PATCH 3/7] More test fixes --- .../xcontent/provider/json/JsonXContentParser.java | 2 +- .../org/elasticsearch/action/bulk/BulkRequestParserTests.java | 3 +-- .../java/org/elasticsearch/action/bulk/BulkRequestTests.java | 2 +- .../xpack/inference/external/response/XContentUtilsTests.java | 2 +- .../huggingface/HuggingFaceElserResponseEntityTests.java | 2 +- .../email/attachment/ReportingAttachmentParserTests.java | 2 +- 6 files changed, 6 insertions(+), 7 deletions(-) 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 ec08d2f1b5c1d..c5e09ba45a09b 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 @@ -70,7 +70,7 @@ private static XContentParseException newXContentParseException(JsonProcessingEx */ private static IOException handleParserException(IOException e) throws IOException { switch (e) { - case JsonEOFException eof -> throw new XContentEOFException(getLocation(eof), eof.getMessage(), e); + case JsonEOFException eof -> throw new XContentEOFException(getLocation(eof), "Unexpected end of file", e); case JsonParseException pe -> throw newXContentParseException(pe); case InputCoercionException ice -> throw newXContentParseException(ice); case CharConversionException cce -> throw new XContentParseException(null, cce.getMessage(), cce); diff --git a/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestParserTests.java b/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestParserTests.java index e2e37ffaf611c..de2dc8f9ea0b5 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestParserTests.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestParserTests.java @@ -24,7 +24,6 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Stream; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; public class BulkRequestParserTests extends ESTestCase { @@ -395,7 +394,7 @@ public void testFailMissingCloseBrace() { req -> fail("expected failure before we got this far") ) ); - assertThat(ex.getMessage(), containsString("[1:14] Unexpected end-of-input")); + assertEquals("[1:14] Unexpected end of file", ex.getMessage()); } public void testFailExtraKeys() { diff --git a/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java b/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java index a085826e511e1..b36b3af1ddb86 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java @@ -432,7 +432,7 @@ public void testBulkActionWithoutCurlyBrace() throws Exception { () -> bulkRequest.add(bulkAction.getBytes(StandardCharsets.UTF_8), 0, bulkAction.length(), null, XContentType.JSON) ); - assertThat(ex.getMessage(), containsString("Unexpected end-of-input")); + assertThat(ex.getMessage(), containsString("Unexpected end of file")); } public void testBulkActionWithAdditionalKeys() throws Exception { diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/XContentUtilsTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/XContentUtilsTests.java index 20dc6810f1553..360936373a010 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/XContentUtilsTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/XContentUtilsTests.java @@ -103,7 +103,7 @@ public void testPositionParserAtTokenAfterField_ThrowsWithMalformedJSON() throws XContentEOFException.class, () -> XContentUtils.positionParserAtTokenAfterField(parser, missingField, errorFormat) ); - assertThat(exception.getMessage(), containsString("[4:1] Unexpected end-of-input")); + assertThat(exception.getMessage(), containsString("[4:1] Unexpected end of file")); } } diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/huggingface/HuggingFaceElserResponseEntityTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/huggingface/HuggingFaceElserResponseEntityTests.java index ca5862d71f8d5..e28d4f9608ae5 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/huggingface/HuggingFaceElserResponseEntityTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/response/huggingface/HuggingFaceElserResponseEntityTests.java @@ -309,7 +309,7 @@ public void testFails_ResponseIsInvalidJson_MissingSquareBracket() { new HttpResult(mock(HttpResponse.class), responseJson.getBytes(StandardCharsets.UTF_8)) ) ); - assertThat(thrownException.getMessage(), containsString("[5:1] Unexpected end-of-input")); + assertThat(thrownException.getMessage(), containsString("[5:1] Unexpected end of file")); assertThat(thrownException.getCause().getMessage(), containsString("expected close marker for Array (start marker at")); } diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/attachment/ReportingAttachmentParserTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/attachment/ReportingAttachmentParserTests.java index f3b75273b2860..fed491c3a008d 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/attachment/ReportingAttachmentParserTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/attachment/ReportingAttachmentParserTests.java @@ -239,7 +239,7 @@ public void testInitialRequestContainsInvalidPayload() throws Exception { XContentParseException.class, () -> reportingAttachmentParser.toAttachment(createWatchExecutionContext(), Payload.EMPTY, attachment) ); - assertThat(e.getMessage(), containsString("Unexpected end-of-input")); + assertThat(e.getMessage(), containsString("Unexpected end of file")); } public void testInitialRequestContainsPathAsObject() throws Exception { From 20a0f9e85e3a83c6748ceecc7dbc4aa84d9f4dac Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Wed, 26 Feb 2025 16:32:50 -0700 Subject: [PATCH 4/7] Wrap parser creation so that bad char sequences do not cause 500 --- .../provider/json/JsonXContentImpl.java | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java index 7f52467caf49b..f59d8b6de2873 100644 --- a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java +++ b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java @@ -18,11 +18,13 @@ import org.elasticsearch.xcontent.XContent; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.XContentGenerator; +import org.elasticsearch.xcontent.XContentParseException; import org.elasticsearch.xcontent.XContentParser; import org.elasticsearch.xcontent.XContentParserConfiguration; import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xcontent.provider.XContentImplUtils; +import java.io.CharConversionException; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -96,21 +98,37 @@ private XContentParser createParser(XContentParserConfiguration config, JsonPars @Override public XContentParser createParser(XContentParserConfiguration config, String content) throws IOException { - return createParser(config, jsonFactory.createParser(content)); + try { + return createParser(config, jsonFactory.createParser(content)); + } catch (CharConversionException e) { + throw new XContentParseException(null, e.getMessage(), e); + } } @Override public XContentParser createParser(XContentParserConfiguration config, InputStream is) throws IOException { - return createParser(config, jsonFactory.createParser(is)); + try { + return createParser(config, jsonFactory.createParser(is)); + } catch (CharConversionException e) { + throw new XContentParseException(null, e.getMessage(), e); + } } @Override public XContentParser createParser(XContentParserConfiguration config, byte[] data, int offset, int length) throws IOException { + try { return createParser(config, jsonFactory.createParser(data, offset, length)); + } catch (CharConversionException e) { + throw new XContentParseException(null, e.getMessage(), e); + } } @Override public XContentParser createParser(XContentParserConfiguration config, Reader reader) throws IOException { + try { return createParser(config, jsonFactory.createParser(reader)); + } catch (CharConversionException e) { + throw new XContentParseException(null, e.getMessage(), e); + } } } From ca7b09ef697a830d7f8c1b493e87a59fc56bc1fa Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Thu, 27 Feb 2025 13:52:05 -0700 Subject: [PATCH 5/7] Unroll safeParse due to performance concerns --- .../provider/json/JsonXContentParser.java | 95 ++++++++++++++----- 1 file changed, 73 insertions(+), 22 deletions(-) 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 c5e09ba45a09b..84026805034a1 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 @@ -18,7 +18,6 @@ import com.fasterxml.jackson.core.exc.StreamConstraintsException; import com.fasterxml.jackson.core.io.JsonEOFException; -import org.elasticsearch.core.CheckedSupplier; import org.elasticsearch.core.IOUtils; import org.elasticsearch.xcontent.XContentEOFException; import org.elasticsearch.xcontent.XContentLocation; @@ -81,22 +80,22 @@ private static IOException handleParserException(IOException e) throws IOExcepti } } - private static T safeParse(CheckedSupplier runnable) throws IOException { + @Override + public Token nextToken() throws IOException { try { - return runnable.get(); + return convertToken(parser.nextToken()); } catch (IOException e) { throw handleParserException(e); } } - @Override - public Token nextToken() throws IOException { - return safeParse(() -> convertToken(parser.nextToken())); - } - @Override public String nextFieldName() throws IOException { - return safeParse(parser::nextFieldName); + try { + return parser.nextFieldName(); + } catch (IOException e) { + throw handleParserException(e); + } } @Override @@ -121,7 +120,11 @@ public String currentName() throws IOException { @Override protected boolean doBooleanValue() throws IOException { - return safeParse(parser::getBooleanValue); + try { + return parser.getBooleanValue(); + } catch (IOException e) { + throw handleParserException(e); + } } @Override @@ -129,7 +132,11 @@ public String text() throws IOException { if (currentToken().isValue() == false) { throwOnNoText(); } - return safeParse(parser::getText); + try { + return parser.getText(); + } catch (IOException e) { + throw handleParserException(e); + } } private void throwOnNoText() { @@ -138,7 +145,11 @@ private void throwOnNoText() { @Override public CharBuffer charBuffer() throws IOException { - return safeParse(() -> CharBuffer.wrap(parser.getTextCharacters(), parser.getTextOffset(), parser.getTextLength())); + try { + return CharBuffer.wrap(parser.getTextCharacters(), parser.getTextOffset(), parser.getTextLength()); + } catch (IOException e) { + throw handleParserException(e); + } } @Override @@ -184,52 +195,92 @@ public boolean hasTextCharacters() { @Override public char[] textCharacters() throws IOException { - return safeParse(parser::getTextCharacters); + try { + return parser.getTextCharacters(); + } catch (IOException e) { + throw handleParserException(e); + } } @Override public int textLength() throws IOException { - return safeParse(parser::getTextLength); + try { + return parser.getTextLength(); + } catch (IOException e) { + throw handleParserException(e); + } } @Override public int textOffset() throws IOException { - return safeParse(parser::getTextOffset); + try { + return parser.getTextOffset(); + } catch (IOException e) { + throw handleParserException(e); + } } @Override public Number numberValue() throws IOException { - return safeParse(parser::getNumberValue); + try { + return parser.getNumberValue(); + } catch (IOException e) { + throw handleParserException(e); + } } @Override public short doShortValue() throws IOException { - return safeParse(parser::getShortValue); + try { + return parser.getShortValue(); + } catch (IOException e) { + throw handleParserException(e); + } } @Override public int doIntValue() throws IOException { - return safeParse(parser::getIntValue); + try { + return parser.getIntValue(); + } catch (IOException e) { + throw handleParserException(e); + } } @Override public long doLongValue() throws IOException { - return safeParse(parser::getLongValue); + try { + return parser.getLongValue(); + } catch (IOException e) { + throw handleParserException(e); + } } @Override public float doFloatValue() throws IOException { - return safeParse(parser::getFloatValue); + try { + return parser.getFloatValue(); + } catch (IOException e) { + throw handleParserException(e); + } } @Override public double doDoubleValue() throws IOException { - return safeParse(parser::getDoubleValue); + try { + return parser.getDoubleValue(); + } catch (IOException e) { + throw handleParserException(e); + } } @Override public byte[] binaryValue() throws IOException { - return safeParse(parser::getBinaryValue); + try { + return parser.getBinaryValue(); + } catch (IOException e) { + throw handleParserException(e); + } } @Override From ddb4c98920c68b2169e537bcf5c2ef75c702130a Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Thu, 27 Feb 2025 13:52:40 -0700 Subject: [PATCH 6/7] Add parser-only microbenchmark --- .../xcontent/JsonParserBenchmark.java | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 benchmarks/src/main/java/org/elasticsearch/benchmark/xcontent/JsonParserBenchmark.java diff --git a/benchmarks/src/main/java/org/elasticsearch/benchmark/xcontent/JsonParserBenchmark.java b/benchmarks/src/main/java/org/elasticsearch/benchmark/xcontent/JsonParserBenchmark.java new file mode 100644 index 0000000000000..a49f01f81c34f --- /dev/null +++ b/benchmarks/src/main/java/org/elasticsearch/benchmark/xcontent/JsonParserBenchmark.java @@ -0,0 +1,87 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.benchmark.xcontent; + +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.io.Streams; +import org.elasticsearch.xcontent.XContentParser; +import org.elasticsearch.xcontent.XContentParserConfiguration; +import org.elasticsearch.xcontent.XContentType; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; + +import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Random; +import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; + +@Fork(1) +@Warmup(iterations = 5) +@Measurement(iterations = 10) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@State(Scope.Thread) +public class JsonParserBenchmark { + private Map sourceBytes; + private BytesReference source; + private Random random; + private List sourcesRandomized; + + final String[] sources = new String[] { "monitor_cluster_stats.json", "monitor_index_stats.json", "monitor_node_stats.json" }; + + @Setup(Level.Iteration) + public void randomizeSource() { + sourcesRandomized = Arrays.asList(sources); + Collections.shuffle(sourcesRandomized, random); + } + + @Setup(Level.Trial) + public void setup() throws IOException { + random = new Random(); + sourceBytes = Arrays.stream(sources).collect(Collectors.toMap(s -> s, s -> { + try { + return readSource(s); + } catch (IOException e) { + throw new RuntimeException(e); + } + })); + } + + @Benchmark + public void parseJson(Blackhole bh) throws IOException { + sourcesRandomized.forEach(source -> { + try { + final XContentParser parser = XContentType.JSON.xContent() + .createParser(XContentParserConfiguration.EMPTY, sourceBytes.get(source).streamInput()); + bh.consume(parser.mapOrdered()); + } catch (IOException e) { + throw new RuntimeException(e); + } + }); + } + + private BytesReference readSource(String fileName) throws IOException { + return Streams.readFully(JsonParserBenchmark.class.getResourceAsStream(fileName)); + } +} From 900da1940aacd3e7b9d46741818353ab3fde85bb Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Thu, 27 Feb 2025 13:54:46 -0700 Subject: [PATCH 7/7] spotless --- .../xcontent/provider/json/JsonXContentImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java index f59d8b6de2873..0d3fc587bd33c 100644 --- a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java +++ b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java @@ -117,7 +117,7 @@ public XContentParser createParser(XContentParserConfiguration config, InputStre @Override public XContentParser createParser(XContentParserConfiguration config, byte[] data, int offset, int length) throws IOException { try { - return createParser(config, jsonFactory.createParser(data, offset, length)); + return createParser(config, jsonFactory.createParser(data, offset, length)); } catch (CharConversionException e) { throw new XContentParseException(null, e.getMessage(), e); } @@ -126,7 +126,7 @@ public XContentParser createParser(XContentParserConfiguration config, byte[] da @Override public XContentParser createParser(XContentParserConfiguration config, Reader reader) throws IOException { try { - return createParser(config, jsonFactory.createParser(reader)); + return createParser(config, jsonFactory.createParser(reader)); } catch (CharConversionException e) { throw new XContentParseException(null, e.getMessage(), e); }