parthchandra commented on code in PR #1422:
URL: https://github.com/apache/arrow-java/pull/1422#discussion_r4449533770
##########
vector/src/test/java/org/apache/arrow/vector/ipc/MessageSerializerTest.java:
##########
@@ -220,6 +223,44 @@ public void testSerializeRecordBatchV5() throws Exception {
alloc.close();
}
+ @Test
+ public void testNegativeNodeLengthDoesNotLeakBody() {
Review Comment:
Can we add tests for these cases -
- A null count of -2 is rejected.
- A null count of -1 is accepted.
- A length of 0 is accepted.
- A length of Long.MIN_VALUE is rejected.
- A length of Integer.MAX_VALUE + 1 is rejected
##########
vector/src/main/java/org/apache/arrow/vector/ipc/message/MessageSerializer.java:
##########
@@ -411,50 +411,51 @@ public static ArrowRecordBatch deserializeRecordBatch(
*/
public static ArrowRecordBatch deserializeRecordBatch(RecordBatch
recordBatchFB, ArrowBuf body)
throws IOException {
- // Now read the body
- int nodesLength = recordBatchFB.nodesLength();
- List<ArrowFieldNode> nodes = new ArrayList<>();
- for (int i = 0; i < nodesLength; ++i) {
- FieldNode node = recordBatchFB.nodes(i);
- if ((int) node.length() != node.length() || (int) node.nullCount() !=
node.nullCount()) {
- throw new IOException(
- "Cannot currently deserialize record batches with "
- + "node length larger than INT_MAX records.");
+ try {
+ // Now read the body
+ int nodesLength = recordBatchFB.nodesLength();
+ List<ArrowFieldNode> nodes = new ArrayList<>();
+ for (int i = 0; i < nodesLength; ++i) {
+ FieldNode node = recordBatchFB.nodes(i);
+ if ((int) node.length() != node.length() || (int) node.nullCount() !=
node.nullCount()) {
Review Comment:
This catches lengths that are too large but not negative lengths.
Maybe add `node.length() < 0` and `node.nullCount() < -1`
--
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]
