Make boolean conversion strict

buildSrc/src/main/resources/checkstyle_suppressions.xml

@@ -261,7 +261,6 @@
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]cluster[/\\]routing[/\\]allocation[/\\]decider[/\\]AllocationDeciders.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]cluster[/\\]service[/\\]InternalClusterService.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]Base64.java" checks="LineLength" />
-  <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]Booleans.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]Numbers.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]blobstore[/\\]fs[/\\]FsBlobStore.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]blobstore[/\\]url[/\\]URLBlobStore.java" checks="LineLength" />
@@ -670,7 +669,6 @@
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]cluster[/\\]settings[/\\]ClusterSettingsIT.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]cluster[/\\]shards[/\\]ClusterSearchShardsIT.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]cluster[/\\]structure[/\\]RoutingIteratorTests.java" checks="LineLength" />
-  <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]BooleansTests.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]blobstore[/\\]FsBlobStoreContainerTests.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]blobstore[/\\]FsBlobStoreTests.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]breaker[/\\]MemoryCircuitBreakerTests.java" checks="LineLength" />

core/src/main/java/org/elasticsearch/action/support/AutoCreateIndex.java

@@ -106,7 +106,7 @@ private AutoCreate(String value) {
             boolean autoCreateIndex;
             List<Tuple<String, Boolean>> expressions = new ArrayList<>();
             try {
-                autoCreateIndex = Booleans.parseBooleanExact(value);
+                autoCreateIndex = Booleans.parseBoolean(value);
             } catch (IllegalArgumentException ex) {
                 try {
                     String[] patterns = Strings.commaDelimitedListToStringArray(value);

core/src/main/java/org/elasticsearch/cluster/metadata/AutoExpandReplicas.java

@@ -33,7 +33,7 @@ final class AutoExpandReplicas {
     public static final Setting<AutoExpandReplicas> SETTING = new Setting<>(IndexMetaData.SETTING_AUTO_EXPAND_REPLICAS, "false", (value) -> {
         final int min;
         final int max;
-        if (Booleans.parseBoolean(value, true) == false) {
+        if (Booleans.isExplicitFalse(value)) {
             return new AutoExpandReplicas(0, 0, false);
         }
         final int dash = value.indexOf('-');

core/src/main/java/org/elasticsearch/common/Booleans.java

@@ -19,34 +19,28 @@
 
 package org.elasticsearch.common;
 
-public class Booleans {
+public final class Booleans {
+    private Booleans() {
+        throw new AssertionError("No instances intended");
+    }
 
     /**
-     * Returns <code>false</code> if text is in <tt>false</tt>, <tt>0</tt>, <tt>off</tt>, <tt>no</tt>; else, true
+     * Parses a char[] representation of a boolean value to <code>boolean</code>.
+     *
+     * @return <code>true</code> iff the sequence of chars is "true", <code>false</code> iff the sequence of chars is "false" or the
+     * provided default value iff either text is <code>null</code> or length == 0.
+     * @throws IllegalArgumentException if the string cannot be parsed to boolean.
      */
     public static boolean parseBoolean(char[] text, int offset, int length, boolean defaultValue) {
-        // TODO: the leniency here is very dangerous: a simple typo will be misinterpreted and the user won't know.
-        // We should remove it and cutover to https://github.com/rmuir/booleanparser
         if (text == null || length == 0) {
             return defaultValue;
+        } else {
+            return parseBoolean(new String(text, offset, length));
         }
-        if (length == 1) {
-            return text[offset] != '0';
-        }
-        if (length == 2) {
-            return !(text[offset] == 'n' && text[offset + 1] == 'o');
-        }
-        if (length == 3) {
-            return !(text[offset] == 'o' && text[offset + 1] == 'f' && text[offset + 2] == 'f');
-        }
-        if (length == 5) {
-            return !(text[offset] == 'f' && text[offset + 1] == 'a' && text[offset + 2] == 'l' && text[offset + 3] == 's' && text[offset + 4] == 'e');
-        }
-        return true;
     }
 
     /**
-     * returns true if the a sequence of chars is one of "true","false","on","off","yes","no","0","1"
+     * returns true iff the sequence of chars is one of "true","false".
      *
      * @param text   sequence to check
      * @param offset offset to start
@@ -56,77 +50,61 @@ public static boolean isBoolean(char[] text, int offset, int length) {
         if (text == null || length == 0) {
             return false;
         }
-        if (length == 1) {
-            return text[offset] == '0' || text[offset] == '1';
-        }
-        if (length == 2) {
-            return (text[offset] == 'n' && text[offset + 1] == 'o') || (text[offset] == 'o' && text[offset + 1] == 'n');
-        }
-        if (length == 3) {
-            return (text[offset] == 'o' && text[offset + 1] == 'f' && text[offset + 2] == 'f') ||
-                    (text[offset] == 'y' && text[offset + 1] == 'e' && text[offset + 2] == 's');
-        }
-        if (length == 4) {
-            return (text[offset] == 't' && text[offset + 1] == 'r' && text[offset + 2] == 'u' && text[offset + 3] == 'e');
-        }
-        if (length == 5) {
-            return (text[offset] == 'f' && text[offset + 1] == 'a' && text[offset + 2] == 'l' && text[offset + 3] == 's' && text[offset + 4] == 'e');
-        }
-        return false;
+        return isBoolean(new String(text, offset, length));
+    }
+
+    public static boolean isBoolean(String value) {
+        return isExplicitFalse(value) || isExplicitTrue(value);
     }
 
-    /***
+    /**
+     * Parses a string representation of a boolean value to <code>boolean</code>.
      *
-     * @return true/false
-     * throws exception if string cannot be parsed to boolean
+     * @return <code>true</code> iff the provided value is "true". <code>false</code> iff the provided value is "false".
+     * @throws IllegalArgumentException if the string cannot be parsed to boolean.
      */
-    public static Boolean parseBooleanExact(String value) {
-        boolean isFalse = isExplicitFalse(value);
-        if (isFalse) {
+    public static boolean parseBoolean(String value) {
+        if (isExplicitFalse(value)) {
             return false;
         }
-        boolean isTrue = isExplicitTrue(value);
-        if (isTrue) {
+        if (isExplicitTrue(value)) {
             return true;
         }
-
-        throw new IllegalArgumentException("Failed to parse value [" + value + "] cannot be parsed to boolean [ true/1/on/yes OR false/0/off/no ]");
+        throw new IllegalArgumentException("Failed to parse value [" + value + "] as boolean (only 'true' and 'false' are allowed)");
     }
 
-    public static Boolean parseBoolean(String value, Boolean defaultValue) {
-        if (value == null) { // only for the null case we do that here!
-            return defaultValue;
-        }
-        return parseBoolean(value, false);
-    }
     /**
-     * Returns <code>true</code> iff the value is neither of the following:
-     *   <tt>false</tt>, <tt>0</tt>, <tt>off</tt>, <tt>no</tt>
-     *   otherwise <code>false</code>
+     *
+     * @param value text to parse.
+     * @param defaultValue The default value to return if the provided value is <code>null</code>.
+     * @return see {@link #parseBoolean(String)}
      */
     public static boolean parseBoolean(String value, boolean defaultValue) {
-        if (value == null) {
+        if (value == null || value.length() == 0) {
+            return defaultValue;
+        }
+        return parseBoolean(value);
+    }
+
+    public static Boolean parseBoolean(String value, Boolean defaultValue) {
+        if (value == null || value.length() == 0) {
             return defaultValue;
         }
-        return !(value.equals("false") || value.equals("0") || value.equals("off") || value.equals("no"));
+        return parseBoolean(value);
     }
 
     /**
-     * Returns <code>true</code> iff the value is either of the following:
-     *   <tt>false</tt>, <tt>0</tt>, <tt>off</tt>, <tt>no</tt>
-     *   otherwise <code>false</code>
+     * @return <code>true</code> iff the value is <tt>false</tt>, otherwise <code>false</code>.
      */
     public static boolean isExplicitFalse(String value) {
-        return value != null && (value.equals("false") || value.equals("0") || value.equals("off") || value.equals("no"));
+        return value != null && value.equals("false");
     }
 
     /**
-     * Returns <code>true</code> iff the value is either of the following:
-     *   <tt>true</tt>, <tt>1</tt>, <tt>on</tt>, <tt>yes</tt>
-     *   otherwise <code>false</code>
+     * @return <code>true</code> iff the value is <tt>true</tt>, otherwise <code>false</code>
      */
     public static boolean isExplicitTrue(String value) {
-        return value != null && (value.equals("true") || value.equals("1") || value.equals("on") || value.equals("yes"));
+        return value != null && value.equals("true");
     }
 
 }

core/src/main/java/org/elasticsearch/common/settings/Setting.java

@@ -559,15 +559,15 @@ public static Setting<Integer> intSetting(String key, int defaultValue, Property
     }
 
     public static Setting<Boolean> boolSetting(String key, boolean defaultValue, Property... properties) {
-        return new Setting<>(key, (s) -> Boolean.toString(defaultValue), Booleans::parseBooleanExact, properties);
+        return new Setting<>(key, (s) -> Boolean.toString(defaultValue), Booleans::parseBoolean, properties);
     }
 
     public static Setting<Boolean> boolSetting(String key, Setting<Boolean> fallbackSetting, Property... properties) {
-        return new Setting<>(key, fallbackSetting, Booleans::parseBooleanExact, properties);
+        return new Setting<>(key, fallbackSetting, Booleans::parseBoolean, properties);
     }
 
     public static Setting<Boolean> boolSetting(String key, Function<Settings, String> defaultValueFn, Property... properties) {
-        return new Setting<>(key, defaultValueFn, Booleans::parseBooleanExact, properties);
+        return new Setting<>(key, defaultValueFn, Booleans::parseBoolean, properties);
     }
 
     public static Setting<ByteSizeValue> byteSizeSetting(String key, ByteSizeValue value, Property... properties) {

core/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContent.java

@@ -68,7 +68,7 @@ public static XContentBuilder contentBuilder() throws IOException {
      */
     public static boolean isStrictDuplicateDetectionEnabled() {
         // Don't allow duplicate keys in JSON content by default but let the user opt out
-        return Booleans.parseBooleanExact(System.getProperty("es.json.strict_duplicate_detection", "true"));
+        return Booleans.parseBoolean(System.getProperty("es.json.strict_duplicate_detection", "true"));
     }
 
     static {

core/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java

@@ -72,9 +72,6 @@ public boolean isBooleanValue() throws IOException {
         switch (currentToken()) {
             case VALUE_BOOLEAN:
                 return true;
-            case VALUE_NUMBER:
-                NumberType numberType = numberType();
-                return numberType == NumberType.LONG || numberType == NumberType.INT;
             case VALUE_STRING:
                 return Booleans.isBoolean(textCharacters(), textOffset(), textLength());
             default:
@@ -85,9 +82,7 @@ public boolean isBooleanValue() throws IOException {
     @Override
     public boolean booleanValue() throws IOException {
         Token token = currentToken();
-        if (token == Token.VALUE_NUMBER) {
-            return intValue() != 0;
-        } else if (token == Token.VALUE_STRING) {
+        if (token == Token.VALUE_STRING) {
             return Booleans.parseBoolean(textCharacters(), textOffset(), textLength(), false /* irrelevant */);
         }
         return doBooleanValue();

core/src/main/java/org/elasticsearch/index/shard/IndexShard.java

@@ -1005,7 +1005,7 @@ private void internalPerformTranslogRecovery(boolean skipTranslogRecovery, boole
         }
         recoveryState.setStage(RecoveryState.Stage.VERIFY_INDEX);
         // also check here, before we apply the translog
-        if (Booleans.parseBoolean(checkIndexOnStartup, false)) {
+        if (Booleans.isExplicitFalse(checkIndexOnStartup) == false) {
             try {
                 checkIndex();
             } catch (IOException ex) {

core/src/main/java/org/elasticsearch/rest/RestRequest.java

@@ -196,7 +196,13 @@ public long paramAsLong(String key, long defaultValue) {
 
     @Override
     public boolean paramAsBoolean(String key, boolean defaultValue) {
-        return Booleans.parseBoolean(param(key), defaultValue);
+        String rawParam = param(key);
+        // treat the sheer presence of a parameter as "true"
+        if (rawParam != null && rawParam.length() == 0) {
+            return true;
+        } else {
+            return Booleans.parseBoolean(rawParam, defaultValue);
+        }
     }
 
     @Override

core/src/test/java/org/elasticsearch/common/BooleansTests.java

@@ -20,7 +20,6 @@
 package org.elasticsearch.common;
 
 import org.elasticsearch.test.ESTestCase;
-import org.hamcrest.Matchers;
 
 import java.util.Locale;
 
@@ -29,58 +28,58 @@
 
 public class BooleansTests extends ESTestCase {
     public void testIsBoolean() {
-        String[] booleans = new String[]{"true", "false", "on", "off", "yes", "no", "0", "1"};
-        String[] notBooleans = new String[]{"11", "00", "sdfsdfsf", "F", "T"};
+        String[] booleans = new String[]{"true", "false"};
+        String[] notBooleans = new String[]{"11", "00", "sdfsdfsf", "F", "T", "on", "off", "yes", "no", "0", "1"};
         assertThat(Booleans.isBoolean(null, 0, 1), is(false));
 
         for (String b : booleans) {
             String t = "prefix" + b + "suffix";
-            assertThat("failed to recognize [" + b + "] as boolean", Booleans.isBoolean(t.toCharArray(), "prefix".length(), b.length()), Matchers.equalTo(true));
+            assertThat("failed to recognize [" + b + "] as boolean",
+                Booleans.isBoolean(t.toCharArray(), "prefix".length(), b.length()), is(true));
         }
 
         for (String nb : notBooleans) {
             String t = "prefix" + nb + "suffix";
-            assertThat("recognized [" + nb + "] as boolean", Booleans.isBoolean(t.toCharArray(), "prefix".length(), nb.length()), Matchers.equalTo(false));
+            assertThat("recognized [" + nb + "] as boolean",
+                Booleans.isBoolean(t.toCharArray(), "prefix".length(), nb.length()), is(false));
         }
     }
 
     public void testParseBoolean() {
-        assertThat(Booleans.parseBoolean(randomFrom("true", "on", "yes", "1"), randomBoolean()), is(true));
-        assertThat(Booleans.parseBoolean(randomFrom("false", "off", "no", "0"), randomBoolean()), is(false));
-        assertThat(Booleans.parseBoolean(randomFrom("true", "on", "yes").toUpperCase(Locale.ROOT), randomBoolean()), is(true));
         assertThat(Booleans.parseBoolean(null, false), is(false));
         assertThat(Booleans.parseBoolean(null, true), is(true));
 
-        assertThat(Booleans.parseBoolean(randomFrom("true", "on", "yes", "1"), randomFrom(Boolean.TRUE, Boolean.FALSE, null)), is(true));
-        assertThat(Booleans.parseBoolean(randomFrom("false", "off", "no", "0"), randomFrom(Boolean.TRUE, Boolean.FALSE, null)), is(false));
-        assertThat(Booleans.parseBoolean(randomFrom("true", "on", "yes").toUpperCase(Locale.ROOT),randomFrom(Boolean.TRUE, Boolean.FALSE, null)), is(true));
+        assertThat(Booleans.parseBoolean("true", randomFrom(Boolean.TRUE, Boolean.FALSE, null)), is(true));
+        assertThat(Booleans.parseBoolean("false", randomFrom(Boolean.TRUE, Boolean.FALSE, null)), is(false));
+        expectThrows(IllegalArgumentException.class,
+            () -> Booleans.parseBoolean("true".toUpperCase(Locale.ROOT),randomFrom(Boolean.TRUE, Boolean.FALSE, null)));
         assertThat(Booleans.parseBoolean(null, Boolean.FALSE), is(false));
         assertThat(Booleans.parseBoolean(null, Boolean.TRUE), is(true));
         assertThat(Booleans.parseBoolean(null, null), nullValue());
 
-        char[] chars = randomFrom("true", "on", "yes", "1").toCharArray();
+        char[] chars = "true".toCharArray();
         assertThat(Booleans.parseBoolean(chars, 0, chars.length, randomBoolean()), is(true));
-        chars = randomFrom("false", "off", "no", "0").toCharArray();
+        chars = "false".toCharArray();
         assertThat(Booleans.parseBoolean(chars,0, chars.length, randomBoolean()), is(false));
-        chars = randomFrom("true", "on", "yes").toUpperCase(Locale.ROOT).toCharArray();
-        assertThat(Booleans.parseBoolean(chars,0, chars.length, randomBoolean()), is(true));
+        final char[] TRUE = "true".toUpperCase(Locale.ROOT).toCharArray();
+        expectThrows(IllegalArgumentException.class, () -> Booleans.parseBoolean(TRUE, 0, TRUE.length, randomBoolean()));
     }
 
     public void testParseBooleanExact() {
-        assertThat(Booleans.parseBooleanExact(randomFrom("true", "on", "yes", "1")), is(true));
-        assertThat(Booleans.parseBooleanExact(randomFrom("false", "off", "no", "0")), is(false));
+        assertThat(Booleans.parseBoolean("true"), is(true));
+        assertThat(Booleans.parseBoolean("false"), is(false));
         try {
-            Booleans.parseBooleanExact(randomFrom("fred", "foo", "barney", null));
-            fail("Expected exception while parsing invalid boolean value ");
+            Booleans.parseBoolean(randomFrom("fred", "foo", "barney", null, "on", "yes", "1", "off", "no", "0", "True", "False"));
+            fail("Expected exception while parsing invalid boolean value");
         } catch (Exception ex) {
             assertTrue(ex instanceof IllegalArgumentException);
         }
     }
 
     public void testIsExplicit() {
-        assertThat(Booleans.isExplicitFalse(randomFrom("true", "on", "yes", "1", "foo", null)), is(false));
-        assertThat(Booleans.isExplicitFalse(randomFrom("false", "off", "no", "0")), is(true));
-        assertThat(Booleans.isExplicitTrue(randomFrom("true", "on", "yes", "1")), is(true));
-        assertThat(Booleans.isExplicitTrue(randomFrom("false", "off", "no", "0", "foo", null)), is(false));
+        assertThat(Booleans.isExplicitFalse(randomFrom("true", "on", "yes", "1", "foo", null, "False")), is(false));
+        assertThat(Booleans.isExplicitFalse("false"), is(true));
+        assertThat(Booleans.isExplicitTrue("true"), is(true));
+        assertThat(Booleans.isExplicitTrue(randomFrom("false", "off", "no", "0", "foo", null, "True")), is(false));
     }
 }

core/src/test/java/org/elasticsearch/common/settings/SettingTests.java

@@ -125,7 +125,7 @@ public void testSimpleUpdate() {
             settingUpdater.apply(build, Settings.EMPTY);
             fail("not a boolean");
         } catch (IllegalArgumentException ex) {
-            assertEquals("Failed to parse value [I am not a boolean] cannot be parsed to boolean [ true/1/on/yes OR false/0/off/no ]",
+            assertEquals("Failed to parse value [I am not a boolean] as boolean (only 'true' and 'false' are allowed)",
                     ex.getMessage());
         }
     }

core/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java

@@ -118,7 +118,7 @@ public void testSpecialTribeSetting() {
                 new SettingsModule(settings);
                 fail();
             } catch (IllegalArgumentException ex) {
-                assertEquals("Failed to parse value [BOOM] cannot be parsed to boolean [ true/1/on/yes OR false/0/off/no ]",
+                assertEquals("Failed to parse value [BOOM] as boolean (only 'true' and 'false' are allowed)",
                         ex.getMessage());
             }
         }