This is an automated email from the ASF dual-hosted git repository.
Fokko pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/parquet-java.git
The following commit(s) were added to refs/heads/master by this push:
new 3e05546cb Fix `ByteBuffer` handling in `VariantUtil` and
`VariantBuilder` (#3472)
3e05546cb is described below
commit 3e05546cb5a4d443b87c9c01abe8f04a71e29318
Author: Robert Yokota <[email protected]>
AuthorDate: Fri May 15 11:45:32 2026 -0700
Fix `ByteBuffer` handling in `VariantUtil` and `VariantBuilder` (#3472)
---
.../org/apache/parquet/variant/VariantBuilder.java | 2 +-
.../org/apache/parquet/variant/VariantUtil.java | 2 +-
.../apache/parquet/variant/TestVariantObject.java | 35 ++++++++++++++++++++++
.../parquet/variant/TestVariantScalarBuilder.java | 11 +++++++
4 files changed, 48 insertions(+), 2 deletions(-)
diff --git
a/parquet-variant/src/main/java/org/apache/parquet/variant/VariantBuilder.java
b/parquet-variant/src/main/java/org/apache/parquet/variant/VariantBuilder.java
index 4b8eb6d8c..bf42b0c44 100644
---
a/parquet-variant/src/main/java/org/apache/parquet/variant/VariantBuilder.java
+++
b/parquet-variant/src/main/java/org/apache/parquet/variant/VariantBuilder.java
@@ -374,7 +374,7 @@ public class VariantBuilder {
writePos += 1;
VariantUtil.writeLong(writeBuffer, writePos, binarySize,
VariantUtil.U32_SIZE);
writePos += VariantUtil.U32_SIZE;
- ByteBuffer.wrap(writeBuffer, writePos, binarySize).put(binary);
+ ByteBuffer.wrap(writeBuffer, writePos, binarySize).put(binary.duplicate());
writePos += binarySize;
}
diff --git
a/parquet-variant/src/main/java/org/apache/parquet/variant/VariantUtil.java
b/parquet-variant/src/main/java/org/apache/parquet/variant/VariantUtil.java
index 4744f0c28..ef1168583 100644
--- a/parquet-variant/src/main/java/org/apache/parquet/variant/VariantUtil.java
+++ b/parquet-variant/src/main/java/org/apache/parquet/variant/VariantUtil.java
@@ -843,7 +843,7 @@ class VariantUtil {
} else {
// ByteBuffer does not have an array, so we need to use the `get`
method to read the bytes.
byte[] metadataArray = new byte[nextOffset - offset];
- slice(metadata, stringStart + offset).get(metadataArray);
+ slice(metadata, pos + stringStart + offset).get(metadataArray);
result.put(new String(metadataArray), id);
}
offset = nextOffset;
diff --git
a/parquet-variant/src/test/java/org/apache/parquet/variant/TestVariantObject.java
b/parquet-variant/src/test/java/org/apache/parquet/variant/TestVariantObject.java
index 22a53976e..ddcf9f7fd 100644
---
a/parquet-variant/src/test/java/org/apache/parquet/variant/TestVariantObject.java
+++
b/parquet-variant/src/test/java/org/apache/parquet/variant/TestVariantObject.java
@@ -341,4 +341,39 @@ public class TestVariantObject {
Assert.assertEquals("Cannot read ARRAY value as OBJECT", e.getMessage());
}
}
+
+ @Test
+ public void testMetadataWithNonZeroPositionReadOnly() {
+ // Build a variant with object fields to populate the metadata dictionary
+ VariantBuilder vb = new VariantBuilder();
+ VariantObjectBuilder obj = vb.startObject();
+ obj.appendKey("name");
+ obj.appendString("Alice");
+ obj.appendKey("age");
+ obj.appendInt(30);
+ vb.endObject();
+ Variant variant = vb.build();
+
+ // Get the raw metadata bytes
+ ByteBuffer metaBuf = variant.getMetadataBuffer();
+ byte[] metaBytes = new byte[metaBuf.remaining()];
+ metaBuf.duplicate().get(metaBytes);
+
+ // Embed in a larger buffer with a non-zero position, then make read-only
+ // to force the else-branch in getMetadataMap.
+ byte[] padded = new byte[10 + metaBytes.length];
+ System.arraycopy(metaBytes, 0, padded, 10, metaBytes.length);
+ ByteBuffer offsetMetadata = ByteBuffer.wrap(padded);
+ offsetMetadata.position(10);
+ offsetMetadata.limit(10 + metaBytes.length);
+ offsetMetadata = offsetMetadata.asReadOnlyBuffer();
+
+ // ImmutableMetadata calls getMetadataMap, which had the bug.
+ // getMetadataMap builds a key->id dictionary from the metadata buffer.
+ // With a non-zero position and read-only buffer, the else-branch is taken,
+ // which previously used the wrong offset.
+ ImmutableMetadata immutableMetadata = new
ImmutableMetadata(offsetMetadata);
+ Assert.assertEquals(0, immutableMetadata.getOrInsert("name"));
+ Assert.assertEquals(1, immutableMetadata.getOrInsert("age"));
+ }
}
diff --git
a/parquet-variant/src/test/java/org/apache/parquet/variant/TestVariantScalarBuilder.java
b/parquet-variant/src/test/java/org/apache/parquet/variant/TestVariantScalarBuilder.java
index 13ceef86d..05a05d806 100644
---
a/parquet-variant/src/test/java/org/apache/parquet/variant/TestVariantScalarBuilder.java
+++
b/parquet-variant/src/test/java/org/apache/parquet/variant/TestVariantScalarBuilder.java
@@ -387,6 +387,17 @@ public class TestVariantScalarBuilder {
}
}
+ @Test
+ public void testBinaryBuilderDoesNotMutateCallerBuffer() {
+ ByteBuffer buf = ByteBuffer.wrap(new byte[] {0, 1, 2, 3});
+ int positionBefore = buf.position();
+ int remainingBefore = buf.remaining();
+ VariantBuilder vb = new VariantBuilder();
+ vb.appendBinary(buf);
+ Assert.assertEquals(positionBefore, buf.position());
+ Assert.assertEquals(remainingBefore, buf.remaining());
+ }
+
@Test
public void testStringBuilder() {
IntStream.range(VariantUtil.MAX_SHORT_STR_SIZE - 3,
VariantUtil.MAX_SHORT_STR_SIZE + 3)
