honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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.
This commit is contained in:
Jason Tedor 2019-06-07 16:10:10 -04:00
parent 5bc0dfce94
commit d6fe4b648d
No known key found for this signature in database
GPG Key ID: FA89F05560F16BC5
2 changed files with 93 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -126,10 +126,11 @@ public class TransportPutMappingAction extends TransportMasterNodeAction<PutMapp
this.validators = validators;
}
private Exception validateRequest(PutMappingRequest request, ClusterState state, Index[] indices) {
Exception validateRequest(PutMappingRequest request, ClusterState state, Index[] indices) {
Exception firstException = null;
for (MappingRequestValidator validator : validators) {
final Exception e = validator.validateRequest(request, state, indices);
if (e == null) continue;
if (firstException == null) {
firstException = e;
} else {

91
server/src/test/java/org/elasticsearch/action/admin/indices/mapping/put/TransportPutMappingRequestValidatorsTests.java Normal file
View File

@ -0,0 +1,91 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.action.admin.indices.mapping.put;
import org.elasticsearch.common.Randomness;
import org.elasticsearch.test.ESTestCase;
import org.hamcrest.Matchers;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
public class TransportPutMappingRequestValidatorsTests extends ESTestCase {
private final MappingRequestValidator EMPTY = (request, state, indices) -> null;
private final MappingRequestValidator FAIL = (request, state, indices) -> new Exception("failure");
public void testValidates() {
final int numberOfValidations = randomIntBetween(0, 8);
final List<MappingRequestValidator> validators = new ArrayList<>(numberOfValidations);
for (int i = 0; i < numberOfValidations; i++) {
validators.add(EMPTY);
}
final TransportPutMappingAction.RequestValidators requestValidators = new TransportPutMappingAction.RequestValidators(validators);
assertNull(requestValidators.validateRequest(null, null, null));
}
public void testFailure() {
final TransportPutMappingAction.RequestValidators validators =
new TransportPutMappingAction.RequestValidators(Collections.singletonList(FAIL));
assertNotNull(validators.validateRequest(null, null, null));
}
public void testValidatesAfterFailure() {
final TransportPutMappingAction.RequestValidators validators =
new TransportPutMappingAction.RequestValidators(Collections.unmodifiableList(Arrays.asList(FAIL, EMPTY)));
assertNotNull(validators.validateRequest(null, null, null));
}
public void testMultipleFailures() {
final int numberOfFailures = randomIntBetween(2, 8);
final List<MappingRequestValidator> validators = new ArrayList<>(numberOfFailures);
for (int i = 0; i < numberOfFailures; i++) {
validators.add(FAIL);
}
final TransportPutMappingAction.RequestValidators requestValidators = new TransportPutMappingAction.RequestValidators(validators);
final Exception e = requestValidators.validateRequest(null, null, null);
assertNotNull(e);
assertThat(e.getSuppressed(), Matchers.arrayWithSize(numberOfFailures - 1));
}
public void testRandom() {
final int numberOfValidations = randomIntBetween(0, 8);
final int numberOfFailures = randomIntBetween(0, 8);
final List<MappingRequestValidator> validators = new ArrayList<>(numberOfValidations + numberOfFailures);
for (int i = 0; i < numberOfValidations; i++) {
validators.add(EMPTY);
}
for (int i = 0; i < numberOfFailures; i++) {
validators.add(FAIL);
}
Randomness.shuffle(validators);
final TransportPutMappingAction.RequestValidators requestValidators = new TransportPutMappingAction.RequestValidators(validators);
final Exception e = requestValidators.validateRequest(null, null, null);
if (numberOfFailures == 0) {
assertNull(e);
} else {
assertNotNull(e);
}
assertThat(e.getSuppressed(), Matchers.arrayWithSize(numberOfFailures - 1));
}
}