From b0a85f6ca38ff741f3d9d89e7cac8be38d970ed8 Mon Sep 17 00:00:00 2001
From: Matthew L Daniel <mdaniel@gmail.com>
Date: Wed, 9 Apr 2014 10:44:53 -0700
Subject: [PATCH] Guard against improper auto_expand_replica values

Previously if the user provided a non-conforming string, it would blow up with
`java.lang.StringIndexOutOfBoundsException: String index out of range: -1`
which is not a *helpful* error message.

Also updated the documentation to make the possible setting values more clear.

Close #5752
---
 .../indices/update-settings.asciidoc          |  5 +-
 .../MetaDataUpdateSettingsService.java        | 46 +++++++++++++------
 2 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/docs/reference/indices/update-settings.asciidoc b/docs/reference/indices/update-settings.asciidoc
index 19af576d75d..8bf3930b480 100644
--- a/docs/reference/indices/update-settings.asciidoc
+++ b/docs/reference/indices/update-settings.asciidoc
@@ -34,8 +34,9 @@ settings API:
 `index.number_of_replicas`::
     The number of replicas each shard has.
 
-`index.auto_expand_replicas`::
-    Set to an actual value (like `0-all`) or `false` to disable it.
+`index.auto_expand_replicas` (string)::
+    Set to a dash delimited lower and upper bound (e.g. `0-5`)
+    or one may use `all` as the upper bound (e.g. `0-all`), or `false` to disable it.
 
 `index.blocks.read_only`::
     Set to `true` to have the index read only, `false` to allow writes
diff --git a/src/main/java/org/elasticsearch/cluster/metadata/MetaDataUpdateSettingsService.java b/src/main/java/org/elasticsearch/cluster/metadata/MetaDataUpdateSettingsService.java
index c9a81087f43..8b297b7bddf 100644
--- a/src/main/java/org/elasticsearch/cluster/metadata/MetaDataUpdateSettingsService.java
+++ b/src/main/java/org/elasticsearch/cluster/metadata/MetaDataUpdateSettingsService.java
@@ -49,6 +49,9 @@ import java.util.*;
  */
 public class MetaDataUpdateSettingsService extends AbstractComponent implements ClusterStateListener {
 
+    // the value we recognize in the "max" position to mean all the nodes
+    private static final String ALL_NODES_VALUE = "all";
+
     private final ClusterService clusterService;
 
     private final AllocationService allocationService;
@@ -70,6 +73,8 @@ public class MetaDataUpdateSettingsService extends AbstractComponent implements
         if (!event.state().nodes().localNodeMaster()) {
             return;
         }
+        // we will want to know this for translating "all" to a number
+        final int dataNodeCount = event.state().nodes().dataNodes().size();
 
         Map<Integer, List<String>> nrReplicasChanged = new HashMap<>();
 
@@ -78,22 +83,37 @@ public class MetaDataUpdateSettingsService extends AbstractComponent implements
             String autoExpandReplicas = indexMetaData.settings().get(IndexMetaData.SETTING_AUTO_EXPAND_REPLICAS);
             if (autoExpandReplicas != null && Booleans.parseBoolean(autoExpandReplicas, true)) { // Booleans only work for false values, just as we want it here
                 try {
-                    int min;
-                    int max;
-                    try {
-                        min = Integer.parseInt(autoExpandReplicas.substring(0, autoExpandReplicas.indexOf('-')));
-                        String sMax = autoExpandReplicas.substring(autoExpandReplicas.indexOf('-') + 1);
-                        if (sMax.equals("all")) {
-                            max = event.state().nodes().dataNodes().size() - 1;
-                        } else {
-                            max = Integer.parseInt(sMax);
-                        }
-                    } catch (Exception e) {
-                        logger.warn("failed to set [{}], wrong format [{}]", e, IndexMetaData.SETTING_AUTO_EXPAND_REPLICAS, autoExpandReplicas);
+                    final int min;
+                    final int max;
+
+                    final int dash = autoExpandReplicas.indexOf('-');
+                    if (-1 == dash) {
+                        logger.warn("Unexpected value [{}] for setting [{}]; it should be dash delimited",
+                                autoExpandReplicas, IndexMetaData.SETTING_AUTO_EXPAND_REPLICAS);
                         continue;
                     }
+                    final String sMin = autoExpandReplicas.substring(0, dash);
+                    try {
+                        min = Integer.parseInt(sMin);
+                    } catch (NumberFormatException e) {
+                        logger.warn("failed to set [{}], minimum value is not a number [{}]",
+                                e, IndexMetaData.SETTING_AUTO_EXPAND_REPLICAS, sMin);
+                        continue;
+                    }
+                    String sMax = autoExpandReplicas.substring(dash + 1);
+                    if (sMax.equals(ALL_NODES_VALUE)) {
+                        max = dataNodeCount - 1;
+                    } else {
+                        try {
+                            max = Integer.parseInt(sMax);
+                        } catch (NumberFormatException e) {
+                            logger.warn("failed to set [{}], maximum value is neither [{}] nor a number [{}]",
+                                    e, IndexMetaData.SETTING_AUTO_EXPAND_REPLICAS, ALL_NODES_VALUE, sMax);
+                            continue;
+                        }
+                    }
 
-                    int numberOfReplicas = event.state().nodes().dataNodes().size() - 1;
+                    int numberOfReplicas = dataNodeCount - 1;
                     if (numberOfReplicas < min) {
                         numberOfReplicas = min;
                     } else if (numberOfReplicas > max) {