Skip to content

Commit

Permalink
Core: Remove potential NPE while resolving concrete indices from Meta…
Browse files Browse the repository at this point in the history 
…Data

Prevents a current edge case resolving concrete aliases or index names in cluster MetaData
that could potentialy lead to NullPointerException when the IndicesOptions don't allow
wildcard expansion and the method is called with aliasesOrIndices argument null or emtpy list.
This change adds a check for that and introduces randomized test that catches this.

Closes #10342
Closes #10339
  • Loading branch information
Christoph Büscher committed Apr 1, 2015
1 parent 896b410 commit c907dc6
Show file tree
Hide file tree
Showing 2 changed files with 139 additions and 10 deletions.
11 changes: 10 additions & 1 deletion src/main/java/org/elasticsearch/cluster/metadata/MetaData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,15 @@ public String[] concreteIndices(IndicesOptions indicesOptions, String... aliases

aliasesOrIndices = convertFromWildcards(aliasesOrIndices, indicesOptions);
}

if (aliasesOrIndices == null || aliasesOrIndices.length == 0) {
if (!indicesOptions.allowNoIndices()) {
throw new ElasticsearchIllegalArgumentException("no indices were specified and wildcard expansion is disabled.");
} else {
return Strings.EMPTY_ARRAY;
}
}

boolean failClosed = indicesOptions.forbidClosedIndices() && !indicesOptions.ignoreUnavailable();

// optimize for single element index (common case)
Expand Down Expand Up @@ -1067,7 +1076,7 @@ public static boolean isAllIndices(String[] aliasesOrIndices) {
* @param types the array containing index names
* @return true if the provided array maps to all indices, false otherwise
*/
public boolean isAllTypes(String[] types) {
public static boolean isAllTypes(String[] types) {
return types == null || types.length == 0 || isExplicitAllPattern(types);
}

Expand Down
138 changes: 129 additions & 9 deletions src/test/java/org/elasticsearch/cluster/metadata/MetaDataTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
package org.elasticsearch.cluster.metadata;

import com.google.common.collect.Sets;

import org.apache.tools.ant.taskdefs.condition.IsTrue;
import org.elasticsearch.ElasticsearchIllegalArgumentException;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.cluster.metadata.IndexMetaData.State;
Expand Down Expand Up @@ -107,9 +109,15 @@ public void testIndexOptions_strict() {
String[] results = md.concreteIndices(IndicesOptions.strictExpandOpen(), Strings.EMPTY_ARRAY);
assertEquals(3, results.length);

results = md.concreteIndices(IndicesOptions.strictExpandOpen(), null);
assertEquals(3, results.length);

results = md.concreteIndices(IndicesOptions.strictExpand(), Strings.EMPTY_ARRAY);
assertEquals(4, results.length);

results = md.concreteIndices(IndicesOptions.strictExpand(), null);
assertEquals(4, results.length);

results = md.concreteIndices(IndicesOptions.strictExpandOpen(), "foofoo*");
assertEquals(3, results.length);
assertThat(results, arrayContainingInAnyOrder("foo", "foobar", "foofoo"));
Expand Down Expand Up @@ -320,6 +328,12 @@ public void testIndexOptions_noExpandWildcards() {
results = md.concreteIndices(noExpandLenient, "foofoobar");
assertEquals(2, results.length);
assertThat(results, arrayContainingInAnyOrder("foo", "foobar"));

results = md.concreteIndices(noExpandLenient, null);
assertEquals(0, results.length);

results = md.concreteIndices(noExpandLenient, Strings.EMPTY_ARRAY);
assertEquals(0, results.length);
}

//ignore unavailable but don't allow no indices
Expand Down Expand Up @@ -478,7 +492,7 @@ public void testIndexOptions_emptyCluster() {
}

@Test
public void convertWildcardsJustIndicesTests() {
public void testConvertWildcardsJustIndicesTests() {
MetaData.Builder mdBuilder = MetaData.builder()
.put(indexBuilder("testXXX"))
.put(indexBuilder("testXYY"))
Expand All @@ -494,7 +508,7 @@ public void convertWildcardsJustIndicesTests() {
}

@Test
public void convertWildcardsTests() {
public void testConvertWildcardsTests() {
MetaData.Builder mdBuilder = MetaData.builder()
.put(indexBuilder("testXXX").putAlias(AliasMetaData.builder("alias1")).putAlias(AliasMetaData.builder("alias2")))
.put(indexBuilder("testXYY").putAlias(AliasMetaData.builder("alias2")))
Expand All @@ -509,7 +523,7 @@ public void convertWildcardsTests() {
}

@Test
public void convertWildcardsOpenClosedIndicesTests() {
public void testConvertWildcardsOpenClosedIndicesTests() {
MetaData.Builder mdBuilder = MetaData.builder()
.put(indexBuilder("testXXX").state(State.OPEN))
.put(indexBuilder("testXXY").state(State.OPEN))
Expand All @@ -529,7 +543,7 @@ private IndexMetaData.Builder indexBuilder(String index) {
}

@Test(expected = IndexMissingException.class)
public void concreteIndicesIgnoreIndicesOneMissingIndex() {
public void testConcreteIndicesIgnoreIndicesOneMissingIndex() {
MetaData.Builder mdBuilder = MetaData.builder()
.put(indexBuilder("testXXX"))
.put(indexBuilder("kuku"));
Expand All @@ -538,7 +552,7 @@ public void concreteIndicesIgnoreIndicesOneMissingIndex() {
}

@Test
public void concreteIndicesIgnoreIndicesOneMissingIndexOtherFound() {
public void testConcreteIndicesIgnoreIndicesOneMissingIndexOtherFound() {
MetaData.Builder mdBuilder = MetaData.builder()
.put(indexBuilder("testXXX"))
.put(indexBuilder("kuku"));
Expand All @@ -547,7 +561,7 @@ public void concreteIndicesIgnoreIndicesOneMissingIndexOtherFound() {
}

@Test(expected = IndexMissingException.class)
public void concreteIndicesIgnoreIndicesAllMissing() {
public void testConcreteIndicesIgnoreIndicesAllMissing() {
MetaData.Builder mdBuilder = MetaData.builder()
.put(indexBuilder("testXXX"))
.put(indexBuilder("kuku"));
Expand All @@ -556,7 +570,7 @@ public void concreteIndicesIgnoreIndicesAllMissing() {
}

@Test
public void concreteIndicesIgnoreIndicesEmptyRequest() {
public void testConcreteIndicesIgnoreIndicesEmptyRequest() {
MetaData.Builder mdBuilder = MetaData.builder()
.put(indexBuilder("testXXX"))
.put(indexBuilder("kuku"));
Expand All @@ -565,7 +579,7 @@ public void concreteIndicesIgnoreIndicesEmptyRequest() {
}

@Test
public void concreteIndicesWildcardExpansion() {
public void testConcreteIndicesWildcardExpansion() {
MetaData.Builder mdBuilder = MetaData.builder()
.put(indexBuilder("testXXX").state(State.OPEN))
.put(indexBuilder("testXXY").state(State.OPEN))
Expand All @@ -579,6 +593,113 @@ public void concreteIndicesWildcardExpansion() {
assertThat(newHashSet(md.concreteIndices(IndicesOptions.fromOptions(true, true, true, true), "testX*")), equalTo(newHashSet("testXXX", "testXXY", "testXYY")));
}

/**
* test resolving _all pattern (null, empty array or "_all") for random IndicesOptions
*/
@Test
public void testConcreteIndicesAllPatternRandom() {
for (int i = 0; i < 10; i++) {
String[] allIndices = null;
switch (randomIntBetween(0, 2)) {
case 0:
break;
case 1:
allIndices = new String[0];
break;
case 2:
allIndices = new String[] { MetaData.ALL };
break;
}

IndicesOptions indicesOptions = IndicesOptions.fromOptions(randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean());
MetaData metadata = MetaData.builder().build();

// with no indices, asking for all indices should return empty list or exception, depending on indices options
if (indicesOptions.allowNoIndices()) {
String[] concreteIndices = metadata.concreteIndices(indicesOptions, allIndices);
assertThat(concreteIndices, notNullValue());
assertThat(concreteIndices.length, equalTo(0));
} else {
checkCorrectException(metadata, indicesOptions, allIndices);
}

// with existing indices, asking for all indices should return all open/closed indices depending on options
metadata = MetaData.builder()
.put(indexBuilder("aaa").state(State.OPEN).putAlias(AliasMetaData.builder("aaa_alias1")))
.put(indexBuilder("bbb").state(State.OPEN).putAlias(AliasMetaData.builder("bbb_alias1")))
.put(indexBuilder("ccc").state(State.CLOSE).putAlias(AliasMetaData.builder("ccc_alias1")))
.build();
if (indicesOptions.expandWildcardsOpen() || indicesOptions.expandWildcardsClosed() || indicesOptions.allowNoIndices()) {
String[] concreteIndices = metadata.concreteIndices(indicesOptions, allIndices);
assertThat(concreteIndices, notNullValue());
int expectedNumberOfIndices = 0;
if (indicesOptions.expandWildcardsOpen()) {
expectedNumberOfIndices += 2;
}
if (indicesOptions.expandWildcardsClosed()) {
expectedNumberOfIndices += 1;
}
assertThat(concreteIndices.length, equalTo(expectedNumberOfIndices));
} else {
checkCorrectException(metadata, indicesOptions, allIndices);
}
}
}

/**
* check for correct exception type depending on indicesOptions and provided index name list
*/
private void checkCorrectException(MetaData metadata, IndicesOptions indicesOptions, String[] allIndices) {
// two different exception types possible
if (!(indicesOptions.expandWildcardsOpen() || indicesOptions.expandWildcardsClosed())
&& (allIndices == null || allIndices.length == 0)) {
try {
metadata.concreteIndices(indicesOptions, allIndices);
fail("no wildcard expansion and null or empty list argument should trigger ElasticsearchIllegalArgumentException");
} catch (ElasticsearchIllegalArgumentException e) {
// expected
}
} else {
try {
metadata.concreteIndices(indicesOptions, allIndices);
fail("wildcard expansion on should trigger IndexMissingException");
} catch (IndexMissingException e) {
// expected
}
}
}

/**
* test resolving wildcard pattern that matches no index of alias for random IndicesOptions
*/
@Test
public void testConcreteIndicesWildcardNoMatch() {
for (int i = 0; i < 10; i++) {
IndicesOptions indicesOptions = IndicesOptions.fromOptions(randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean());
MetaData metadata = MetaData.builder().build();

metadata = MetaData.builder()
.put(indexBuilder("aaa").state(State.OPEN).putAlias(AliasMetaData.builder("aaa_alias1")))
.put(indexBuilder("bbb").state(State.OPEN).putAlias(AliasMetaData.builder("bbb_alias1")))
.put(indexBuilder("ccc").state(State.CLOSE).putAlias(AliasMetaData.builder("ccc_alias1")))
.build();

// asking for non existing wildcard pattern should return empty list or exception
if (indicesOptions.allowNoIndices()) {
String[] concreteIndices = metadata.concreteIndices(indicesOptions, "Foo*");
assertThat(concreteIndices, notNullValue());
assertThat(concreteIndices.length, equalTo(0));
} else {
try {
metadata.concreteIndices(indicesOptions, "Foo*");
fail("expecting exeption when result empty and allowNoIndicec=false");
} catch (IndexMissingException e) {
// expected exception
}
}
}
}

@Test
public void testIsAllIndices_null() throws Exception {
assertThat(MetaData.isAllIndices(null), equalTo(true));
Expand Down Expand Up @@ -636,7 +757,6 @@ public void testIsExplicitAllIndices_normalIndexes() throws Exception {

@Test
public void testIsExplicitAllIndices_wildcard() throws Exception {
MetaData metaData = MetaData.builder().build();
assertThat(MetaData.isExplicitAllPattern(new String[]{"*"}), equalTo(false));
}

Expand Down

0 comments on commit c907dc6

Please sign in to comment.