diff --git a/src/iceberg/expression/json_serde.cc b/src/iceberg/expression/json_serde.cc index 1afed021f..59f3fdcc2 100644 --- a/src/iceberg/expression/json_serde.cc +++ b/src/iceberg/expression/json_serde.cc @@ -86,7 +86,7 @@ nlohmann::json MakeTransformJson(std::string_view transform_str, /// Helper to check if a JSON term represents a transform bool IsTransformTerm(const nlohmann::json& json) { - return json.is_object() && json.contains(kType) && + return json.is_object() && json.contains(kType) && json[kType].is_string() && json[kType].get() == kTransform && json.contains(kTerm); } @@ -228,8 +228,9 @@ nlohmann::json ToJson(const BoundTransform& transform) { Result> NamedReferenceFromJson( const nlohmann::json& json) { - if (json.is_object() && json.contains(kType) && - json[kType].get() == kReference && json.contains(kTerm)) { + if (json.is_object() && json.contains(kType) && json[kType].is_string() && + json[kType].get() == kReference && json.contains(kTerm) && + json[kTerm].is_string()) { return NamedReference::Make(json[kTerm].get()); } if (!json.is_string()) [[unlikely]] { @@ -331,7 +332,7 @@ Result GetInt64Checked(const nlohmann::json& json) { Result LiteralFromJson(const nlohmann::json& json, const Type* type) { // If {"type": "literal", "value": } wrapper is present, unwrap it first. - if (json.is_object() && json.contains(kType) && + if (json.is_object() && json.contains(kType) && json[kType].is_string() && json[kType].get() == kLiteral && json.contains(kValue)) { return LiteralFromJson(json[kValue], type); } @@ -499,7 +500,7 @@ Result LiteralFromJson(const nlohmann::json& json, const Type* type) { Result LiteralFromJson(const nlohmann::json& json) { // Unwrap {"type": "literal", "value": } wrapper - if (json.is_object() && json.contains(kType) && + if (json.is_object() && json.contains(kType) && json[kType].is_string() && json[kType].get() == kLiteral && json.contains(kValue)) { return LiteralFromJson(json[kValue]); } @@ -619,7 +620,7 @@ Result> ExpressionFromJson(const nlohmann::json& jso SafeDumpJson(json)); } - if (json[kType].get() == kLiteral) { + if (json[kType].is_string() && json[kType].get() == kLiteral) { if (!json.contains(kValue) || !json[kValue].is_boolean()) [[unlikely]] { return JsonParseError( "Expression of type 'literal' must have a boolean 'value' field: {}", diff --git a/src/iceberg/test/expression_json_test.cc b/src/iceberg/test/expression_json_test.cc index 1c993cc10..1afecc716 100644 --- a/src/iceberg/test/expression_json_test.cc +++ b/src/iceberg/test/expression_json_test.cc @@ -577,4 +577,31 @@ INSTANTIATE_TEST_SUITE_P( return info.param.name; }); +// A non-string "type" node must produce a parse error, not an uncaught +// nlohmann type_error escaping the Result contract. +TEST(ExpressionJsonTest, NonStringTypeIsParseError) { + EXPECT_THAT(ExpressionFromJson(R"({"type": 42, "term": "a"})"_json), + IsError(ErrorKind::kJsonParseError)); + EXPECT_THAT(ExpressionFromJson(R"({"type": 42, "left": true, "right": true})"_json), + IsError(ErrorKind::kJsonParseError)); + EXPECT_THAT(LiteralFromJson(R"({"type": 42, "value": 1})"_json), + IsError(ErrorKind::kJsonParseError)); + + // A reference wrapper with a non-string term is likewise a parse error. + EXPECT_THAT( + ExpressionFromJson( + R"({"type":"eq","term":{"type":"reference","term":42},"value":1})"_json), + IsError(ErrorKind::kJsonParseError)); + + // A non-string "type" on a predicate's term node, routed through the + // transform-term check and then the named-reference parser. + EXPECT_THAT( + ExpressionFromJson(R"({"type":"eq","term":{"type":42,"term":"x"},"value":1})"_json), + IsError(ErrorKind::kJsonParseError)); + + // A non-string "type" on the type-aware LiteralFromJson overload. + EXPECT_THAT(LiteralFromJson(R"({"type":42,"value":1})"_json, int32().get()), + IsError(ErrorKind::kJsonParseError)); +} + } // namespace iceberg