LuciferYang opened a new issue, #979:
URL: https://github.com/apache/iceberg-cpp/issues/979
## Summary
The expression JSON deserialization functions in
`src/iceberg/expression/json_serde.cc` call `json[kType].get<std::string>()`
(and one `json[kTerm].get<std::string>()`) guarded only by `is_object()` and
`contains(kType)`. When `"type"` or `"term"` is a number, bool, null, or array,
nlohmann throws `json::type_error.302`. These functions return `Result<...>`
and the expression parse chain has no `try`/`catch`, so the exception escapes
the `Result` contract and terminates any caller that lacks an exception barrier.
## Root Cause
The unguarded `get<std::string>()` calls sit behind only `is_object() &&
contains(kType)`: `IsTransformTerm`, `NamedReferenceFromJson` (both the `type`
and the `term` node), both `LiteralFromJson` overloads' wrapper check, and
`ExpressionFromJson`. The sibling `OperationTypeFromJson` already checks
`is_string()` first, and `util/json_util_internal.h` exists precisely to
convert nlohmann throws into `JsonParseError`. The unguarded sites are the
outliers, not the policy.
## Impact
These parsers run on real deserialization paths, all through
`ICEBERG_ASSIGN_OR_RAISE`, which forwards a `Result` error but not a thrown
exception. Table-metadata parsing is the live path today: `FieldFromJson`
(`src/iceberg/json_serde.cc`) parses a field's `initial-default` /
`write-default` through the type-aware `LiteralFromJson`, so a metadata file
whose default is wrapped as `{"type": <non-string>, "value": ...}` throws
instead of returning a parse error — the same path #877/#878 hardened. REST
catalog responses reach the expression sites too: the scan-metrics report
filter (`metrics/json_serde.cc`) and the residual, partition, and plan filters
(`catalog/rest/json_serde.cc`) all parse server-supplied JSON. A non-string
discriminator on any of these throws `type_error.302` out of a
`Result`-returning function and past the `ICEBERG_ASSIGN_OR_RAISE` call site,
which is the same uncaught-exception-escaping-`Result` class as the merged #857.
Per `SECURITY-THREAT-MODEL.md`, catalog-supplied metadata is trusted input,
so this is a robustness and contract issue, not a security one.
## Proposed Fix
Add an `is_string()` check to each guard so a non-string `type`/`term`
returns `JsonParseError`, mirroring `OperationTypeFromJson`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
