From 4b55c8c496ad8cefe4483d039f184d2007eab562 Mon Sep 17 00:00:00 2001
From: Robert Yokota <rayokota@gmail.com>
Date: Fri, 10 Apr 2026 18:17:45 -0700
Subject: [PATCH] Fix ByteBuffer handling in VariantUtil and VariantBuilder

---
 .../parquet/variant/VariantBuilder.java       |  2 +-
 .../apache/parquet/variant/VariantUtil.java   |  2 +-
 .../parquet/variant/TestVariantObject.java    | 35 +++++++++++++++++++
 .../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 4b8eb6d8c3..bf42b0c44f 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 void appendBinary(ByteBuffer binary) {
     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 4744f0c28d..ef1168583a 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 @@ static HashMap<String, Integer> getMetadataMap(ByteBuffer metadata) {
       } 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 22a53976e9..ddcf9f7fda 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 void testInvalidObject() {
       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 13ceef86d1..05a05d8069 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 void testBinaryBuilder() {
     }
   }
 
+  @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)