fix: reject malformed Arrow field IDs (#845)

Author: fallintoplace

src/iceberg/schema_internal.cc

@@ -19,8 +19,6 @@
 
 #include "iceberg/schema_internal.h"
 
-#include <cerrno>
-#include <charconv>
 #include <cstring>
 #include <optional>
 #include <string>
@@ -30,6 +28,7 @@
 #include "iceberg/schema.h"
 #include "iceberg/type.h"
 #include "iceberg/util/macros.h"
+#include "iceberg/util/string_util.h"
 
 namespace iceberg {
 
@@ -228,7 +227,7 @@ Status ToArrowSchema(const Schema& schema, ArrowSchema* out) {
 
 namespace {
 
-int32_t GetFieldId(const ArrowSchema& schema) {
+Result<int32_t> GetFieldId(const ArrowSchema& schema) {
   if (schema.metadata == nullptr) {
     return kUnknownFieldId;
   }
@@ -241,19 +240,21 @@ int32_t GetFieldId(const ArrowSchema& schema) {
     return kUnknownFieldId;
   }
 
-  int32_t field_id = kUnknownFieldId;
-  std::from_chars(field_id_value.data, field_id_value.data + field_id_value.size_bytes,
-                  field_id);
+  std::string_view field_id(field_id_value.data, field_id_value.size_bytes);
+  auto field_id_result = StringUtils::ParseNumber<int32_t>(field_id);
+  if (!field_id_result.has_value()) {
+    return InvalidSchema("Invalid Arrow field ID: '{}'", field_id);
+  }
 
-  return field_id;
+  return field_id_result.value();
 }
 
 Result<std::shared_ptr<Type>> FromArrowSchema(const ArrowSchema& schema) {
   auto to_schema_field =
       [](const ArrowSchema& schema) -> Result<std::unique_ptr<SchemaField>> {
     ICEBERG_ASSIGN_OR_RAISE(auto field_type, FromArrowSchema(schema));
 
-    auto field_id = GetFieldId(schema);
+    ICEBERG_ASSIGN_OR_RAISE(auto field_id, GetFieldId(schema));
     bool is_optional = (schema.flags & ARROW_FLAG_NULLABLE) != 0;
     if (field_type->type_id() == TypeId::kUnknown && !is_optional) {
       return InvalidSchema("Arrow null field '{}' must be nullable", schema.name);

src/iceberg/test/arrow_test.cc

@@ -387,6 +387,25 @@ TEST_P(FromArrowSchemaTest, PrimitiveType) {
   ASSERT_EQ(*field.type(), *param.iceberg_type);
 }
 
+TEST(FromArrowSchemaTest, RejectMalformedFieldIdMetadata) {
+  for (const auto& field_id : {"1x", "2147483648", ""}) {
+    auto metadata =
+        ::arrow::key_value_metadata(std::unordered_map<std::string, std::string>{
+            {std::string(kParquetFieldIdKey), field_id}});
+    auto arrow_schema = ::arrow::schema({::arrow::field(
+        "foo", ::arrow::int32(), /*nullable=*/true, std::move(metadata))});
+    ArrowSchema exported_schema;
+    ASSERT_TRUE(::arrow::ExportSchema(*arrow_schema, &exported_schema).ok());
+
+    auto result = FromArrowSchema(exported_schema, /*schema_id=*/1);
+    ArrowSchemaRelease(&exported_schema);
+
+    EXPECT_THAT(result, IsError(ErrorKind::kInvalidSchema));
+    EXPECT_THAT(result,
+                HasErrorMessage(std::format("Invalid Arrow field ID: '{}'", field_id)));
+  }
+}
+
 INSTANTIATE_TEST_SUITE_P(
     SchemaConversion, FromArrowSchemaTest,
     ::testing::Values(