Core: Validates bool values in yaml for node settings

- Added parseBooleanExact in booleans which throws exception in case of parse failure - Used ParseExact in static's to make it consistent + code review fixes - Added unit test - Changed Exception Type to ElasticSearchIllegalArg from Parse - used isExplicit* Closes #8097
elastic · Oct 27, 2014 · cb33508 · cb33508
1 parent ea421d7
commit cb33508
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 6 deletions.
diff --git a/src/main/java/org/elasticsearch/cluster/node/DiscoveryNode.java b/src/main/java/org/elasticsearch/cluster/node/DiscoveryNode.java
@@ -23,6 +23,7 @@
 import com.google.common.collect.ImmutableMap;
 import org.elasticsearch.ElasticsearchIllegalArgumentException;
 import org.elasticsearch.Version;
+import org.elasticsearch.common.Booleans;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
@@ -72,23 +73,23 @@ public static boolean nodeRequiresLocalStorage(Settings settings) {
 
     public static boolean clientNode(Settings settings) {
         String client = settings.get("node.client");
-        return client != null && client.equals("true");
+        return Booleans.isExplicitTrue(client);
     }
 
     public static boolean masterNode(Settings settings) {
         String master = settings.get("node.master");
         if (master == null) {
             return !clientNode(settings);
         }
-        return master.equals("true");
+        return Booleans.isExplicitTrue(master);
     }
 
     public static boolean dataNode(Settings settings) {
         String data = settings.get("node.data");
         if (data == null) {
             return !clientNode(settings);
         }
-        return data.equals("true");
+        return Booleans.isExplicitTrue(data);
     }
 
     public static final ImmutableList<DiscoveryNode> EMPTY_LIST = ImmutableList.of();
@@ -244,7 +245,7 @@ public boolean dataNode() {
         if (data == null) {
             return !clientNode();
         }
-        return data.equals("true");
+        return Booleans.parseBooleanExact(data);
     }
 
     /**
@@ -259,7 +260,7 @@ public boolean isDataNode() {
      */
     public boolean clientNode() {
         String client = attributes.get("client");
-        return client != null && client.equals("true");
+        return client != null && Booleans.parseBooleanExact(client);
     }
 
     public boolean isClientNode() {
@@ -274,7 +275,7 @@ public boolean masterNode() {
         if (master == null) {
             return !clientNode();
         }
-        return master.equals("true");
+        return Booleans.parseBooleanExact(master);
     }
 
     /**

diff --git a/src/main/java/org/elasticsearch/common/Booleans.java b/src/main/java/org/elasticsearch/common/Booleans.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.common;
 
+import org.elasticsearch.ElasticsearchIllegalArgumentException;
 /**
  *
  */
@@ -78,6 +79,26 @@ public static boolean isBoolean(char[] text, int offset, int length) {
         return false;
     }
 
+    /***
+     *
+     * @param value
+     * @return true/false
+     * throws exception if string cannot be parsed to boolean
+     */
+    public static Boolean parseBooleanExact(String value){
+
+        boolean isFalse = isExplicitFalse(value);
+        if (isFalse) {
+            return false;
+        }
+        boolean isTrue = isExplicitTrue(value);
+        if (isTrue) {
+            return true;
+        }
+
+        throw new ElasticsearchIllegalArgumentException("value cannot be parsed to boolean [ true/1/on/yes OR false/0/off/no ]  ");
+    }
+
     public static Boolean parseBoolean(String value, Boolean defaultValue) {
         if (value == null) { // only for the null case we do that here!
             return defaultValue;

diff --git a/src/test/java/org/elasticsearch/common/BooleansTests.java b/src/test/java/org/elasticsearch/common/BooleansTests.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.common;
 
+import org.elasticsearch.ElasticsearchIllegalArgumentException;
 import org.elasticsearch.test.ElasticsearchTestCase;
 import org.hamcrest.Matchers;
 import org.junit.Test;
@@ -69,6 +70,18 @@ public void parseBoolean() {
         assertThat(Booleans.parseBoolean(chars,0, chars.length, randomBoolean()), is(true));
     }
 
+    @Test
+    public void parseBooleanExact() {
+        assertThat(Booleans.parseBooleanExact(randomFrom("true", "on", "yes", "1")), is(true));
+        assertThat(Booleans.parseBooleanExact(randomFrom("false", "off", "no", "0")), is(false));
+        try {
+            Booleans.parseBooleanExact(randomFrom(null, "fred", "foo", "barney"));
+            fail("Expected exception while parsing invalid boolean value ");
+        } catch (Exception ex) {
+            assertTrue(ex instanceof ElasticsearchIllegalArgumentException);
+        }
+    }
+
     public void testIsExplict() {
         assertThat(Booleans.isExplicitFalse(randomFrom("true", "on", "yes", "1", "foo", null)), is(false));
         assertThat(Booleans.isExplicitFalse(randomFrom("false", "off", "no", "0")), is(true));