Skip to content

Commit 061bec4

Browse files
smalyshevsmalyshev
authored
[8.19] Improve exception handling for JsonXContentParser (#123439) (#132481)
* 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 * Old Java doesn't have pattern matching, do it the hard way
1 parent b795996 commit 061bec4

File tree

4 files changed

+172
-40
lines changed

4 files changed

+172
-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: 62 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,54 @@ 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+
if (e instanceof JsonEOFException eof) {
74+
throw new XContentEOFException(getLocation(eof), "Unexpected end of file", e);
75+
} else if (e instanceof JsonParseException pe) {
76+
throw newXContentParseException(pe);
77+
} else if (e instanceof InputCoercionException ice) {
78+
throw newXContentParseException(ice);
79+
} else if (e instanceof CharConversionException cce) {
80+
throw new XContentParseException(null, cce.getMessage(), cce);
81+
} else if (e instanceof StreamConstraintsException sce) {
82+
throw newXContentParseException(sce);
83+
} else {
84+
return e;
85+
}
5686
}
5787

5888
@Override
5989
public Token nextToken() throws IOException {
6090
try {
6191
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);
92+
} catch (IOException e) {
93+
throw handleParserException(e);
6794
}
6895
}
6996

7097
@Override
7198
public String nextFieldName() throws IOException {
7299
try {
73100
return parser.nextFieldName();
74-
} catch (JsonParseException e) {
75-
throw newXContentParseException(e);
101+
} catch (IOException e) {
102+
throw handleParserException(e);
76103
}
77104
}
78105

@@ -100,8 +127,8 @@ public String currentName() throws IOException {
100127
protected boolean doBooleanValue() throws IOException {
101128
try {
102129
return parser.getBooleanValue();
103-
} catch (JsonParseException e) {
104-
throw newXContentParseException(e);
130+
} catch (IOException e) {
131+
throw handleParserException(e);
105132
}
106133
}
107134

@@ -112,8 +139,8 @@ public String text() throws IOException {
112139
}
113140
try {
114141
return parser.getText();
115-
} catch (JsonParseException e) {
116-
throw newXContentParseException(e);
142+
} catch (IOException e) {
143+
throw handleParserException(e);
117144
}
118145
}
119146

@@ -139,8 +166,8 @@ private void throwOnNoText() {
139166
public CharBuffer charBuffer() throws IOException {
140167
try {
141168
return CharBuffer.wrap(parser.getTextCharacters(), parser.getTextOffset(), parser.getTextLength());
142-
} catch (JsonParseException e) {
143-
throw newXContentParseException(e);
169+
} catch (IOException e) {
170+
throw handleParserException(e);
144171
}
145172
}
146173

@@ -189,89 +216,89 @@ public boolean hasTextCharacters() {
189216
public char[] textCharacters() throws IOException {
190217
try {
191218
return parser.getTextCharacters();
192-
} catch (JsonParseException e) {
193-
throw newXContentParseException(e);
219+
} catch (IOException e) {
220+
throw handleParserException(e);
194221
}
195222
}
196223

197224
@Override
198225
public int textLength() throws IOException {
199226
try {
200227
return parser.getTextLength();
201-
} catch (JsonParseException e) {
202-
throw newXContentParseException(e);
228+
} catch (IOException e) {
229+
throw handleParserException(e);
203230
}
204231
}
205232

206233
@Override
207234
public int textOffset() throws IOException {
208235
try {
209236
return parser.getTextOffset();
210-
} catch (JsonParseException e) {
211-
throw newXContentParseException(e);
237+
} catch (IOException e) {
238+
throw handleParserException(e);
212239
}
213240
}
214241

215242
@Override
216243
public Number numberValue() throws IOException {
217244
try {
218245
return parser.getNumberValue();
219-
} catch (InputCoercionException | JsonParseException e) {
220-
throw newXContentParseException(e);
246+
} catch (IOException e) {
247+
throw handleParserException(e);
221248
}
222249
}
223250

224251
@Override
225252
public short doShortValue() throws IOException {
226253
try {
227254
return parser.getShortValue();
228-
} catch (InputCoercionException | JsonParseException e) {
229-
throw newXContentParseException(e);
255+
} catch (IOException e) {
256+
throw handleParserException(e);
230257
}
231258
}
232259

233260
@Override
234261
public int doIntValue() throws IOException {
235262
try {
236263
return parser.getIntValue();
237-
} catch (InputCoercionException | JsonParseException e) {
238-
throw newXContentParseException(e);
264+
} catch (IOException e) {
265+
throw handleParserException(e);
239266
}
240267
}
241268

242269
@Override
243270
public long doLongValue() throws IOException {
244271
try {
245272
return parser.getLongValue();
246-
} catch (InputCoercionException | JsonParseException e) {
247-
throw newXContentParseException(e);
273+
} catch (IOException e) {
274+
throw handleParserException(e);
248275
}
249276
}
250277

251278
@Override
252279
public float doFloatValue() throws IOException {
253280
try {
254281
return parser.getFloatValue();
255-
} catch (InputCoercionException | JsonParseException e) {
256-
throw newXContentParseException(e);
282+
} catch (IOException e) {
283+
throw handleParserException(e);
257284
}
258285
}
259286

260287
@Override
261288
public double doDoubleValue() throws IOException {
262289
try {
263290
return parser.getDoubleValue();
264-
} catch (InputCoercionException | JsonParseException e) {
265-
throw newXContentParseException(e);
291+
} catch (IOException e) {
292+
throw handleParserException(e);
266293
}
267294
}
268295

269296
@Override
270297
public byte[] binaryValue() throws IOException {
271298
try {
272299
return parser.getBinaryValue();
273-
} catch (JsonParseException e) {
274-
throw newXContentParseException(e);
300+
} catch (IOException e) {
301+
throw handleParserException(e);
275302
}
276303
}
277304

‎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)