fix: align boolean parsing with Iceberg Java

Boolean values were parsed ad hoc in four places, and not consistently:
`arrow_s3_file_io.cc` rejected anything that was not "true" or "false",
while `config.h`, `auth_managers.cc` and `rest_catalog.cc` read any
unrecognized value as false. Each site spelled out the "true"/"false"
literals itself, and `StringUtils` covered only numbers.

Add `StringUtils::ParseBoolean()`, which mirrors Java's
Boolean.parseBoolean, and `PropertyUtil::PropertyAsBoolean()` /
`PropertyUtil::PropertyAsOptionalBoolean()`, which mirror Java's
PropertyUtil.propertyAsBoolean and propertyAsNullableBoolean, and read
every boolean through them.

That settles the codebase on Java's contract: "true" ignoring case reads
as true, and every other value reads as false. `s3.path-style-access` and
`s3.ssl.enabled` change behavior accordingly: a malformed value used to
fail the FileIO build and now reads as false, as it does in Java, whose
S3FileIOProperties reads both with PropertyUtil.propertyAsBoolean. The
test that pinned the old behavior is rewritten to pin the new one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: manuzhang

src/iceberg/arrow/s3/arrow_s3_file_io.cc

@@ -37,6 +37,7 @@
 #include "iceberg/arrow/s3/s3_properties.h"
 #include "iceberg/logging/log_macros.h"
 #include "iceberg/util/macros.h"
+#include "iceberg/util/property_util.h"
 #include "iceberg/util/string_util.h"
 
 namespace iceberg::arrow {
@@ -52,22 +53,6 @@ const std::string* FindProperty(
   return it == properties.end() ? nullptr : &it->second;
 }
 
-Result<std::optional<bool>> ParseOptionalBool(
-    const std::unordered_map<std::string, std::string>& properties,
-    std::string_view key) {
-  const auto* value = FindProperty(properties, key);
-  if (value == nullptr) {
-    return std::nullopt;
-  }
-  if (StringUtils::EqualsIgnoreCase(*value, "true")) {
-    return true;
-  }
-  if (StringUtils::EqualsIgnoreCase(*value, "false")) {
-    return false;
-  }
-  return InvalidArgument(R"("{}" must be "true" or "false")", key);
-}
-
 Status EnsureS3Initialized() {
   static const ::arrow::Status init_status = []() {
     auto options = ::arrow::fs::S3GlobalOptions::Defaults();
@@ -136,15 +121,15 @@ Result<::arrow::fs::S3Options> ConfigureS3Options(
     options.endpoint_override = SplitEndpointScheme(endpoint_env, options);
   }
 
-  ICEBERG_ASSIGN_OR_RAISE(const auto path_style_access,
-                          ParseOptionalBool(properties, S3Properties::kPathStyleAccess));
+  const auto path_style_access =
+      PropertyUtil::PropertyAsOptionalBoolean(properties, S3Properties::kPathStyleAccess);
   if (path_style_access.has_value()) {
     options.force_virtual_addressing = !*path_style_access;
   }
 
   // Explicit `s3.ssl.enabled` overrides any endpoint-derived scheme.
-  ICEBERG_ASSIGN_OR_RAISE(const auto ssl_enabled,
-                          ParseOptionalBool(properties, S3Properties::kSslEnabled));
+  const auto ssl_enabled =
+      PropertyUtil::PropertyAsOptionalBoolean(properties, S3Properties::kSslEnabled);
   if (ssl_enabled.has_value()) {
     options.scheme = *ssl_enabled ? "https" : "http";
   }

src/iceberg/catalog/rest/auth/auth_managers.cc

@@ -23,6 +23,7 @@
 
 #include "iceberg/catalog/rest/auth/auth_manager_internal.h"
 #include "iceberg/catalog/rest/auth/auth_properties.h"
+#include "iceberg/util/property_util.h"
 #include "iceberg/util/string_util.h"
 
 namespace iceberg::rest::auth {
@@ -47,8 +48,8 @@ const std::unordered_set<std::string, StringHash, StringEqual>& KnownAuthTypes()
 std::string InferAuthType(
     const std::unordered_map<std::string, std::string>& properties) {
   // Deprecated alias: rest.sigv4-enabled=true forces SigV4.
-  if (auto it = properties.find(AuthProperties::kSigV4Enabled);
-      it != properties.end() && StringUtils::EqualsIgnoreCase(it->second, "true")) {
+  if (PropertyUtil::PropertyAsBoolean(properties, AuthProperties::kSigV4Enabled,
+                                      /*default_value=*/false)) {
     return AuthProperties::kAuthTypeSigV4;
   }
 

src/iceberg/catalog/rest/rest_catalog.cc

@@ -523,7 +523,7 @@ Result<std::shared_ptr<MetricsReporter>> RestCatalog::MakeTableReporter(
     const TableIdentifier& identifier,
     const std::shared_ptr<auth::AuthSession>& table_session) const {
   auto metrics_enabled = config_.Get(RestCatalogProperties::kMetricsReportingEnabled);
-  if (StringUtils::ToLower(metrics_enabled) == "true" &&
+  if (StringUtils::ParseBoolean(metrics_enabled) &&
       supported_endpoints_.contains(Endpoint::ReportMetrics())) {
     ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Metrics(identifier));
     auto post = [client = client_](const std::string& endpoint, const std::string& body,

src/iceberg/test/CMakeLists.txt

@@ -143,6 +143,7 @@ add_iceberg_test(util_test
                  roaring_position_bitmap_test.cc
                  position_delete_index_test.cc
                  position_delete_range_consumer_test.cc
+                 property_util_test.cc
                  resolving_file_io_test.cc
                  retry_util_test.cc
                  string_util_test.cc

src/iceberg/test/arrow_s3_file_io_test.cc

@@ -245,10 +245,13 @@ TEST_F(ArrowS3FileIOTest, RejectsIncompleteStaticCredentials) {
                           "S3 client access key ID and secret access key must be set"));
 }
 
-TEST_F(ArrowS3FileIOTest, RejectsInvalidBooleanProperties) {
-  auto result =
-      MakeS3FileIO({{std::string(S3Properties::kPathStyleAccess), "not-a-bool"}});
-  EXPECT_THAT(result, IsError(ErrorKind::kInvalidArgument));
+TEST_F(ArrowS3FileIOTest, InvalidBooleanPropertyReadsAsFalse) {
+  // As in Java, where these properties go through PropertyUtil.propertyAsBoolean, a
+  // value that does not spell a boolean reads as false instead of being rejected.
+  auto options =
+      ConfigureS3Options({{std::string(S3Properties::kPathStyleAccess), "not-a-bool"}});
+  ASSERT_THAT(options, IsOk());
+  EXPECT_TRUE(options->force_virtual_addressing);
 }
 
 TEST_F(ArrowS3FileIOTest, ReadWrite) {

src/iceberg/test/property_util_test.cc

@@ -0,0 +1,72 @@
+/*
+ * 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.
+ */
+
+#include "iceberg/util/property_util.h"
+
+#include <optional>
+#include <string>
+#include <string_view>
+#include <unordered_map>
+
+#include <gtest/gtest.h>
+
+namespace iceberg {
+
+namespace {
+
+constexpr std::string_view kKey = "test.enabled";
+
+std::unordered_map<std::string, std::string> Properties(std::string_view value) {
+  return {{std::string(kKey), std::string(value)}};
+}
+
+}  // namespace
+
+TEST(PropertyUtilTest, BooleanDefaultsWhenPropertyIsAbsent) {
+  const std::unordered_map<std::string, std::string> empty;
+  EXPECT_TRUE(PropertyUtil::PropertyAsBoolean(empty, kKey, true));
+  EXPECT_FALSE(PropertyUtil::PropertyAsBoolean(empty, kKey, false));
+  EXPECT_EQ(PropertyUtil::PropertyAsOptionalBoolean(empty, kKey), std::nullopt);
+}
+
+TEST(PropertyUtilTest, BooleanIgnoresCase) {
+  for (const auto* value : {"true", "TRUE", "TrUe"}) {
+    EXPECT_TRUE(PropertyUtil::PropertyAsBoolean(Properties(value), kKey, false)) << value;
+    EXPECT_EQ(PropertyUtil::PropertyAsOptionalBoolean(Properties(value), kKey), true)
+        << value;
+  }
+  for (const auto* value : {"false", "FALSE", "FaLsE"}) {
+    EXPECT_FALSE(PropertyUtil::PropertyAsBoolean(Properties(value), kKey, true)) << value;
+    EXPECT_EQ(PropertyUtil::PropertyAsOptionalBoolean(Properties(value), kKey), false)
+        << value;
+  }
+}
+
+TEST(PropertyUtilTest, NonBooleanValueReadsAsFalse) {
+  // Matches Java's PropertyUtil.propertyAsBoolean: a value that does not spell a
+  // boolean is not rejected, and the default does not apply either, since the
+  // property is set.
+  for (const auto* value : {"", " true", "yes", "1", "ture"}) {
+    EXPECT_FALSE(PropertyUtil::PropertyAsBoolean(Properties(value), kKey, true)) << value;
+    EXPECT_EQ(PropertyUtil::PropertyAsOptionalBoolean(Properties(value), kKey), false)
+        << value;
+  }
+}
+
+}  // namespace iceberg

src/iceberg/test/string_util_test.cc

@@ -32,6 +32,18 @@ TEST(StringUtilsTest, ToLower) {
   ASSERT_EQ(StringUtils::ToLower("123"), "123");
 }
 
+TEST(StringUtilsTest, ParseBoolean) {
+  for (const auto* value : {"true", "TRUE", "TrUe"}) {
+    EXPECT_TRUE(StringUtils::ParseBoolean(value)) << value;
+  }
+  // As in Java's Boolean.parseBoolean, a value that is not "true" reads as false
+  // whether it spells a boolean or not.
+  for (const auto* value :
+       {"false", "FALSE", "FaLsE", "", " true", "true ", "yes", "1", "ture"}) {
+    EXPECT_FALSE(StringUtils::ParseBoolean(value)) << value;
+  }
+}
+
 TEST(StringUtilsTest, ToUpper) {
   ASSERT_EQ(StringUtils::ToUpper("abc"), "ABC");
   ASSERT_EQ(StringUtils::ToUpper("A-bC"), "A-BC");

src/iceberg/util/config.h

@@ -54,7 +54,7 @@ U DefaultFromString(const std::string& val) {
   if constexpr (std::is_same_v<U, std::string>) {
     return val;
   } else if constexpr (std::is_same_v<U, bool>) {
-    return StringUtils::EqualsIgnoreCase(val, "true");
+    return StringUtils::ParseBoolean(val);
   } else if constexpr ((std::is_signed_v<U> && std::is_integral_v<U>) ||
                        std::is_floating_point_v<U>) {
     ICEBERG_ASSIGN_OR_THROW(auto res, StringUtils::ParseNumber<U>(val));

src/iceberg/util/property_util.cc

@@ -20,6 +20,7 @@
 #include "iceberg/util/property_util.h"
 
 #include <cstdint>
+#include <string>
 
 #include "iceberg/table_properties.h"
 #include "iceberg/util/string_util.h"
@@ -46,4 +47,20 @@ Status PropertyUtil::ValidateCommitProperties(
   return {};
 }
 
+bool PropertyUtil::PropertyAsBoolean(
+    const std::unordered_map<std::string, std::string>& properties, std::string_view key,
+    bool default_value) {
+  return PropertyAsOptionalBoolean(properties, key).value_or(default_value);
+}
+
+std::optional<bool> PropertyUtil::PropertyAsOptionalBoolean(
+    const std::unordered_map<std::string, std::string>& properties,
+    std::string_view key) {
+  auto it = properties.find(std::string(key));
+  if (it == properties.end()) {
+    return std::nullopt;
+  }
+  return StringUtils::ParseBoolean(it->second);
+}
+
 }  // namespace iceberg

src/iceberg/util/property_util.h

@@ -22,7 +22,9 @@
 /// \file iceberg/util/property_util.h
 /// \brief Provide property conversion helpers.
 
+#include <optional>
 #include <string>
+#include <string_view>
 #include <unordered_map>
 
 #include "iceberg/iceberg_export.h"
@@ -34,6 +36,33 @@ class ICEBERG_EXPORT PropertyUtil {
  public:
   static Status ValidateCommitProperties(
       const std::unordered_map<std::string, std::string>& properties);
+
+  /// \brief Read a boolean property from a property map.
+  ///
+  /// Mirrors Java's PropertyUtil.propertyAsBoolean: the value is parsed with
+  /// StringUtils::ParseBoolean, so anything that is not "true" ignoring case reads as
+  /// false rather than being rejected.
+  ///
+  /// \param properties The property map to read from.
+  /// \param key The property key.
+  /// \param default_value Returned when the property is absent.
+  /// \return The parsed value, or default_value if the property is absent.
+  static bool PropertyAsBoolean(
+      const std::unordered_map<std::string, std::string>& properties,
+      std::string_view key, bool default_value);
+
+  /// \brief Read a boolean property that may be unset.
+  ///
+  /// Like PropertyAsBoolean, but returns std::nullopt when the property is absent so
+  /// callers can distinguish an unset property from an explicit "false". Mirrors Java's
+  /// PropertyUtil.propertyAsNullableBoolean.
+  ///
+  /// \param properties The property map to read from.
+  /// \param key The property key.
+  /// \return The parsed value, or std::nullopt if the property is absent.
+  static std::optional<bool> PropertyAsOptionalBoolean(
+      const std::unordered_map<std::string, std::string>& properties,
+      std::string_view key);
 };
 
 }  // namespace iceberg