Skip to content

Commit 94e4caf

Browse files
committed
Fix possible NPE in put mapping validators (#43000)
When applying put mapping validators, we apply all the validators in the collection. If a failure occurs, we collect that as a top-level exception, and suppress any additional failures into the top-level exception. However, if a request passes the validator after a top-level exception has been collected, we would try to suppress a null exception into the top-level exception. This is a violation of the Throwable#addSuppressed API. This commit addresses this, and adds test to cover the logic of collecting the failures when validating a put mapping request.
1 parent f0ceb0a commit 94e4caf

File tree

2 files changed

+93
-1
lines changed

2 files changed

+93
-1
lines changed

server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/TransportPutMappingAction.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,10 +126,11 @@ public RequestValidators(Collection<MappingRequestValidator> validators) {
126126
this.validators = validators;
127127
}
128128

129-
private Exception validateRequest(PutMappingRequest request, ClusterState state, Index[] indices) {
129+
Exception validateRequest(PutMappingRequest request, ClusterState state, Index[] indices) {
130130
Exception firstException = null;
131131
for (MappingRequestValidator validator : validators) {
132132
final Exception e = validator.validateRequest(request, state, indices);
133+
if (e == null) continue;
133134
if (firstException == null) {
134135
firstException = e;
135136
} else {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.action.admin.indices.mapping.put;
21+
22+
import org.elasticsearch.common.Randomness;
23+
import org.elasticsearch.test.ESTestCase;
24+
import org.hamcrest.Matchers;
25+
26+
import java.util.ArrayList;
27+
import java.util.Arrays;
28+
import java.util.Collections;
29+
import java.util.List;
30+
31+
public class TransportPutMappingRequestValidatorsTests extends ESTestCase {
32+
33+
private final MappingRequestValidator EMPTY = (request, state, indices) -> null;
34+
private final MappingRequestValidator FAIL = (request, state, indices) -> new Exception("failure");
35+
36+
public void testValidates() {
37+
final int numberOfValidations = randomIntBetween(0, 8);
38+
final List<MappingRequestValidator> validators = new ArrayList<>(numberOfValidations);
39+
for (int i = 0; i < numberOfValidations; i++) {
40+
validators.add(EMPTY);
41+
}
42+
final TransportPutMappingAction.RequestValidators requestValidators = new TransportPutMappingAction.RequestValidators(validators);
43+
assertNull(requestValidators.validateRequest(null, null, null));
44+
}
45+
46+
public void testFailure() {
47+
final TransportPutMappingAction.RequestValidators validators =
48+
new TransportPutMappingAction.RequestValidators(Collections.singletonList(FAIL));
49+
assertNotNull(validators.validateRequest(null, null, null));
50+
}
51+
52+
public void testValidatesAfterFailure() {
53+
final TransportPutMappingAction.RequestValidators validators =
54+
new TransportPutMappingAction.RequestValidators(Collections.unmodifiableList(Arrays.asList(FAIL, EMPTY)));
55+
assertNotNull(validators.validateRequest(null, null, null));
56+
}
57+
58+
public void testMultipleFailures() {
59+
final int numberOfFailures = randomIntBetween(2, 8);
60+
final List<MappingRequestValidator> validators = new ArrayList<>(numberOfFailures);
61+
for (int i = 0; i < numberOfFailures; i++) {
62+
validators.add(FAIL);
63+
}
64+
final TransportPutMappingAction.RequestValidators requestValidators = new TransportPutMappingAction.RequestValidators(validators);
65+
final Exception e = requestValidators.validateRequest(null, null, null);
66+
assertNotNull(e);
67+
assertThat(e.getSuppressed(), Matchers.arrayWithSize(numberOfFailures - 1));
68+
}
69+
70+
public void testRandom() {
71+
final int numberOfValidations = randomIntBetween(0, 8);
72+
final int numberOfFailures = randomIntBetween(0, 8);
73+
final List<MappingRequestValidator> validators = new ArrayList<>(numberOfValidations + numberOfFailures);
74+
for (int i = 0; i < numberOfValidations; i++) {
75+
validators.add(EMPTY);
76+
}
77+
for (int i = 0; i < numberOfFailures; i++) {
78+
validators.add(FAIL);
79+
}
80+
Randomness.shuffle(validators);
81+
final TransportPutMappingAction.RequestValidators requestValidators = new TransportPutMappingAction.RequestValidators(validators);
82+
final Exception e = requestValidators.validateRequest(null, null, null);
83+
if (numberOfFailures == 0) {
84+
assertNull(e);
85+
} else {
86+
assertNotNull(e);
87+
}
88+
assertThat(e.getSuppressed(), Matchers.arrayWithSize(numberOfFailures - 1));
89+
}
90+
91+
}

0 commit comments

Comments
 (0)