Improve exception handling for JsonXContentParser (#123439) (#132480)

* Improve exception handling for JsonXContentParser
* Wrap parser creation so that bad char sequences do not cause 500
* Unroll safeParse due to performance concerns
* Add parser-only microbenchmark

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<String, BytesReference> sourceBytes;
+    private BytesReference source;
+    private Random random;
+    private List<String> 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));
+    }
+}

libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java

@@ -28,6 +28,8 @@ import java.io.OutputStream;
 import java.io.Reader;
 import java.util.Set;
 
+import static org.elasticsearch.xcontent.provider.json.JsonXContentParser.handleParserException;
+
 /**
  * A JSON based content implementation using Jackson.
  */
@@ -95,21 +97,37 @@ public class JsonXContentImpl implements XContent {
 
     @Override
     public XContentParser createParser(XContentParserConfiguration config, String content) throws IOException {
-        return createParser(config, jsonFactory.createParser(content));
+        try {
+            return createParser(config, jsonFactory.createParser(content));
+        } catch (IOException e) {
+            throw handleParserException(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 (IOException e) {
+            throw handleParserException(e);
+        }
     }
 
     @Override
     public XContentParser createParser(XContentParserConfiguration config, byte[] data, int offset, int length) throws IOException {
-        return createParser(config, jsonFactory.createParser(data, offset, length));
+        try {
+            return createParser(config, jsonFactory.createParser(data, offset, length));
+        } catch (IOException e) {
+            throw handleParserException(e);
+        }
     }
 
     @Override
     public XContentParser createParser(XContentParserConfiguration config, Reader reader) throws IOException {
-        return createParser(config, jsonFactory.createParser(reader));
+        try {
+            return createParser(config, jsonFactory.createParser(reader));
+        } catch (IOException e) {
+            throw handleParserException(e);
+        }
     }
 }

libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentParser.java

@@ -15,6 +15,7 @@ import com.fasterxml.jackson.core.JsonParser;
 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.IOUtils;
@@ -28,6 +29,7 @@ import org.elasticsearch.xcontent.XContentType;
 import org.elasticsearch.xcontent.provider.XContentParserConfigurationImpl;
 import org.elasticsearch.xcontent.support.AbstractXContentParser;
 
+import java.io.CharConversionException;
 import java.io.IOException;
 import java.nio.CharBuffer;
 
@@ -50,20 +52,42 @@ public class JsonXContentParser extends AbstractXContentParser {
         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;
+        }
+    }
+
+    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.
+     */
+    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;
+            }
+        }
     }
 
     @Override
     public Token nextToken() 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);
+        } catch (IOException e) {
+            throw handleParserException(e);
         }
     }
 
@@ -71,8 +95,8 @@ public class JsonXContentParser extends AbstractXContentParser {
     public String nextFieldName() throws IOException {
         try {
             return parser.nextFieldName();
-        } catch (JsonParseException e) {
-            throw newXContentParseException(e);
+        } catch (IOException e) {
+            throw handleParserException(e);
         }
     }
 
@@ -100,8 +124,8 @@ public class JsonXContentParser extends AbstractXContentParser {
     protected boolean doBooleanValue() throws IOException {
         try {
             return parser.getBooleanValue();
-        } catch (JsonParseException e) {
-            throw newXContentParseException(e);
+        } catch (IOException e) {
+            throw handleParserException(e);
         }
     }
 
@@ -112,8 +136,8 @@ public class JsonXContentParser extends AbstractXContentParser {
         }
         try {
             return parser.getText();
-        } catch (JsonParseException e) {
-            throw newXContentParseException(e);
+        } catch (IOException e) {
+            throw handleParserException(e);
         }
     }
 
@@ -139,8 +163,8 @@ public class JsonXContentParser extends AbstractXContentParser {
     public CharBuffer charBuffer() throws IOException {
         try {
             return CharBuffer.wrap(parser.getTextCharacters(), parser.getTextOffset(), parser.getTextLength());
-        } catch (JsonParseException e) {
-            throw newXContentParseException(e);
+        } catch (IOException e) {
+            throw handleParserException(e);
         }
     }
 
@@ -189,8 +213,8 @@ public class JsonXContentParser extends AbstractXContentParser {
     public char[] textCharacters() throws IOException {
         try {
             return parser.getTextCharacters();
-        } catch (JsonParseException e) {
-            throw newXContentParseException(e);
+        } catch (IOException e) {
+            throw handleParserException(e);
         }
     }
 
@@ -198,8 +222,8 @@ public class JsonXContentParser extends AbstractXContentParser {
     public int textLength() throws IOException {
         try {
             return parser.getTextLength();
-        } catch (JsonParseException e) {
-            throw newXContentParseException(e);
+        } catch (IOException e) {
+            throw handleParserException(e);
         }
     }
 
@@ -207,8 +231,8 @@ public class JsonXContentParser extends AbstractXContentParser {
     public int textOffset() throws IOException {
         try {
             return parser.getTextOffset();
-        } catch (JsonParseException e) {
-            throw newXContentParseException(e);
+        } catch (IOException e) {
+            throw handleParserException(e);
         }
     }
 
@@ -216,8 +240,8 @@ public class JsonXContentParser extends AbstractXContentParser {
     public Number numberValue() throws IOException {
         try {
             return parser.getNumberValue();
-        } catch (InputCoercionException | JsonParseException e) {
-            throw newXContentParseException(e);
+        } catch (IOException e) {
+            throw handleParserException(e);
         }
     }
 
@@ -225,8 +249,8 @@ public class JsonXContentParser extends AbstractXContentParser {
     public short doShortValue() throws IOException {
         try {
             return parser.getShortValue();
-        } catch (InputCoercionException | JsonParseException e) {
-            throw newXContentParseException(e);
+        } catch (IOException e) {
+            throw handleParserException(e);
         }
     }
 
@@ -234,8 +258,8 @@ public class JsonXContentParser extends AbstractXContentParser {
     public int doIntValue() throws IOException {
         try {
             return parser.getIntValue();
-        } catch (InputCoercionException | JsonParseException e) {
-            throw newXContentParseException(e);
+        } catch (IOException e) {
+            throw handleParserException(e);
         }
     }
 
@@ -243,8 +267,8 @@ public class JsonXContentParser extends AbstractXContentParser {
     public long doLongValue() throws IOException {
         try {
             return parser.getLongValue();
-        } catch (InputCoercionException | JsonParseException e) {
-            throw newXContentParseException(e);
+        } catch (IOException e) {
+            throw handleParserException(e);
         }
     }
 
@@ -252,8 +276,8 @@ public class JsonXContentParser extends AbstractXContentParser {
     public float doFloatValue() throws IOException {
         try {
             return parser.getFloatValue();
-        } catch (InputCoercionException | JsonParseException e) {
-            throw newXContentParseException(e);
+        } catch (IOException e) {
+            throw handleParserException(e);
         }
     }
 
@@ -261,8 +285,8 @@ public class JsonXContentParser extends AbstractXContentParser {
     public double doDoubleValue() throws IOException {
         try {
             return parser.getDoubleValue();
-        } catch (InputCoercionException | JsonParseException e) {
-            throw newXContentParseException(e);
+        } catch (IOException e) {
+            throw handleParserException(e);
         }
     }
 
@@ -270,8 +294,8 @@ public class JsonXContentParser extends AbstractXContentParser {
     public byte[] binaryValue() throws IOException {
         try {
             return parser.getBinaryValue();
-        } catch (JsonParseException e) {
-            throw newXContentParseException(e);
+        } catch (IOException e) {
+            throw handleParserException(e);
         }
     }
 

x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/attachment/ReportingAttachmentParserTests.java

@@ -239,7 +239,7 @@ public class ReportingAttachmentParserTests extends ESTestCase {
             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 {