This is an automated email from the ASF dual-hosted git repository.
nastra pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/main by this push:
new a5e238e5b9 Core: Tolerate explicit null config fields across REST
parsers (#17250)
a5e238e5b9 is described below
commit a5e238e5b935e27c78cb95dca50f210f65cbe3b3
Author: Eunbin Son <[email protected]>
AuthorDate: Mon Aug 3 15:56:24 2026 +0900
Core: Tolerate explicit null config fields across REST parsers (#17250)
A REST server may send "config": null in a load view response. The reader
used json.has(CONFIG), so a null value fell through to getStringMap and
threw IllegalArgumentException. Use json.hasNonNull to match the sibling
LoadTableResponseParser, which already skips a null config and yields an
empty config map.
Generated-by: Claude Code
* Core: Tolerate explicit null in remaining REST parser optional fields
The same has() + non-null getter pattern that broke LoadViewResponseParser
on an explicit "config": null exists in other REST parsers.
JsonUtil.getString,
getBool, getStringMap and JsonUtil.get all reject a NullNode, while has()
only
checks that the key is present, so an explicit null throws instead of
falling
back to the absent-field behavior.
Switch to hasNonNull() for the optional fields where treating an explicit
null
as absent matches the field's documented default:
CreateViewRequestParser properties
RemoteSignRequestParser properties, body, provider
PlanTableScanRequestParser case-sensitive, use-snapshot-schema
TableScanResponseParser delete-files, file-scan-tasks
RESTFileScanTaskParser delete-file-references
OAuth2Util expires_in, scope
The filter fields (PlanTableScanRequestParser.filter and
RESTFileScanTaskParser.residual-filter) are left alone: #14657 added
testFilterFieldWithExplicitNullThrowsError to pin the throw when aligning
the
field with the OpenAPI spec, and silently reading a null filter as "no
filter"
would turn a malformed request into a full scan rather than an error.
Each parser gets a test covering the explicit-null input.
Generated-by: Claude Code
* Core: Comment why filter fields reject an explicit null in REST parsers
huaxingao asked to note in code why PlanTableScanRequestParser.filter and
RESTFileScanTaskParser.residual-filter still reject an explicit null while
the
other optional fields now tolerate it. Add a comment on each: those fields
are
non-nullable in the OpenAPI spec (#14657 pins the throw with
testFilterFieldWithExplicitNullThrowsError), so the throw is the aligned
behavior rather than silently reading a null filter as "no filter".
Generated-by: Claude Code
---
.../iceberg/rest/RESTFileScanTaskParser.java | 5 +++-
.../iceberg/rest/TableScanResponseParser.java | 4 +--
.../org/apache/iceberg/rest/auth/OAuth2Util.java | 4 +--
.../rest/requests/CreateViewRequestParser.java | 2 +-
.../rest/requests/PlanTableScanRequestParser.java | 7 +++--
.../rest/requests/RemoteSignRequestParser.java | 6 ++--
.../rest/responses/LoadViewResponseParser.java | 2 +-
.../rest/requests/TestCreateViewRequestParser.java | 24 +++++++++++++++
.../requests/TestPlanTableScanRequestParser.java | 9 ++++++
.../rest/requests/TestRemoteSignRequestParser.java | 17 +++++++++++
.../rest/responses/TestLoadViewResponseParser.java | 34 ++++++++++++++++++++++
.../rest/responses/TestOAuthTokenResponse.java | 11 +++++++
.../responses/TestPlanTableScanResponseParser.java | 31 ++++++++++++++++++++
13 files changed, 144 insertions(+), 12 deletions(-)
diff --git
a/core/src/main/java/org/apache/iceberg/rest/RESTFileScanTaskParser.java
b/core/src/main/java/org/apache/iceberg/rest/RESTFileScanTaskParser.java
index ccd2872e04..53ef00155c 100644
--- a/core/src/main/java/org/apache/iceberg/rest/RESTFileScanTaskParser.java
+++ b/core/src/main/java/org/apache/iceberg/rest/RESTFileScanTaskParser.java
@@ -84,7 +84,7 @@ class RESTFileScanTaskParser {
int specId = dataFile.specId();
DeleteFile[] deleteFiles = null;
- if (jsonNode.has(DELETE_FILE_REFERENCES)) {
+ if (jsonNode.hasNonNull(DELETE_FILE_REFERENCES)) {
List<Integer> indices = JsonUtil.getIntegerList(DELETE_FILE_REFERENCES,
jsonNode);
Preconditions.checkArgument(
indices.isEmpty() || Collections.max(indices) <
allDeleteFiles.size(),
@@ -95,6 +95,9 @@ class RESTFileScanTaskParser {
}
Expression filter = null;
+ // An explicit null residual-filter is intentionally rejected to stay
aligned with the OpenAPI
+ // spec, where residual-filter is a non-nullable expression: has() +
fromJson throws rather than
+ // dropping the residual.
if (jsonNode.has(RESIDUAL_FILTER)) {
filter = ExpressionParser.fromJson(jsonNode.get(RESIDUAL_FILTER));
}
diff --git
a/core/src/main/java/org/apache/iceberg/rest/TableScanResponseParser.java
b/core/src/main/java/org/apache/iceberg/rest/TableScanResponseParser.java
index 02d9890494..27a046e4ce 100644
--- a/core/src/main/java/org/apache/iceberg/rest/TableScanResponseParser.java
+++ b/core/src/main/java/org/apache/iceberg/rest/TableScanResponseParser.java
@@ -44,7 +44,7 @@ public class TableScanResponseParser {
public static List<DeleteFile> parseDeleteFiles(
JsonNode node, Map<Integer, PartitionSpec> specsById) {
- if (node.has(DELETE_FILES)) {
+ if (node.hasNonNull(DELETE_FILES)) {
JsonNode deleteFiles = JsonUtil.get(DELETE_FILES, node);
Preconditions.checkArgument(
deleteFiles.isArray(), "Cannot parse delete files from non-array:
%s", deleteFiles);
@@ -65,7 +65,7 @@ public class TableScanResponseParser {
List<DeleteFile> deleteFiles,
Map<Integer, PartitionSpec> specsById,
boolean caseSensitive) {
- if (node.has(FILE_SCAN_TASKS)) {
+ if (node.hasNonNull(FILE_SCAN_TASKS)) {
JsonNode scanTasks = JsonUtil.get(FILE_SCAN_TASKS, node);
Preconditions.checkArgument(
scanTasks.isArray(), "Cannot parse file scan tasks from non-array:
%s", scanTasks);
diff --git a/core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Util.java
b/core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Util.java
index a9504a7a4e..b8db459ede 100644
--- a/core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Util.java
+++ b/core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Util.java
@@ -369,11 +369,11 @@ public class OAuth2Util {
.withTokenType(JsonUtil.getString(TOKEN_TYPE, json))
.withIssuedTokenType(JsonUtil.getStringOrNull(ISSUED_TOKEN_TYPE,
json));
- if (json.has(EXPIRES_IN)) {
+ if (json.hasNonNull(EXPIRES_IN)) {
builder.setExpirationInSeconds(JsonUtil.getInt(EXPIRES_IN, json));
}
- if (json.has(SCOPE)) {
+ if (json.hasNonNull(SCOPE)) {
builder.addScopes(parseScope(JsonUtil.getString(SCOPE, json)));
}
diff --git
a/core/src/main/java/org/apache/iceberg/rest/requests/CreateViewRequestParser.java
b/core/src/main/java/org/apache/iceberg/rest/requests/CreateViewRequestParser.java
index 7a66bc4a1e..4d593af48f 100644
---
a/core/src/main/java/org/apache/iceberg/rest/requests/CreateViewRequestParser.java
+++
b/core/src/main/java/org/apache/iceberg/rest/requests/CreateViewRequestParser.java
@@ -90,7 +90,7 @@ public class CreateViewRequestParser {
.viewVersion(viewVersion)
.schema(schema);
- if (json.has(PROPERTIES)) {
+ if (json.hasNonNull(PROPERTIES)) {
builder.properties(JsonUtil.getStringMap(PROPERTIES, json));
}
diff --git
a/core/src/main/java/org/apache/iceberg/rest/requests/PlanTableScanRequestParser.java
b/core/src/main/java/org/apache/iceberg/rest/requests/PlanTableScanRequestParser.java
index cdfca722f8..586b9d1321 100644
---
a/core/src/main/java/org/apache/iceberg/rest/requests/PlanTableScanRequestParser.java
+++
b/core/src/main/java/org/apache/iceberg/rest/requests/PlanTableScanRequestParser.java
@@ -111,17 +111,20 @@ public class PlanTableScanRequestParser {
List<String> select = JsonUtil.getStringListOrNull(SELECT, json);
Expression filter = null;
+ // Unlike the optional fields above, an explicit null filter is
intentionally rejected to stay
+ // aligned with the OpenAPI spec, where filter is a non-nullable
expression: has() + fromJson
+ // throws rather than treating null as "no filter" and silently widening
the scan.
if (json.has(FILTER)) {
filter = ExpressionParser.fromJson(json.get(FILTER));
}
boolean caseSensitive = true;
- if (json.has(CASE_SENSITIVE)) {
+ if (json.hasNonNull(CASE_SENSITIVE)) {
caseSensitive = JsonUtil.getBool(CASE_SENSITIVE, json);
}
boolean useSnapshotSchema = false;
- if (json.has(USE_SNAPSHOT_SCHEMA)) {
+ if (json.hasNonNull(USE_SNAPSHOT_SCHEMA)) {
useSnapshotSchema = JsonUtil.getBool(USE_SNAPSHOT_SCHEMA, json);
}
diff --git
a/core/src/main/java/org/apache/iceberg/rest/requests/RemoteSignRequestParser.java
b/core/src/main/java/org/apache/iceberg/rest/requests/RemoteSignRequestParser.java
index a29326a76b..1b56ec0983 100644
---
a/core/src/main/java/org/apache/iceberg/rest/requests/RemoteSignRequestParser.java
+++
b/core/src/main/java/org/apache/iceberg/rest/requests/RemoteSignRequestParser.java
@@ -95,15 +95,15 @@ public class RemoteSignRequestParser {
.uri(uri)
.headers(headers);
- if (json.has(PROPERTIES)) {
+ if (json.hasNonNull(PROPERTIES)) {
builder.properties(JsonUtil.getStringMap(PROPERTIES, json));
}
- if (json.has(BODY)) {
+ if (json.hasNonNull(BODY)) {
builder.body(JsonUtil.getString(BODY, json));
}
- if (json.has(PROVIDER)) {
+ if (json.hasNonNull(PROVIDER)) {
builder.provider(JsonUtil.getString(PROVIDER, json));
}
diff --git
a/core/src/main/java/org/apache/iceberg/rest/responses/LoadViewResponseParser.java
b/core/src/main/java/org/apache/iceberg/rest/responses/LoadViewResponseParser.java
index a8aaf17e5d..7964a8455a 100644
---
a/core/src/main/java/org/apache/iceberg/rest/responses/LoadViewResponseParser.java
+++
b/core/src/main/java/org/apache/iceberg/rest/responses/LoadViewResponseParser.java
@@ -76,7 +76,7 @@ public class LoadViewResponseParser {
ImmutableLoadViewResponse.Builder builder =
ImmutableLoadViewResponse.builder().metadataLocation(metadataLocation).metadata(metadata);
- if (json.has(CONFIG)) {
+ if (json.hasNonNull(CONFIG)) {
builder.config(JsonUtil.getStringMap(CONFIG, json));
}
diff --git
a/core/src/test/java/org/apache/iceberg/rest/requests/TestCreateViewRequestParser.java
b/core/src/test/java/org/apache/iceberg/rest/requests/TestCreateViewRequestParser.java
index a228c94a08..1f30ba6654 100644
---
a/core/src/test/java/org/apache/iceberg/rest/requests/TestCreateViewRequestParser.java
+++
b/core/src/test/java/org/apache/iceberg/rest/requests/TestCreateViewRequestParser.java
@@ -127,4 +127,28 @@ public class TestCreateViewRequestParser {
assertThat(CreateViewRequestParser.toJson(CreateViewRequestParser.fromJson(json),
true))
.isEqualTo(expectedJson);
}
+
+ @Test
+ public void nullProperties() {
+ String viewVersion =
+ ViewVersionParser.toJson(
+ ImmutableViewVersion.builder()
+ .schemaId(0)
+ .versionId(1)
+ .timestampMillis(23L)
+ .defaultNamespace(Namespace.of("ns1"))
+ .build());
+
+ String json =
+ "{\"name\":\"view-name\","
+ + "\"location\":\"location\","
+ + "\"view-version\":"
+ + viewVersion
+ + ","
+ + "\"schema\":{\"type\":\"struct\",\"schema-id\":0,"
+ +
"\"fields\":[{\"id\":1,\"name\":\"x\",\"required\":true,\"type\":\"long\"}]},"
+ + "\"properties\":null}";
+
+ assertThat(CreateViewRequestParser.fromJson(json).properties()).isEmpty();
+ }
}
diff --git
a/core/src/test/java/org/apache/iceberg/rest/requests/TestPlanTableScanRequestParser.java
b/core/src/test/java/org/apache/iceberg/rest/requests/TestPlanTableScanRequestParser.java
index 28087298b4..7fb049b8e2 100644
---
a/core/src/test/java/org/apache/iceberg/rest/requests/TestPlanTableScanRequestParser.java
+++
b/core/src/test/java/org/apache/iceberg/rest/requests/TestPlanTableScanRequestParser.java
@@ -283,4 +283,13 @@ public class TestPlanTableScanRequestParser {
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Cannot parse expression from non-object: null");
}
+
+ @Test
+ public void nullCaseSensitiveAndUseSnapshotSchema() {
+ String json =
"{\"snapshot-id\":123,\"case-sensitive\":null,\"use-snapshot-schema\":null}";
+
+ PlanTableScanRequest request = PlanTableScanRequestParser.fromJson(json);
+ assertThat(request.caseSensitive()).isTrue();
+ assertThat(request.useSnapshotSchema()).isFalse();
+ }
}
diff --git
a/core/src/test/java/org/apache/iceberg/rest/requests/TestRemoteSignRequestParser.java
b/core/src/test/java/org/apache/iceberg/rest/requests/TestRemoteSignRequestParser.java
index 3515588e44..bb3f6ef3d9 100644
---
a/core/src/test/java/org/apache/iceberg/rest/requests/TestRemoteSignRequestParser.java
+++
b/core/src/test/java/org/apache/iceberg/rest/requests/TestRemoteSignRequestParser.java
@@ -151,6 +151,23 @@ public class TestRemoteSignRequestParser {
+ "}");
}
+ @Test
+ public void nullPropertiesBodyAndProvider() {
+ String json =
+ "{\"region\":\"us-west-2\","
+ + "\"method\":\"PUT\","
+ + "\"uri\":\"http://localhost:49208/iceberg-signer-test\",";
+ + "\"headers\":{},"
+ + "\"properties\":null,"
+ + "\"body\":null,"
+ + "\"provider\":null}";
+
+ RemoteSignRequest request = RemoteSignRequestParser.fromJson(json);
+ assertThat(request.properties()).isEmpty();
+ assertThat(request.body()).isNull();
+ assertThat(request.provider()).isNull();
+ }
+
@Test
public void roundTripSerdeWithProperties() {
RemoteSignRequest request =
diff --git
a/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadViewResponseParser.java
b/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadViewResponseParser.java
index f3de08cd29..b4ae60b174 100644
---
a/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadViewResponseParser.java
+++
b/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadViewResponseParser.java
@@ -245,4 +245,38 @@ public class TestLoadViewResponseParser {
assertThat(LoadViewResponseParser.toJson(LoadViewResponseParser.fromJson(json),
true))
.isEqualTo(expectedJson);
}
+
+ @Test
+ public void nullConfig() {
+ ViewMetadata viewMetadata =
+ ViewMetadata.builder()
+ .assignUUID("386b9f01-002b-4d8c-b77f-42c3fd3b7c9b")
+ .setLocation("location")
+ .addSchema(new Schema(Types.NestedField.required(1, "x",
Types.LongType.get())))
+ .addVersion(
+ ImmutableViewVersion.builder()
+ .schemaId(0)
+ .versionId(1)
+ .timestampMillis(23L)
+ .defaultNamespace(Namespace.of("ns1"))
+ .build())
+ .setCurrentVersionId(1)
+ .build();
+
+ LoadViewResponse response =
+ ImmutableLoadViewResponse.builder()
+ .metadata(viewMetadata)
+ .metadataLocation("custom-location")
+ .build();
+
+ String jsonWithoutConfig = LoadViewResponseParser.toJson(response, true);
+ // a REST server may send an explicit "config": null, which must parse as
an empty config
+ String jsonWithNullConfig =
+ jsonWithoutConfig.substring(0, jsonWithoutConfig.length() - 2)
+ + ",\n \"config\" : null\n}";
+
+ LoadViewResponse parsed =
LoadViewResponseParser.fromJson(jsonWithNullConfig);
+ assertThat(parsed.config()).isEmpty();
+ assertThat(LoadViewResponseParser.toJson(parsed,
true)).isEqualTo(jsonWithoutConfig);
+ }
}
diff --git
a/core/src/test/java/org/apache/iceberg/rest/responses/TestOAuthTokenResponse.java
b/core/src/test/java/org/apache/iceberg/rest/responses/TestOAuthTokenResponse.java
index 6c11c0a4c9..6a9bb649ca 100644
---
a/core/src/test/java/org/apache/iceberg/rest/responses/TestOAuthTokenResponse.java
+++
b/core/src/test/java/org/apache/iceberg/rest/responses/TestOAuthTokenResponse.java
@@ -131,6 +131,17 @@ public class TestOAuthTokenResponse extends
RequestResponseTestBase<OAuthTokenRe
.hasMessageContaining("Cannot parse to a string value: token_type:
34");
}
+ @Test
+ public void nullExpiresInAndScope() throws Exception {
+ OAuthTokenResponse response =
+ deserialize(
+ "{\"access_token\":\"bearer-token\",\"token_type\":\"bearer\","
+ + "\"expires_in\":null,\"scope\":null}");
+
+ assertThat(response.expiresInSeconds()).isNull();
+ assertThat(response.scopes()).isEmpty();
+ }
+
@Test
void invalidScopeReportedInErrorMsg() {
assertThatThrownBy(() -> OAuthTokenResponse.builder().addScope("bad
scope"))
diff --git
a/core/src/test/java/org/apache/iceberg/rest/responses/TestPlanTableScanResponseParser.java
b/core/src/test/java/org/apache/iceberg/rest/responses/TestPlanTableScanResponseParser.java
index 6354e7bf24..919efb5bbd 100644
---
a/core/src/test/java/org/apache/iceberg/rest/responses/TestPlanTableScanResponseParser.java
+++
b/core/src/test/java/org/apache/iceberg/rest/responses/TestPlanTableScanResponseParser.java
@@ -430,6 +430,37 @@ public class TestPlanTableScanResponseParser {
assertThat(PlanTableScanResponseParser.toJson(copyResponse)).isEqualTo(expectedJson);
}
+ @Test
+ public void nullDeleteFilesAndFileScanTasks() {
+ PlanTableScanResponse response =
+ PlanTableScanResponseParser.fromJson(
+
"{\"status\":\"completed\",\"delete-files\":null,\"file-scan-tasks\":null}",
+ PARTITION_SPECS_BY_ID,
+ false);
+
+ assertThat(response.planStatus()).isEqualTo(PlanStatus.COMPLETED);
+ assertThat(response.fileScanTasks()).isNull();
+ }
+
+ @Test
+ public void nullDeleteFileReferences() {
+ PlanTableScanResponse response =
+ PlanTableScanResponseParser.fromJson(
+ "{\"status\":\"completed\","
+ + "\"file-scan-tasks\":["
+ + "{\"data-file\":{\"spec-id\":0,\"content\":\"data\","
+ + "\"file-path\":\"/path/to/data-a.parquet\","
+ + "\"file-format\":\"parquet\",\"partition\":[0],"
+ +
"\"file-size-in-bytes\":10,\"record-count\":1,\"sort-order-id\":0},"
+ + "\"delete-file-references\":null}]"
+ + "}",
+ PARTITION_SPECS_BY_ID,
+ false);
+
+ assertThat(response.fileScanTasks()).hasSize(1);
+ assertThat(response.fileScanTasks().get(0).deletes()).isEmpty();
+ }
+
@Test
public void emptyOrInvalidCredentials() {
assertThat(
