Core: Prevent potential NPEs when retrieving JSON fields (#5438)
diff --git a/core/src/main/java/org/apache/iceberg/MetadataUpdateParser.java b/core/src/main/java/org/apache/iceberg/MetadataUpdateParser.java
index 9b7a76c..52b6ebf 100644
--- a/core/src/main/java/org/apache/iceberg/MetadataUpdateParser.java
+++ b/core/src/main/java/org/apache/iceberg/MetadataUpdateParser.java
@@ -382,8 +382,7 @@
}
private static MetadataUpdate readAddSchema(JsonNode node) {
- Preconditions.checkArgument(node.hasNonNull(SCHEMA), "Cannot parse missing field: schema");
- JsonNode schemaNode = node.get(SCHEMA);
+ JsonNode schemaNode = JsonUtil.get(SCHEMA, node);
Schema schema = SchemaParser.fromJson(schemaNode);
int lastColumnId = JsonUtil.getInt(LAST_COLUMN_ID, node);
return new MetadataUpdate.AddSchema(schema, lastColumnId);
@@ -395,8 +394,7 @@
}
private static MetadataUpdate readAddPartitionSpec(JsonNode node) {
- Preconditions.checkArgument(node.hasNonNull(SPEC), "Missing required field: spec");
- JsonNode specNode = node.get(SPEC);
+ JsonNode specNode = JsonUtil.get(SPEC, node);
UnboundPartitionSpec spec = PartitionSpecParser.fromJson(specNode);
return new MetadataUpdate.AddPartitionSpec(spec);
}
@@ -407,9 +405,7 @@
}
private static MetadataUpdate readAddSortOrder(JsonNode node) {
- Preconditions.checkArgument(
- node.hasNonNull(SORT_ORDER), "Cannot parse missing field: sort-order");
- JsonNode sortOrderNode = node.get(SORT_ORDER);
+ JsonNode sortOrderNode = JsonUtil.get(SORT_ORDER, node);
UnboundSortOrder sortOrder = SortOrderParser.fromJson(sortOrderNode);
return new MetadataUpdate.AddSortOrder(sortOrder);
}
@@ -420,8 +416,7 @@
}
private static MetadataUpdate readAddSnapshot(JsonNode node) {
- Preconditions.checkArgument(node.has(SNAPSHOT), "Cannot parse missing field: snapshot");
- Snapshot snapshot = SnapshotParser.fromJson(null, node.get(SNAPSHOT));
+ Snapshot snapshot = SnapshotParser.fromJson(null, JsonUtil.get(SNAPSHOT, node));
return new MetadataUpdate.AddSnapshot(snapshot);
}
diff --git a/core/src/main/java/org/apache/iceberg/PartitionSpecParser.java b/core/src/main/java/org/apache/iceberg/PartitionSpecParser.java
index 0d0fff1..0d8e75f 100644
--- a/core/src/main/java/org/apache/iceberg/PartitionSpecParser.java
+++ b/core/src/main/java/org/apache/iceberg/PartitionSpecParser.java
@@ -89,7 +89,7 @@
Preconditions.checkArgument(json.isObject(), "Cannot parse spec from non-object: %s", json);
int specId = JsonUtil.getInt(SPEC_ID, json);
UnboundPartitionSpec.Builder builder = UnboundPartitionSpec.builder().withSpecId(specId);
- buildFromJsonFields(builder, json.get(FIELDS));
+ buildFromJsonFields(builder, JsonUtil.get(FIELDS, json));
return builder.build();
}
diff --git a/core/src/main/java/org/apache/iceberg/SchemaParser.java b/core/src/main/java/org/apache/iceberg/SchemaParser.java
index 8cbc255..2be4a78 100644
--- a/core/src/main/java/org/apache/iceberg/SchemaParser.java
+++ b/core/src/main/java/org/apache/iceberg/SchemaParser.java
@@ -183,7 +183,6 @@
private static Type typeFromJson(JsonNode json) {
if (json.isTextual()) {
return Types.fromPrimitiveString(json.asText());
-
} else if (json.isObject()) {
JsonNode typeObj = json.get(TYPE);
if (typeObj != null) {
@@ -202,7 +201,7 @@
}
private static Types.StructType structFromJson(JsonNode json) {
- JsonNode fieldArray = json.get(FIELDS);
+ JsonNode fieldArray = JsonUtil.get(FIELDS, json);
Preconditions.checkArgument(
fieldArray.isArray(), "Cannot parse struct fields from non-array: %s", fieldArray);
@@ -215,7 +214,7 @@
int id = JsonUtil.getInt(ID, field);
String name = JsonUtil.getString(NAME, field);
- Type type = typeFromJson(field.get(TYPE));
+ Type type = typeFromJson(JsonUtil.get(TYPE, field));
String doc = JsonUtil.getStringOrNull(DOC, field);
boolean isRequired = JsonUtil.getBool(REQUIRED, field);
@@ -231,7 +230,7 @@
private static Types.ListType listFromJson(JsonNode json) {
int elementId = JsonUtil.getInt(ELEMENT_ID, json);
- Type elementType = typeFromJson(json.get(ELEMENT));
+ Type elementType = typeFromJson(JsonUtil.get(ELEMENT, json));
boolean isRequired = JsonUtil.getBool(ELEMENT_REQUIRED, json);
if (isRequired) {
@@ -243,10 +242,10 @@
private static Types.MapType mapFromJson(JsonNode json) {
int keyId = JsonUtil.getInt(KEY_ID, json);
- Type keyType = typeFromJson(json.get(KEY));
+ Type keyType = typeFromJson(JsonUtil.get(KEY, json));
int valueId = JsonUtil.getInt(VALUE_ID, json);
- Type valueType = typeFromJson(json.get(VALUE));
+ Type valueType = typeFromJson(JsonUtil.get(VALUE, json));
boolean isRequired = JsonUtil.getBool(VALUE_REQUIRED, json);
diff --git a/core/src/main/java/org/apache/iceberg/SortOrderParser.java b/core/src/main/java/org/apache/iceberg/SortOrderParser.java
index 6d4f6ef..79ff32d 100644
--- a/core/src/main/java/org/apache/iceberg/SortOrderParser.java
+++ b/core/src/main/java/org/apache/iceberg/SortOrderParser.java
@@ -125,7 +125,7 @@
json.isObject(), "Cannot parse sort order from non-object: %s", json);
int orderId = JsonUtil.getInt(ORDER_ID, json);
UnboundSortOrder.Builder builder = UnboundSortOrder.builder().withOrderId(orderId);
- buildFromJsonFields(builder, json.get(FIELDS));
+ buildFromJsonFields(builder, JsonUtil.get(FIELDS, json));
return builder.build();
}
diff --git a/core/src/main/java/org/apache/iceberg/TableMetadataParser.java b/core/src/main/java/org/apache/iceberg/TableMetadataParser.java
index 2abfba9..6be1df3 100644
--- a/core/src/main/java/org/apache/iceberg/TableMetadataParser.java
+++ b/core/src/main/java/org/apache/iceberg/TableMetadataParser.java
@@ -362,7 +362,7 @@
Preconditions.checkArgument(
formatVersion == 1, "%s must exist in format v%s", SCHEMAS, formatVersion);
- schema = SchemaParser.fromJson(node.get(SCHEMA));
+ schema = SchemaParser.fromJson(JsonUtil.get(SCHEMA, node));
currentSchemaId = schema.schemaId();
schemas = ImmutableList.of(schema);
}
@@ -393,7 +393,7 @@
specs =
ImmutableList.of(
PartitionSpecParser.fromJsonFields(
- schema, TableMetadata.INITIAL_SPEC_ID, node.get(PARTITION_SPEC)));
+ schema, TableMetadata.INITIAL_SPEC_ID, JsonUtil.get(PARTITION_SPEC, node)));
}
Integer lastAssignedPartitionId = JsonUtil.getIntOrNull(LAST_PARTITION_ID, node);
@@ -443,7 +443,7 @@
refs = ImmutableMap.of();
}
- JsonNode snapshotArray = node.get(SNAPSHOTS);
+ JsonNode snapshotArray = JsonUtil.get(SNAPSHOTS, node);
Preconditions.checkArgument(
snapshotArray.isArray(), "Cannot parse snapshots from non-array: %s", snapshotArray);
@@ -508,7 +508,7 @@
Iterator<String> refNames = refMap.fieldNames();
while (refNames.hasNext()) {
String refName = refNames.next();
- JsonNode refNode = refMap.get(refName);
+ JsonNode refNode = JsonUtil.get(refName, refMap);
Preconditions.checkArgument(
refNode.isObject(), "Cannot parse ref %s from non-object: %s", refName, refMap);
SnapshotRef ref = SnapshotRefParser.fromJson(refNode);
diff --git a/core/src/main/java/org/apache/iceberg/puffin/FileMetadataParser.java b/core/src/main/java/org/apache/iceberg/puffin/FileMetadataParser.java
index 96ccd15..eb7b51f 100644
--- a/core/src/main/java/org/apache/iceberg/puffin/FileMetadataParser.java
+++ b/core/src/main/java/org/apache/iceberg/puffin/FileMetadataParser.java
@@ -95,11 +95,9 @@
static FileMetadata fileMetadataFromJson(JsonNode json) {
ImmutableList.Builder<BlobMetadata> blobs = ImmutableList.builder();
- JsonNode blobsJson = json.get(BLOBS);
+ JsonNode blobsJson = JsonUtil.get(BLOBS, json);
Preconditions.checkArgument(
- blobsJson != null && blobsJson.isArray(),
- "Cannot parse blobs from non-array: %s",
- blobsJson);
+ blobsJson.isArray(), "Cannot parse blobs from non-array: %s", blobsJson);
for (JsonNode blobJson : blobsJson) {
blobs.add(blobMetadataFromJson(blobJson));
}
diff --git a/core/src/main/java/org/apache/iceberg/rest/responses/ErrorResponseParser.java b/core/src/main/java/org/apache/iceberg/rest/responses/ErrorResponseParser.java
index 5900e70..a2973e0 100644
--- a/core/src/main/java/org/apache/iceberg/rest/responses/ErrorResponseParser.java
+++ b/core/src/main/java/org/apache/iceberg/rest/responses/ErrorResponseParser.java
@@ -96,10 +96,10 @@
public static ErrorResponse fromJson(JsonNode jsonNode) {
Preconditions.checkArgument(
jsonNode != null && jsonNode.isObject(),
- "Cannot parse error respone from non-object value: %s",
+ "Cannot parse error response from non-object value: %s",
jsonNode);
Preconditions.checkArgument(jsonNode.has(ERROR), "Cannot parse missing field: error");
- JsonNode error = jsonNode.get(ERROR);
+ JsonNode error = JsonUtil.get(ERROR, jsonNode);
String message = JsonUtil.getStringOrNull(MESSAGE, error);
String type = JsonUtil.getStringOrNull(TYPE, error);
Integer code = JsonUtil.getIntOrNull(CODE, error);
diff --git a/core/src/main/java/org/apache/iceberg/util/JsonUtil.java b/core/src/main/java/org/apache/iceberg/util/JsonUtil.java
index 33a548d..bd91961 100644
--- a/core/src/main/java/org/apache/iceberg/util/JsonUtil.java
+++ b/core/src/main/java/org/apache/iceberg/util/JsonUtil.java
@@ -98,6 +98,12 @@
}
}
+ public static JsonNode get(String property, JsonNode node) {
+ Preconditions.checkArgument(
+ node.hasNonNull(property), "Cannot parse missing field: %s", property);
+ return node.get(property);
+ }
+
public static int getInt(String property, JsonNode node) {
Preconditions.checkArgument(node.has(property), "Cannot parse missing int %s", property);
JsonNode pNode = node.get(property);
diff --git a/core/src/test/java/org/apache/iceberg/puffin/TestFileMetadataParser.java b/core/src/test/java/org/apache/iceberg/puffin/TestFileMetadataParser.java
index c16b232..81192be 100644
--- a/core/src/test/java/org/apache/iceberg/puffin/TestFileMetadataParser.java
+++ b/core/src/test/java/org/apache/iceberg/puffin/TestFileMetadataParser.java
@@ -81,7 +81,7 @@
public void testMissingBlobs() {
assertThatThrownBy(() -> FileMetadataParser.fromJson("{\"properties\": {}}"))
.isInstanceOf(IllegalArgumentException.class)
- .hasMessage("Cannot parse blobs from non-array: null");
+ .hasMessage("Cannot parse missing field: blobs");
}
@Test
