You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by tf...@apache.org on 2023/09/19 18:26:29 UTC

[solr] branch branch_9x updated: SOLR-16978: Be case insensitive when parsing booleans from text (#1931)

This is an automated email from the ASF dual-hosted git repository.

tflobbe pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/branch_9x by this push:
     new 3fdeae37c62 SOLR-16978: Be case insensitive when parsing booleans from text (#1931)
3fdeae37c62 is described below

commit 3fdeae37c6235ce3858ff35857bab580d845251c
Author: Tomas Eduardo Fernandez Lobbe <tf...@apache.org>
AuthorDate: Tue Sep 19 11:03:05 2023 -0700

    SOLR-16978: Be case insensitive when parsing booleans from text (#1931)
---
 solr/CHANGES.txt                                   |   2 +
 .../cloud/api/collections/TestCollectionAPI.java   |  28 ++++
 .../src/test/org/apache/solr/core/TestSolrXml.java |   4 +-
 .../src/test/org/apache/solr/util/TestUtils.java   |  71 ---------
 .../java/org/apache/solr/common/util/StrUtils.java |  10 +-
 .../org/apache/solr/common/util/StrUtilsTest.java  | 167 +++++++++++++++++++++
 6 files changed, 205 insertions(+), 77 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index d45cea79d7e..457dcae7a03 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -156,6 +156,8 @@ Other Changes
 
 * SOLR-16623: new SolrJettyTestRule for tests needing HTTP or Jetty. (David Smiley, Joshua Ouma)
 
+* SOLR-16978: Be case insensitive when parsing booleans from text (Tomás Fernández Löbbe)
+
 ==================  9.3.0 ==================
 
 Upgrade Notes
diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java
index 760d3b73d02..29c103cb9a3 100644
--- a/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java
+++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java
@@ -45,6 +45,7 @@ import org.apache.solr.common.cloud.Replica;
 import org.apache.solr.common.cloud.Slice;
 import org.apache.solr.common.cloud.SolrZkClient;
 import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.params.CollectionAdminParams;
 import org.apache.solr.common.params.CollectionParams;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.params.ShardParams;
@@ -1317,4 +1318,31 @@ public class TestCollectionAPI extends ReplicaPropertiesBase {
       cl.shutdown();
     }
   }
+
+  @Test
+  public void testCreateCollectionBooleanValues() throws Exception {
+    try (CloudSolrClient client = createCloudClient(null)) {
+      String collectionName = "testCreateCollectionBooleanValues";
+      ModifiableSolrParams params = new ModifiableSolrParams();
+      params.set("action", CollectionParams.CollectionAction.CREATE.toString());
+      params.set("name", collectionName);
+      params.set("collection.configName", "conf1");
+      params.set("numShards", "1");
+      params.set(CollectionAdminParams.PER_REPLICA_STATE, "False");
+      QueryRequest request = new QueryRequest(params);
+      request.setPath("/admin/collections");
+
+      try {
+        client.request(request);
+        waitForCollection(ZkStateReader.from(cloudClient), collectionName, 1);
+      } finally {
+        try {
+          CollectionAdminRequest.deleteCollection(collectionName).process(client);
+        } catch (Exception e) {
+          // Delete if possible, ignore otherwise. If the test failed, let the original exception
+          // bubble up
+        }
+      }
+    }
+  }
 }
diff --git a/solr/core/src/test/org/apache/solr/core/TestSolrXml.java b/solr/core/src/test/org/apache/solr/core/TestSolrXml.java
index d8c0ba8d128..03d7fb61abf 100644
--- a/solr/core/src/test/org/apache/solr/core/TestSolrXml.java
+++ b/solr/core/src/test/org/apache/solr/core/TestSolrXml.java
@@ -305,11 +305,11 @@ public class TestSolrXml extends SolrTestCaseJ4 {
 
   public void testFailAtConfigParseTimeWhenBoolTypeIsExpectedAndValueIsInvalidString() {
     String solrXml =
-        "<solr><solrcloud><bool name=\"genericCoreNodeNames\">NOT_A_BOOLEAN</bool></solrcloud></solr>";
+        "<solr><solrcloud><bool name=\"genericCoreNodeNames\">FOO</bool></solrcloud></solr>";
 
     SolrException thrown =
         assertThrows(SolrException.class, () -> SolrXmlConfig.fromString(solrHome, solrXml));
-    assertEquals("invalid boolean value: NOT_A_BOOLEAN", thrown.getMessage());
+    assertEquals("invalid boolean value: FOO", thrown.getMessage());
   }
 
   public void testFailAtConfigParseTimeWhenIntTypeIsExpectedAndBoolTypeIsGiven() {
diff --git a/solr/core/src/test/org/apache/solr/util/TestUtils.java b/solr/core/src/test/org/apache/solr/util/TestUtils.java
index 90b118b7f1c..949c2dc15fe 100644
--- a/solr/core/src/test/org/apache/solr/util/TestUtils.java
+++ b/solr/core/src/test/org/apache/solr/util/TestUtils.java
@@ -39,83 +39,12 @@ import org.apache.solr.common.util.ContentStreamBase;
 import org.apache.solr.common.util.JavaBinCodec;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.common.util.SimpleOrderedMap;
-import org.apache.solr.common.util.StrUtils;
 import org.apache.solr.common.util.Utils;
 import org.junit.Assert;
 
 /** */
 public class TestUtils extends SolrTestCaseJ4 {
 
-  public void testJoin() {
-    assertEquals("a|b|c", StrUtils.join(asList("a", "b", "c"), '|'));
-    assertEquals("a,b,c", StrUtils.join(asList("a", "b", "c"), ','));
-    assertEquals("a\\,b,c", StrUtils.join(asList("a,b", "c"), ','));
-    assertEquals("a,b|c", StrUtils.join(asList("a,b", "c"), '|'));
-
-    assertEquals("a\\\\b|c", StrUtils.join(asList("a\\b", "c"), '|'));
-  }
-
-  public void testEscapeTextWithSeparator() {
-    assertEquals("a", StrUtils.escapeTextWithSeparator("a", '|'));
-    assertEquals("a", StrUtils.escapeTextWithSeparator("a", ','));
-
-    assertEquals("a\\|b", StrUtils.escapeTextWithSeparator("a|b", '|'));
-    assertEquals("a|b", StrUtils.escapeTextWithSeparator("a|b", ','));
-    assertEquals("a,b", StrUtils.escapeTextWithSeparator("a,b", '|'));
-    assertEquals("a\\,b", StrUtils.escapeTextWithSeparator("a,b", ','));
-    assertEquals("a\\\\b", StrUtils.escapeTextWithSeparator("a\\b", ','));
-
-    assertEquals("a\\\\\\,b", StrUtils.escapeTextWithSeparator("a\\,b", ','));
-  }
-
-  public void testSplitEscaping() {
-    List<String> arr = StrUtils.splitSmart("\\r\\n:\\t\\f\\b", ":", true);
-    assertEquals(2, arr.size());
-    assertEquals("\r\n", arr.get(0));
-    assertEquals("\t\f\b", arr.get(1));
-
-    arr = StrUtils.splitSmart("\\r\\n:\\t\\f\\b", ":", false);
-    assertEquals(2, arr.size());
-    assertEquals("\\r\\n", arr.get(0));
-    assertEquals("\\t\\f\\b", arr.get(1));
-
-    arr = StrUtils.splitWS("\\r\\n \\t\\f\\b", true);
-    assertEquals(2, arr.size());
-    assertEquals("\r\n", arr.get(0));
-    assertEquals("\t\f\b", arr.get(1));
-
-    arr = StrUtils.splitWS("\\r\\n \\t\\f\\b", false);
-    assertEquals(2, arr.size());
-    assertEquals("\\r\\n", arr.get(0));
-    assertEquals("\\t\\f\\b", arr.get(1));
-
-    arr = StrUtils.splitSmart("\\:foo\\::\\:bar\\:", ":", true);
-    assertEquals(2, arr.size());
-    assertEquals(":foo:", arr.get(0));
-    assertEquals(":bar:", arr.get(1));
-
-    arr = StrUtils.splitWS("\\ foo\\  \\ bar\\ ", true);
-    assertEquals(2, arr.size());
-    assertEquals(" foo ", arr.get(0));
-    assertEquals(" bar ", arr.get(1));
-
-    arr = StrUtils.splitFileNames("/h/s,/h/\\,s,");
-    assertEquals(2, arr.size());
-    assertEquals("/h/s", arr.get(0));
-    assertEquals("/h/,s", arr.get(1));
-
-    arr = StrUtils.splitFileNames("/h/s");
-    assertEquals(1, arr.size());
-    assertEquals("/h/s", arr.get(0));
-  }
-
-  public void testToLower() {
-    assertEquals(List.of(), StrUtils.toLower(List.of()));
-    assertEquals(List.of(""), StrUtils.toLower(List.of("")));
-    assertEquals(List.of("foo"), StrUtils.toLower(List.of("foo")));
-    assertEquals(List.of("bar", "baz-123"), StrUtils.toLower(List.of("BAR", "Baz-123")));
-  }
-
   public void testNamedLists() {
     SimpleOrderedMap<Integer> map = new SimpleOrderedMap<>();
     map.add("test", 10);
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java b/solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java
index 9e813258cd3..6c209770e42 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java
@@ -267,10 +267,11 @@ public class StrUtils {
    */
   public static boolean parseBool(String s) {
     if (s != null) {
-      if (s.startsWith("true") || s.startsWith("on") || s.startsWith("yes")) {
+      String lowerS = s.toLowerCase(Locale.ROOT);
+      if (lowerS.startsWith("true") || lowerS.startsWith("on") || lowerS.startsWith("yes")) {
         return true;
       }
-      if (s.startsWith("false") || s.startsWith("off") || s.equals("no")) {
+      if (lowerS.startsWith("false") || lowerS.startsWith("off") || lowerS.equals("no")) {
         return false;
       }
     }
@@ -285,10 +286,11 @@ public class StrUtils {
    */
   public static boolean parseBool(String s, boolean def) {
     if (s != null) {
-      if (s.startsWith("true") || s.startsWith("on") || s.startsWith("yes")) {
+      String lowerS = s.toLowerCase(Locale.ROOT);
+      if (lowerS.startsWith("true") || lowerS.startsWith("on") || lowerS.startsWith("yes")) {
         return true;
       }
-      if (s.startsWith("false") || s.startsWith("off") || s.equals("no")) {
+      if (lowerS.startsWith("false") || lowerS.startsWith("off") || lowerS.equals("no")) {
         return false;
       }
     }
diff --git a/solr/solrj/src/test/org/apache/solr/common/util/StrUtilsTest.java b/solr/solrj/src/test/org/apache/solr/common/util/StrUtilsTest.java
new file mode 100644
index 00000000000..7b4b30b2895
--- /dev/null
+++ b/solr/solrj/src/test/org/apache/solr/common/util/StrUtilsTest.java
@@ -0,0 +1,167 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.common.util;
+
+import static java.util.Arrays.asList;
+
+import java.util.List;
+import org.apache.solr.SolrTestCase;
+import org.apache.solr.common.SolrException;
+
+public class StrUtilsTest extends SolrTestCase {
+
+  public void testParseBool() {
+    assertTrue(StrUtils.parseBool("true"));
+    assertTrue(StrUtils.parseBool("True"));
+    assertTrue(StrUtils.parseBool("TRUE"));
+    assertTrue(StrUtils.parseBool("true  "));
+    assertTrue(StrUtils.parseBool("trueAbc"));
+
+    assertTrue(StrUtils.parseBool("on"));
+    assertTrue(StrUtils.parseBool("ON"));
+    assertTrue(StrUtils.parseBool("on   "));
+    assertTrue(StrUtils.parseBool("onABC"));
+
+    assertTrue(StrUtils.parseBool("yes"));
+    assertTrue(StrUtils.parseBool("YES"));
+    assertTrue(StrUtils.parseBool("yes   "));
+    assertTrue(StrUtils.parseBool("yesABC"));
+
+    assertFalse(StrUtils.parseBool("false"));
+    assertFalse(StrUtils.parseBool("False"));
+    assertFalse(StrUtils.parseBool("FALSE"));
+    assertFalse(StrUtils.parseBool("false  "));
+    assertFalse(StrUtils.parseBool("falseAbc"));
+
+    assertFalse(StrUtils.parseBool("off"));
+    assertFalse(StrUtils.parseBool("OFF"));
+    assertFalse(StrUtils.parseBool("off   "));
+    assertFalse(StrUtils.parseBool("offAbc"));
+
+    assertFalse(StrUtils.parseBool("no"));
+    assertFalse(StrUtils.parseBool("NO"));
+
+    assertThrows(SolrException.class, () -> StrUtils.parseBool("foo"));
+    assertThrows(SolrException.class, () -> StrUtils.parseBool(""));
+    assertThrows(SolrException.class, () -> StrUtils.parseBool(null));
+  }
+
+  public void testParseBoolWithDefault() {
+    assertTrue(StrUtils.parseBool("true", false));
+    assertTrue(StrUtils.parseBool("True", false));
+    assertTrue(StrUtils.parseBool("TRUE", false));
+    assertTrue(StrUtils.parseBool("true  ", false));
+    assertTrue(StrUtils.parseBool("trueAbc", false));
+
+    assertTrue(StrUtils.parseBool("on", false));
+    assertTrue(StrUtils.parseBool("ON", false));
+    assertTrue(StrUtils.parseBool("on   ", false));
+    assertTrue(StrUtils.parseBool("onABC", false));
+
+    assertTrue(StrUtils.parseBool("yes", false));
+    assertTrue(StrUtils.parseBool("YES", false));
+    assertTrue(StrUtils.parseBool("yes   ", false));
+    assertTrue(StrUtils.parseBool("yesABC", false));
+
+    assertFalse(StrUtils.parseBool("false", true));
+    assertFalse(StrUtils.parseBool("False", true));
+    assertFalse(StrUtils.parseBool("FALSE", true));
+    assertFalse(StrUtils.parseBool("false  ", true));
+    assertFalse(StrUtils.parseBool("falseAbc", true));
+
+    assertFalse(StrUtils.parseBool("off", true));
+    assertFalse(StrUtils.parseBool("OFF", true));
+    assertFalse(StrUtils.parseBool("off   ", true));
+    assertFalse(StrUtils.parseBool("offAbc", true));
+
+    assertFalse(StrUtils.parseBool("no", true));
+    assertFalse(StrUtils.parseBool("NO", true));
+    assertFalse(StrUtils.parseBool("foo", false));
+    assertFalse(StrUtils.parseBool("", false));
+    assertFalse(StrUtils.parseBool(null, false));
+  }
+
+  public void testJoin() {
+    assertEquals("a|b|c", StrUtils.join(asList("a", "b", "c"), '|'));
+    assertEquals("a,b,c", StrUtils.join(asList("a", "b", "c"), ','));
+    assertEquals("a\\,b,c", StrUtils.join(asList("a,b", "c"), ','));
+    assertEquals("a,b|c", StrUtils.join(asList("a,b", "c"), '|'));
+
+    assertEquals("a\\\\b|c", StrUtils.join(asList("a\\b", "c"), '|'));
+  }
+
+  public void testEscapeTextWithSeparator() {
+    assertEquals("a", StrUtils.escapeTextWithSeparator("a", '|'));
+    assertEquals("a", StrUtils.escapeTextWithSeparator("a", ','));
+
+    assertEquals("a\\|b", StrUtils.escapeTextWithSeparator("a|b", '|'));
+    assertEquals("a|b", StrUtils.escapeTextWithSeparator("a|b", ','));
+    assertEquals("a,b", StrUtils.escapeTextWithSeparator("a,b", '|'));
+    assertEquals("a\\,b", StrUtils.escapeTextWithSeparator("a,b", ','));
+    assertEquals("a\\\\b", StrUtils.escapeTextWithSeparator("a\\b", ','));
+
+    assertEquals("a\\\\\\,b", StrUtils.escapeTextWithSeparator("a\\,b", ','));
+  }
+
+  public void testSplitEscaping() {
+    List<String> arr = StrUtils.splitSmart("\\r\\n:\\t\\f\\b", ":", true);
+    assertEquals(2, arr.size());
+    assertEquals("\r\n", arr.get(0));
+    assertEquals("\t\f\b", arr.get(1));
+
+    arr = StrUtils.splitSmart("\\r\\n:\\t\\f\\b", ":", false);
+    assertEquals(2, arr.size());
+    assertEquals("\\r\\n", arr.get(0));
+    assertEquals("\\t\\f\\b", arr.get(1));
+
+    arr = StrUtils.splitWS("\\r\\n \\t\\f\\b", true);
+    assertEquals(2, arr.size());
+    assertEquals("\r\n", arr.get(0));
+    assertEquals("\t\f\b", arr.get(1));
+
+    arr = StrUtils.splitWS("\\r\\n \\t\\f\\b", false);
+    assertEquals(2, arr.size());
+    assertEquals("\\r\\n", arr.get(0));
+    assertEquals("\\t\\f\\b", arr.get(1));
+
+    arr = StrUtils.splitSmart("\\:foo\\::\\:bar\\:", ":", true);
+    assertEquals(2, arr.size());
+    assertEquals(":foo:", arr.get(0));
+    assertEquals(":bar:", arr.get(1));
+
+    arr = StrUtils.splitWS("\\ foo\\  \\ bar\\ ", true);
+    assertEquals(2, arr.size());
+    assertEquals(" foo ", arr.get(0));
+    assertEquals(" bar ", arr.get(1));
+
+    arr = StrUtils.splitFileNames("/h/s,/h/\\,s,");
+    assertEquals(2, arr.size());
+    assertEquals("/h/s", arr.get(0));
+    assertEquals("/h/,s", arr.get(1));
+
+    arr = StrUtils.splitFileNames("/h/s");
+    assertEquals(1, arr.size());
+    assertEquals("/h/s", arr.get(0));
+  }
+
+  public void testToLower() {
+    assertEquals(List.of(), StrUtils.toLower(List.of()));
+    assertEquals(List.of(""), StrUtils.toLower(List.of("")));
+    assertEquals(List.of("foo"), StrUtils.toLower(List.of("foo")));
+    assertEquals(List.of("bar", "baz-123"), StrUtils.toLower(List.of("BAR", "Baz-123")));
+  }
+}