Skip to content

Core: Validate non-string elements in JsonUtil.getStringArray #16586

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
stevenzwu merged 2 commits into from
May 30, 2026
Merged

Core: Validate non-string elements in JsonUtil.getStringArray #16586

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
99d051f
Core: Validate non-string elements in JsonUtil.getStringArray
stevenzwu May 27, 2026
a3a82fc
Core: Add property-aware getStringArray and migrate two callers
stevenzwu May 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/src/main/java/org/apache/iceberg/rest/requests/RemoteSignRequestParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public static Map<String, List<String>> headersFromJson(String property, JsonNod
.forEach(
entry -> {
String key = entry.getKey();
List<String> values = Arrays.asList(JsonUtil.getStringArray(entry.getValue()));
List<String> values = Arrays.asList(JsonUtil.getStringArray(key, headersNode));
headers.put(key, values);
});
return headers;
Expand Down
9 changes: 8 additions & 1 deletion core/src/main/java/org/apache/iceberg/util/JsonUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,11 +272,18 @@ public static String[] getStringArray(JsonNode node) {
ArrayNode arrayNode = (ArrayNode) node;
String[] arr = new String[arrayNode.size()];
for (int i = 0; i < arr.length; i++) {
arr[i] = arrayNode.get(i).asText();
JsonNode element = arrayNode.get(i);
Preconditions.checkArgument(
element.isTextual(), "Cannot parse string from non-text value: %s", element);
arr[i] = element.asText();
}
return arr;
}

public static String[] getStringArray(String property, JsonNode node) {
return getStringList(property, node).toArray(new String[0]);
Copy link
Copy Markdown
Contributor Author

@stevenzwu stevenzwu May 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toArray is the same pattern as the existing code

  public static int[] getIntArrayOrNull(String property, JsonNode node) {
    if (!node.has(property) || node.get(property).isNull()) {
      return null;
    }

    return ArrayUtil.toIntArray(getIntegerList(property, node));
  }

}

public static List<String> getStringList(String property, JsonNode node) {
Preconditions.checkArgument(node.has(property), "Cannot parse missing list: %s", property);
return ImmutableList.<String>builder()
Expand Down
3 changes: 1 addition & 2 deletions core/src/main/java/org/apache/iceberg/view/ViewVersionParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,7 @@ public static ViewVersion fromJson(JsonNode node) {

String defaultCatalog = JsonUtil.getStringOrNull(DEFAULT_CATALOG, node);

Namespace defaultNamespace =
Namespace.of(JsonUtil.getStringArray(JsonUtil.get(DEFAULT_NAMESPACE, node)));
Namespace defaultNamespace = Namespace.of(JsonUtil.getStringArray(DEFAULT_NAMESPACE, node));

return ImmutableViewVersion.builder()
.versionId(versionId)
Expand Down
47 changes: 47 additions & 0 deletions core/src/test/java/org/apache/iceberg/util/TestJsonUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,53 @@ public void getLongSetOrNull() throws JsonProcessingException {
.containsExactlyElementsOf(Arrays.asList(23L, 45L));
}

@Test
public void getStringArray() throws JsonProcessingException {
assertThatThrownBy(() -> JsonUtil.getStringArray(JsonUtil.mapper().readTree("null")))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Cannot parse string array from non-array: null");

assertThatThrownBy(() -> JsonUtil.getStringArray(JsonUtil.mapper().readTree("23")))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Cannot parse string array from non-array: 23");

assertThatThrownBy(() -> JsonUtil.getStringArray(JsonUtil.mapper().readTree("[\"23\", 45]")))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Cannot parse string from non-text value: 45");

assertThat(JsonUtil.getStringArray(JsonUtil.mapper().readTree("[\"23\", \"45\"]")))
.containsExactly("23", "45");

assertThat(JsonUtil.getStringArray(JsonUtil.mapper().readTree("[]"))).isEmpty();
}

@Test
public void getStringArrayWithProperty() throws JsonProcessingException {
assertThatThrownBy(() -> JsonUtil.getStringArray("items", JsonUtil.mapper().readTree("{}")))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Cannot parse missing list: items");

assertThatThrownBy(
() -> JsonUtil.getStringArray("items", JsonUtil.mapper().readTree("{\"items\": null}")))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Cannot parse JSON array from non-array value: items: null");

assertThatThrownBy(
() ->
JsonUtil.getStringArray(
"items", JsonUtil.mapper().readTree("{\"items\": [\"23\", 45]}")))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Cannot parse string from non-text value in items: 45");

assertThat(
JsonUtil.getStringArray(
"items", JsonUtil.mapper().readTree("{\"items\": [\"23\", \"45\"]}")))
.containsExactly("23", "45");

assertThat(JsonUtil.getStringArray("items", JsonUtil.mapper().readTree("{\"items\": []}")))
.isEmpty();
}

@Test
public void getStringList() throws JsonProcessingException {
assertThatThrownBy(() -> JsonUtil.getStringList("items", JsonUtil.mapper().readTree("{}")))
Expand Down
2 changes: 1 addition & 1 deletion core/src/test/java/org/apache/iceberg/view/TestViewVersionParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public void missingDefaultCatalog() {
"{\"version-id\":1,\"timestamp-ms\":12345,\"schema-id\":1,"
+ "\"summary\":{\"operation\":\"create\"},\"representations\":[]}"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Cannot parse missing field: default-namespace");
.hasMessage("Cannot parse missing list: default-namespace");
}

@Test
Expand Down