Skip to content

Commit b953dce

Browse files
authored
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 630a4a8 commit b953dce

File tree

2 files changed

+90
-1
lines changed

2 files changed

+90
-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,88 @@
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.List;
28+
29+
public class TransportPutMappingRequestValidatorsTests extends ESTestCase {
30+
31+
private final MappingRequestValidator EMPTY = (request, state, indices) -> null;
32+
private final MappingRequestValidator FAIL = (request, state, indices) -> new Exception("failure");
33+
34+
public void testValidates() {
35+
final int numberOfValidations = randomIntBetween(0, 8);
36+
final List<MappingRequestValidator> validators = new ArrayList<>(numberOfValidations);
37+
for (int i = 0; i < numberOfValidations; i++) {
38+
validators.add(EMPTY);
39+
}
40+
final TransportPutMappingAction.RequestValidators requestValidators = new TransportPutMappingAction.RequestValidators(validators);
41+
assertNull(requestValidators.validateRequest(null, null, null));
42+
}
43+
44+
public void testFailure() {
45+
final TransportPutMappingAction.RequestValidators validators = new TransportPutMappingAction.RequestValidators(List.of(FAIL));
46+
assertNotNull(validators.validateRequest(null, null, null));
47+
}
48+
49+
public void testValidatesAfterFailure() {
50+
final TransportPutMappingAction.RequestValidators validators =
51+
new TransportPutMappingAction.RequestValidators(List.of(FAIL, EMPTY));
52+
assertNotNull(validators.validateRequest(null, null, null));
53+
}
54+
55+
public void testMultipleFailures() {
56+
final int numberOfFailures = randomIntBetween(2, 8);
57+
final List<MappingRequestValidator> validators = new ArrayList<>(numberOfFailures);
58+
for (int i = 0; i < numberOfFailures; i++) {
59+
validators.add(FAIL);
60+
}
61+
final TransportPutMappingAction.RequestValidators requestValidators = new TransportPutMappingAction.RequestValidators(validators);
62+
final Exception e = requestValidators.validateRequest(null, null, null);
63+
assertNotNull(e);
64+
assertThat(e.getSuppressed(), Matchers.arrayWithSize(numberOfFailures - 1));
65+
}
66+
67+
public void testRandom() {
68+
final int numberOfValidations = randomIntBetween(0, 8);
69+
final int numberOfFailures = randomIntBetween(0, 8);
70+
final List<MappingRequestValidator> validators = new ArrayList<>(numberOfValidations + numberOfFailures);
71+
for (int i = 0; i < numberOfValidations; i++) {
72+
validators.add(EMPTY);
73+
}
74+
for (int i = 0; i < numberOfFailures; i++) {
75+
validators.add(FAIL);
76+
}
77+
Randomness.shuffle(validators);
78+
final TransportPutMappingAction.RequestValidators requestValidators = new TransportPutMappingAction.RequestValidators(validators);
79+
final Exception e = requestValidators.validateRequest(null, null, null);
80+
if (numberOfFailures == 0) {
81+
assertNull(e);
82+
} else {
83+
assertNotNull(e);
84+
}
85+
assertThat(e.getSuppressed(), Matchers.arrayWithSize(numberOfFailures - 1));
86+
}
87+
88+
}

0 commit comments

Comments
 (0)