Skip to content

Commit 91b6267

Browse files
smalyshevsmalyshev
authored
Improve exception handling for JsonXContentParser (#123439)
* 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
1 parent 3a8f3a7 commit 91b6267

File tree

4 files changed

+169
-40
lines changed

4 files changed

+169
-40
lines changed

‎benchmarks/src/main/java/org/elasticsearch/benchmark/xcontent/JsonParserBenchmark.java

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.benchmark.xcontent;
11+
12+
import org.elasticsearch.common.bytes.BytesReference;
13+
import org.elasticsearch.common.io.Streams;
14+
import org.elasticsearch.xcontent.XContentParser;
15+
import org.elasticsearch.xcontent.XContentParserConfiguration;
16+
import org.elasticsearch.xcontent.XContentType;
17+
import org.openjdk.jmh.annotations.Benchmark;
18+
import org.openjdk.jmh.annotations.BenchmarkMode;
19+
import org.openjdk.jmh.annotations.Fork;
20+
import org.openjdk.jmh.annotations.Level;
21+
import org.openjdk.jmh.annotations.Measurement;
22+
import org.openjdk.jmh.annotations.Mode;
23+
import org.openjdk.jmh.annotations.OutputTimeUnit;
24+
import org.openjdk.jmh.annotations.Scope;
25+
import org.openjdk.jmh.annotations.Setup;
26+
import org.openjdk.jmh.annotations.State;
27+
import org.openjdk.jmh.annotations.Warmup;
28+
import org.openjdk.jmh.infra.Blackhole;
29+
30+
import java.io.IOException;
31+
import java.util.Arrays;
32+
import java.util.Collections;
33+
import java.util.List;
34+
import java.util.Map;
35+
import java.util.Random;
36+
import java.util.concurrent.TimeUnit;
37+
import java.util.stream.Collectors;
38+
39+
@Fork(1)
40+
@Warmup(iterations = 5)
41+
@Measurement(iterations = 10)
42+
@BenchmarkMode(Mode.AverageTime)
43+
@OutputTimeUnit(TimeUnit.NANOSECONDS)
44+
@State(Scope.Thread)
45+
public class JsonParserBenchmark {
46+
private Map<String, BytesReference> sourceBytes;
47+
private BytesReference source;
48+
private Random random;
49+
private List<String> sourcesRandomized;
50+
51+
final String[] sources = new String[] { "monitor_cluster_stats.json", "monitor_index_stats.json", "monitor_node_stats.json" };
52+
53+
@Setup(Level.Iteration)
54+
public void randomizeSource() {
55+
sourcesRandomized = Arrays.asList(sources);
56+
Collections.shuffle(sourcesRandomized, random);
57+
}
58+
59+
@Setup(Level.Trial)
60+
public void setup() throws IOException {
61+
random = new Random();
62+
sourceBytes = Arrays.stream(sources).collect(Collectors.toMap(s -> s, s -> {
63+
try {
64+
return readSource(s);
65+
} catch (IOException e) {
66+
throw new RuntimeException(e);
67+
}
68+
}));
69+
}
70+
71+
@Benchmark
72+
public void parseJson(Blackhole bh) throws IOException {
73+
sourcesRandomized.forEach(source -> {
74+
try {
75+
final XContentParser parser = XContentType.JSON.xContent()
76+
.createParser(XContentParserConfiguration.EMPTY, sourceBytes.get(source).streamInput());
77+
bh.consume(parser.mapOrdered());
78+
} catch (IOException e) {
79+
throw new RuntimeException(e);
80+
}
81+
});
82+
}
83+
84+
private BytesReference readSource(String fileName) throws IOException {
85+
return Streams.readFully(JsonParserBenchmark.class.getResourceAsStream(fileName));
86+
}
87+
}

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

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
import java.io.Reader;
2929
import java.util.Set;
3030

31+
import static org.elasticsearch.xcontent.provider.json.JsonXContentParser.handleParserException;
32+
3133
/**
3234
* A JSON based content implementation using Jackson.
3335
*/
@@ -95,21 +97,37 @@ private XContentParser createParser(XContentParserConfiguration config, JsonPars
9597

9698
@Override
9799
public XContentParser createParser(XContentParserConfiguration config, String content) throws IOException {
98-
return createParser(config, jsonFactory.createParser(content));
100+
try {
101+
return createParser(config, jsonFactory.createParser(content));
102+
} catch (IOException e) {
103+
throw handleParserException(e);
104+
}
99105
}
100106

101107
@Override
102108
public XContentParser createParser(XContentParserConfiguration config, InputStream is) throws IOException {
103-
return createParser(config, jsonFactory.createParser(is));
109+
try {
110+
return createParser(config, jsonFactory.createParser(is));
111+
} catch (IOException e) {
112+
throw handleParserException(e);
113+
}
104114
}
105115

106116
@Override
107117
public XContentParser createParser(XContentParserConfiguration config, byte[] data, int offset, int length) throws IOException {
108-
return createParser(config, jsonFactory.createParser(data, offset, length));
118+
try {
119+
return createParser(config, jsonFactory.createParser(data, offset, length));
120+
} catch (IOException e) {
121+
throw handleParserException(e);
122+
}
109123
}
110124

111125
@Override
112126
public XContentParser createParser(XContentParserConfiguration config, Reader reader) throws IOException {
113-
return createParser(config, jsonFactory.createParser(reader));
127+
try {
128+
return createParser(config, jsonFactory.createParser(reader));
129+
} catch (IOException e) {
130+
throw handleParserException(e);
131+
}
114132
}
115133
}

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

Lines changed: 59 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import com.fasterxml.jackson.core.JsonProcessingException;
1616
import com.fasterxml.jackson.core.JsonToken;
1717
import com.fasterxml.jackson.core.exc.InputCoercionException;
18+
import com.fasterxml.jackson.core.exc.StreamConstraintsException;
1819
import com.fasterxml.jackson.core.io.JsonEOFException;
1920

2021
import org.elasticsearch.core.IOUtils;
@@ -28,6 +29,7 @@
2829
import org.elasticsearch.xcontent.provider.XContentParserConfigurationImpl;
2930
import org.elasticsearch.xcontent.support.AbstractXContentParser;
3031

32+
import java.io.CharConversionException;
3133
import java.io.IOException;
3234
import java.nio.CharBuffer;
3335

@@ -50,29 +52,51 @@ public void allowDuplicateKeys(boolean allowDuplicateKeys) {
5052
parser.configure(JsonParser.Feature.STRICT_DUPLICATE_DETECTION, allowDuplicateKeys == false);
5153
}
5254

53-
private static XContentParseException newXContentParseException(JsonProcessingException e) {
55+
private static XContentLocation getLocation(JsonProcessingException e) {
5456
JsonLocation loc = e.getLocation();
55-
throw new XContentParseException(new XContentLocation(loc.getLineNr(), loc.getColumnNr()), e.getMessage(), e);
57+
if (loc != null) {
58+
return new XContentLocation(loc.getLineNr(), loc.getColumnNr());
59+
} else {
60+
return null;
61+
}
62+
}
63+
64+
private static XContentParseException newXContentParseException(JsonProcessingException e) {
65+
return new XContentParseException(getLocation(e), e.getMessage(), e);
66+
}
67+
68+
/**
69+
* Handle parser exception depending on type.
70+
* This converts known exceptions to XContentParseException and rethrows them.
71+
*/
72+
static IOException handleParserException(IOException e) throws IOException {
73+
switch (e) {
74+
case JsonEOFException eof -> throw new XContentEOFException(getLocation(eof), "Unexpected end of file", e);
75+
case JsonParseException pe -> throw newXContentParseException(pe);
76+
case InputCoercionException ice -> throw newXContentParseException(ice);
77+
case CharConversionException cce -> throw new XContentParseException(null, cce.getMessage(), cce);
78+
case StreamConstraintsException sce -> throw newXContentParseException(sce);
79+
default -> {
80+
return e;
81+
}
82+
}
5683
}
5784

5885
@Override
5986
public Token nextToken() throws IOException {
6087
try {
6188
return convertToken(parser.nextToken());
62-
} catch (JsonEOFException e) {
63-
JsonLocation location = e.getLocation();
64-
throw new XContentEOFException(new XContentLocation(location.getLineNr(), location.getColumnNr()), "Unexpected end of file", e);
65-
} catch (JsonParseException e) {
66-
throw newXContentParseException(e);
89+
} catch (IOException e) {
90+
throw handleParserException(e);
6791
}
6892
}
6993

7094
@Override
7195
public String nextFieldName() throws IOException {
7296
try {
7397
return parser.nextFieldName();
74-
} catch (JsonParseException e) {
75-
throw newXContentParseException(e);
98+
} catch (IOException e) {
99+
throw handleParserException(e);
76100
}
77101
}
78102

@@ -100,8 +124,8 @@ public String currentName() throws IOException {
100124
protected boolean doBooleanValue() throws IOException {
101125
try {
102126
return parser.getBooleanValue();
103-
} catch (JsonParseException e) {
104-
throw newXContentParseException(e);
127+
} catch (IOException e) {
128+
throw handleParserException(e);
105129
}
106130
}
107131

@@ -112,8 +136,8 @@ public String text() throws IOException {
112136
}
113137
try {
114138
return parser.getText();
115-
} catch (JsonParseException e) {
116-
throw newXContentParseException(e);
139+
} catch (IOException e) {
140+
throw handleParserException(e);
117141
}
118142
}
119143

@@ -139,8 +163,8 @@ private void throwOnNoText() {
139163
public CharBuffer charBuffer() throws IOException {
140164
try {
141165
return CharBuffer.wrap(parser.getTextCharacters(), parser.getTextOffset(), parser.getTextLength());
142-
} catch (JsonParseException e) {
143-
throw newXContentParseException(e);
166+
} catch (IOException e) {
167+
throw handleParserException(e);
144168
}
145169
}
146170

@@ -189,89 +213,89 @@ public boolean hasTextCharacters() {
189213
public char[] textCharacters() throws IOException {
190214
try {
191215
return parser.getTextCharacters();
192-
} catch (JsonParseException e) {
193-
throw newXContentParseException(e);
216+
} catch (IOException e) {
217+
throw handleParserException(e);
194218
}
195219
}
196220

197221
@Override
198222
public int textLength() throws IOException {
199223
try {
200224
return parser.getTextLength();
201-
} catch (JsonParseException e) {
202-
throw newXContentParseException(e);
225+
} catch (IOException e) {
226+
throw handleParserException(e);
203227
}
204228
}
205229

206230
@Override
207231
public int textOffset() throws IOException {
208232
try {
209233
return parser.getTextOffset();
210-
} catch (JsonParseException e) {
211-
throw newXContentParseException(e);
234+
} catch (IOException e) {
235+
throw handleParserException(e);
212236
}
213237
}
214238

215239
@Override
216240
public Number numberValue() throws IOException {
217241
try {
218242
return parser.getNumberValue();
219-
} catch (InputCoercionException | JsonParseException e) {
220-
throw newXContentParseException(e);
243+
} catch (IOException e) {
244+
throw handleParserException(e);
221245
}
222246
}
223247

224248
@Override
225249
public short doShortValue() throws IOException {
226250
try {
227251
return parser.getShortValue();
228-
} catch (InputCoercionException | JsonParseException e) {
229-
throw newXContentParseException(e);
252+
} catch (IOException e) {
253+
throw handleParserException(e);
230254
}
231255
}
232256

233257
@Override
234258
public int doIntValue() throws IOException {
235259
try {
236260
return parser.getIntValue();
237-
} catch (InputCoercionException | JsonParseException e) {
238-
throw newXContentParseException(e);
261+
} catch (IOException e) {
262+
throw handleParserException(e);
239263
}
240264
}
241265

242266
@Override
243267
public long doLongValue() throws IOException {
244268
try {
245269
return parser.getLongValue();
246-
} catch (InputCoercionException | JsonParseException e) {
247-
throw newXContentParseException(e);
270+
} catch (IOException e) {
271+
throw handleParserException(e);
248272
}
249273
}
250274

251275
@Override
252276
public float doFloatValue() throws IOException {
253277
try {
254278
return parser.getFloatValue();
255-
} catch (InputCoercionException | JsonParseException e) {
256-
throw newXContentParseException(e);
279+
} catch (IOException e) {
280+
throw handleParserException(e);
257281
}
258282
}
259283

260284
@Override
261285
public double doDoubleValue() throws IOException {
262286
try {
263287
return parser.getDoubleValue();
264-
} catch (InputCoercionException | JsonParseException e) {
265-
throw newXContentParseException(e);
288+
} catch (IOException e) {
289+
throw handleParserException(e);
266290
}
267291
}
268292

269293
@Override
270294
public byte[] binaryValue() throws IOException {
271295
try {
272296
return parser.getBinaryValue();
273-
} catch (JsonParseException e) {
274-
throw newXContentParseException(e);
297+
} catch (IOException e) {
298+
throw handleParserException(e);
275299
}
276300
}
277301

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ public void testInitialRequestContainsInvalidPayload() throws Exception {
239239
XContentParseException.class,
240240
() -> reportingAttachmentParser.toAttachment(createWatchExecutionContext(), Payload.EMPTY, attachment)
241241
);
242-
assertThat(e.getMessage(), containsString("Unexpected end-of-input"));
242+
assertThat(e.getMessage(), containsString("Unexpected end of file"));
243243
}
244244

245245
public void testInitialRequestContainsPathAsObject() throws Exception {

0 commit comments

Comments
 (0)