Copilot commented on code in PR #960:
URL: https://github.com/apache/iceberg-cpp/pull/960#discussion_r4249517783
##########
src/iceberg/catalog/rest/json_serde.cc:
##########
@@ -474,6 +479,59 @@ Result<RenameTableRequest>
RenameTableRequestFromJson(const nlohmann::json& json
return request;
}
+nlohmann::json ToJson(const FieldLabel& field_label) {
+ nlohmann::json json;
+ json[kFieldId] = field_label.field_id;
+ json[kLabels] = field_label.labels;
+ return json;
+}
+
+Result<FieldLabel> FieldLabelFromJson(const nlohmann::json& json) {
+ if (!json.is_object()) {
+ return JsonParseError("Cannot parse field label from non-object: {}",
+ SafeDumpJson(json));
+ }
+ FieldLabel field_label;
+ ICEBERG_ASSIGN_OR_RAISE(field_label.field_id, GetJsonInteger<int32_t>(json,
kFieldId));
+ ICEBERG_ASSIGN_OR_RAISE(field_label.labels,
+ GetJsonValue<decltype(field_label.labels)>(json,
kLabels));
+ return field_label;
+}
+
+nlohmann::json ToJson(const Labels& labels) {
+ nlohmann::json json = nlohmann::json::object();
+ if (!labels.object_labels.empty()) {
+ json[kObjectLabels] = labels.object_labels;
+ }
+ if (!labels.fields.empty()) {
+ nlohmann::json fields = nlohmann::json::array();
+ for (const auto& field_label : labels.fields) {
+ fields.push_back(ToJson(field_label));
+ }
+ json[kFields] = std::move(fields);
+ }
+ return json;
+}
+
+// A non-object value is treated as absent labels rather than failing the load.
+Result<Labels> LabelsFromJson(const nlohmann::json& json) {
+ Labels labels;
+ ICEBERG_ASSIGN_OR_RAISE(
+ labels.object_labels,
+ GetJsonValueOrDefault<decltype(labels.object_labels)>(json,
kObjectLabels));
+ if (auto it = json.find(kFields); it != json.end() && !it->is_null()) {
+ if (!it->is_array()) {
+ return JsonParseError("Cannot parse '{}' from non-array: {}", kFields,
+ SafeDumpJson(*it));
+ }
Review Comment:
`LabelsFromJson` calls `json.find(...)` without first verifying `json` is an
object. For non-object `labels` payloads (e.g., string/array),
`nlohmann::json::find` can throw a `type_error`, which would bypass the
`Result<>` error path and potentially crash. Add an early guard like `if
(!json.is_object()) return Labels{};` (and keep the stricter validation for
malformed *object* shapes). This also makes the behavior match the comment and
the unit tests that expect non-object labels to be ignored.
--
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]
