[BCEL-238] Defensively copy BootstrapMethod argument arrays

src/changes/changes.xml

@@ -61,6 +61,7 @@ The <action> type attribute can be add,update,fix,remove.
   <body>
     <release version="6.14.0" date="YYYY-MM-DD" description="This is a feature and maintenance release. Java 8 or later is required.">
       <!-- FIX -->
+      <action issue="BCEL-238" type="fix" due-to="agamal026">Defensively copy BootstrapMethod argument arrays on construction, assignment, and retrieval.</action>
       <action                  type="fix" dev="ggregory" due-to="Gary Gregory">Use modern Mockito on Java 11+ (#528).</action>
       <action                  type="fix" dev="ggregory" due-to="Max Kammerer, akashchamp, Gary Gregory" issue="BCEL-280">MethodGen.setMaxLocals() should account for LocalVariableTable entries (#529).</action>
       <action                  type="fix" dev="ggregory" due-to="Gary Gregory">Fix constant pool module access test on Java 27 (#534).</action>

src/main/java/org/apache/bcel/classfile/BootstrapMethod.java

@@ -77,6 +77,8 @@ private BootstrapMethod(final int bootstrapMethodRef, final int numBootstrapArgu
     /**
      * Constructs a BootstrapMethod.
      *
+     * <p>The supplied argument array is copied. A {@code null} array is treated as an empty array.</p>
+     *
      * @param bootstrapMethodRef int index into constant_pool of CONSTANT_MethodHandle.
      * @param bootstrapArguments int[] indices into constant_pool of CONSTANT_[type]_info.
      */
@@ -116,12 +118,14 @@ public final void dump(final DataOutputStream file) throws IOException {
     }
 
     /**
-     * Gets the bootstrap arguments.
+     * Gets a copy of the bootstrap arguments.
+     *
+     * <p>Changes to the returned array do not affect this object. Use {@link #setBootstrapArguments(int[])} to update the arguments.</p>
      *
-     * @return int[] of bootstrap_method indices into constant_pool of CONSTANT_[type]_info.
+     * @return a copy of the bootstrap_method indices into constant_pool of CONSTANT_[type]_info.
      */
     public int[] getBootstrapArguments() {
-        return bootstrapArguments;
+        return bootstrapArguments.clone();
     }
 
     /**
@@ -143,12 +147,14 @@ public int getNumBootstrapArguments() {
     }
 
     /**
-     * Sets the bootstrap arguments.
+     * Sets the bootstrap arguments from a copy of the supplied array.
+     *
+     * <p>A {@code null} array is treated as an empty array.</p>
      *
      * @param bootstrapArguments int[] indices into constant_pool of CONSTANT_[type]_info.
      */
     public void setBootstrapArguments(final int[] bootstrapArguments) {
-        this.bootstrapArguments = ArrayUtils.nullToEmpty(bootstrapArguments);
+        this.bootstrapArguments = ArrayUtils.nullToEmpty(bootstrapArguments).clone();
     }
 
     /**

src/test/java/org/apache/bcel/classfile/BootstrapMethodCopyTest.java

@@ -20,27 +20,42 @@
 package org.apache.bcel.classfile;
 
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 
 import org.junit.jupiter.api.Test;
 
 /**
- * Tests that {@link BootstrapMethod} copies do not share the mutable {@code bootstrapArguments} array with their source.
+ * Tests that {@link BootstrapMethod} copies preserve their source's values and can be updated independently.
  */
 class BootstrapMethodCopyTest {
 
+    private static void assertIndependentUpdates(final BootstrapMethod original, final BootstrapMethod copy) {
+        assertEquals(original.getBootstrapMethodRef(), copy.getBootstrapMethodRef());
+        assertArrayEquals(original.getBootstrapArguments(), copy.getBootstrapArguments());
+
+        final int[] changedArguments = copy.getBootstrapArguments();
+        changedArguments[0] = 999;
+        copy.setBootstrapArguments(changedArguments);
+        assertArrayEquals(new int[] {999, 20, 30}, copy.getBootstrapArguments());
+        assertArrayEquals(new int[] {10, 20, 30}, original.getBootstrapArguments());
+
+        original.setBootstrapArguments(new int[] {40, 50});
+        original.setBootstrapMethodRef(2);
+        assertArrayEquals(new int[] {40, 50}, original.getBootstrapArguments());
+        assertArrayEquals(new int[] {999, 20, 30}, copy.getBootstrapArguments());
+        assertEquals(2, original.getBootstrapMethodRef());
+        assertEquals(1, copy.getBootstrapMethodRef());
+    }
+
     @Test
-    void testCopyConstructorDoesNotShareArguments() {
+    void testCopyConstructorCanBeUpdatedIndependently() {
         final BootstrapMethod original = new BootstrapMethod(1, new int[] {10, 20, 30});
-        final BootstrapMethod copy = new BootstrapMethod(original);
-        copy.getBootstrapArguments()[0] = 999;
-        assertArrayEquals(new int[] {10, 20, 30}, original.getBootstrapArguments());
+        assertIndependentUpdates(original, new BootstrapMethod(original));
     }
 
     @Test
-    void testCopyDoesNotShareArguments() {
+    void testCopyCanBeUpdatedIndependently() {
         final BootstrapMethod original = new BootstrapMethod(1, new int[] {10, 20, 30});
-        final BootstrapMethod copy = original.copy();
-        copy.getBootstrapArguments()[0] = 999;
-        assertArrayEquals(new int[] {10, 20, 30}, original.getBootstrapArguments());
+        assertIndependentUpdates(original, original.copy());
     }
 }

src/test/java/org/apache/bcel/classfile/BootstrapMethodTest.java

@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.bcel.classfile;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+
+import org.junit.jupiter.api.Test;
+
+class BootstrapMethodTest {
+
+    private static byte[] dump(final BootstrapMethod method) throws IOException {
+        try (ByteArrayOutputStream bytes = new ByteArrayOutputStream(); DataOutputStream output = new DataOutputStream(bytes)) {
+            method.dump(output);
+            return bytes.toByteArray();
+        }
+    }
+
+    @Test
+    void testConstructorCopiesArguments() {
+        final int[] arguments = {10, 20, 30};
+        final BootstrapMethod method = new BootstrapMethod(1, arguments);
+        arguments[0] = 999;
+        assertArrayEquals(new int[] {10, 20, 30}, method.getBootstrapArguments());
+        assertEquals(3, method.getNumBootstrapArguments());
+    }
+
+    @Test
+    void testConstructorNormalizesNull() throws IOException {
+        final BootstrapMethod method = new BootstrapMethod(1, (int[]) null);
+        assertArrayEquals(new int[0], method.getBootstrapArguments());
+        assertEquals(0, method.getNumBootstrapArguments());
+        assertArrayEquals(new int[0], new BootstrapMethod(method).getBootstrapArguments());
+        assertArrayEquals(new int[0], method.copy().getBootstrapArguments());
+        assertArrayEquals(new byte[] {0, 1, 0, 0}, dump(method));
+    }
+
+    @Test
+    void testDumpChangesOnlyAfterExplicitUpdate() throws IOException {
+        final int[] arguments = {10, 20};
+        final BootstrapMethod method = new BootstrapMethod(1, arguments);
+        arguments[0] = 99;
+        method.getBootstrapArguments()[1] = 88;
+        assertArrayEquals(new byte[] {0, 1, 0, 2, 0, 10, 0, 20}, dump(method));
+
+        final int[] replacement = {30};
+        method.setBootstrapArguments(replacement);
+        replacement[0] = 77;
+        assertArrayEquals(new byte[] {0, 1, 0, 1, 0, 30}, dump(method));
+    }
+
+    @Test
+    void testGetterReturnsSnapshot() {
+        final BootstrapMethod method = new BootstrapMethod(1, new int[] {10, 20, 30});
+        final int[] arguments = method.getBootstrapArguments();
+        arguments[0] = 999;
+        assertArrayEquals(new int[] {10, 20, 30}, method.getBootstrapArguments());
+    }
+
+    @Test
+    void testReadAndDumpArguments() throws IOException {
+        final byte[] expected = {0, 1, 0, 3, 0, 10, 0, 20, 0, 30};
+        try (DataInputStream input = new DataInputStream(new ByteArrayInputStream(expected))) {
+            final BootstrapMethod method = new BootstrapMethod(input);
+            assertEquals(1, method.getBootstrapMethodRef());
+            assertEquals(3, method.getNumBootstrapArguments());
+            assertArrayEquals(new int[] {10, 20, 30}, method.getBootstrapArguments());
+            assertArrayEquals(expected, dump(method));
+        }
+    }
+
+    @Test
+    void testSetterCopiesArguments() {
+        final BootstrapMethod method = new BootstrapMethod(1, new int[] {10});
+        final int[] arguments = {20, 30};
+        method.setBootstrapArguments(arguments);
+        arguments[0] = 999;
+        assertArrayEquals(new int[] {20, 30}, method.getBootstrapArguments());
+        assertEquals(2, method.getNumBootstrapArguments());
+    }
+
+    @Test
+    void testSetterNormalizesNull() throws IOException {
+        final BootstrapMethod method = new BootstrapMethod(1, new int[] {10, 20, 30});
+        method.setBootstrapArguments(null);
+        assertArrayEquals(new int[0], method.getBootstrapArguments());
+        assertEquals(0, method.getNumBootstrapArguments());
+        assertArrayEquals(new int[0], method.copy().getBootstrapArguments());
+        assertArrayEquals(new byte[] {0, 1, 0, 0}, dump(method));
+    }
+}