Skip to content

Commit

Permalink
Aliases: Throw exception if index is null when creating alias
Browse files Browse the repository at this point in the history
Fixes a bug where alias creation would allow `null` for index name, which thereby
applied the alias to _all_ indices.  This patch makes the validator throw an
exception if the index is null.

```bash
POST /_aliases
{
   "actions": [
      {
         "add": {
            "alias": "empty-alias",
            "index": null
         }
      }
   ]
}
```
```json
{
   "error": "ActionRequestValidationException[Validation Failed: 1: Alias action [add]: [index] may not be null;]",
   "status": 400
}
```

The reason this bug wasn't caught by the existing tests is because
the old test for nullness only validated against a cluster which had
zero indices.  The null index is translated into "_all", and since
there are no indices, this fails because the index doesn't exist.
 So the test passes.

However, as soon as you add an index, "_all" resolves and you get the
situation described in the original bug report:  null index is
accepted by the alias, resolves to "_all" and gets applied to everything.

Fixes #7976
  • Loading branch information
polyfractal committed Oct 10, 2014
1 parent fcea4c5 commit ca92987
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 9 deletions.
Expand Up @@ -294,10 +294,6 @@ public ActionRequestValidationException validate() {
+ "]: [alias] may not be empty string", validationException);
}
}
if (CollectionUtils.isEmpty(aliasAction.indices)) {
validationException = addValidationError("Alias action [" + aliasAction.actionType().name().toLowerCase(Locale.ENGLISH)
+ "]: indices may not be empty", validationException);
}
}
if (!CollectionUtils.isEmpty(aliasAction.indices)) {
for (String index : aliasAction.indices) {
Expand All @@ -306,6 +302,9 @@ public ActionRequestValidationException validate() {
+ "]: [index] may not be empty string", validationException);
}
}
} else {
validationException = addValidationError("Alias action [" + aliasAction.actionType().name().toLowerCase(Locale.ENGLISH)
+ "]: Property [index] was either missing or null", validationException);
}
}
return validationException;
Expand Down
32 changes: 27 additions & 5 deletions src/test/java/org/elasticsearch/aliases/IndexAliasesTests.java
Expand Up @@ -744,9 +744,31 @@ public void testIndicesGetAliases() throws Exception {
assertThat(existsResponse.exists(), equalTo(false));
}

@Test(expected = IndexMissingException.class)
public void testAddAliasNullIndex() {
admin().indices().prepareAliases().addAliasAction(AliasAction.newAddAliasAction(null, "alias1")).get();
@Test
public void testAddAliasNullWithoutExistingIndices() {
try {
assertAcked(admin().indices().prepareAliases().addAliasAction(AliasAction.newAddAliasAction(null, "alias1")));
fail("create alias should have failed due to null index");
} catch (ElasticsearchIllegalArgumentException e) {
assertThat(e.getMessage(), equalTo("Validation Failed: 1: Alias action [add]: [index] may not be null;"));
}

}

@Test
public void testAddAliasNullWithExistingIndices() throws Exception {
logger.info("--> creating index [test]");
createIndex("test");
ensureGreen();

logger.info("--> aliasing index [null] with [empty-alias]");

try {
assertAcked(admin().indices().prepareAliases().addAlias((String) null, "empty-alias"));
fail("create alias should have failed due to null index");
} catch (ElasticsearchIllegalArgumentException e) {
assertThat(e.getMessage(), equalTo("Validation Failed: 1: Alias action [add]: [index] may not be null;"));
}
}

@Test(expected = ActionRequestValidationException.class)
Expand All @@ -771,7 +793,7 @@ public void testAddAliasNullAliasNullIndex() {
assertTrue("Should throw " + ActionRequestValidationException.class.getSimpleName(), false);
} catch (ActionRequestValidationException e) {
assertThat(e.validationErrors(), notNullValue());
assertThat(e.validationErrors().size(), equalTo(1));
assertThat(e.validationErrors().size(), equalTo(2));
}
}

Expand Down Expand Up @@ -928,7 +950,7 @@ public void testAddAliasWithFilterNoMapping() throws Exception {
.addAlias("test", "a", FilterBuilders.matchAllFilter()) // <-- no fail, b/c no field mentioned
.get();
}

private void checkAliases() {
GetAliasesResponse getAliasesResponse = admin().indices().prepareGetAliases("alias1").get();
assertThat(getAliasesResponse.getAliases().get("test").size(), equalTo(1));
Expand Down

0 comments on commit ca92987

Please sign in to comment.