(tomcat) 08/14: Copy the byte array in the UniqueId(byte[]) constructor

[re...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 9e5b68da33689c63b9ab9219702e27dfd887108b
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 9 11:09:27 2026 +0200

    Copy the byte array in the UniqueId(byte[]) constructor
    
    The single-argument constructor retained the caller's array by
    reference, unlike the three-argument constructor which copies. Since
    UniqueId computes equals() and hashCode() from that array and is used
    as a map key, e.g. in TwoPhaseCommitInterceptor, a caller mutating the
    original array after construction could silently corrupt key lookups.
    No current call site mutates a shared array, so this is a latent API
    hazard rather than an active bug.
    
    Make the constructor store a defensive copy and update the Javadoc of
    the constructor and of getBytes() accordingly. Add TestUniqueId to
    cover the copy semantics, null handling and map key stability.
---
 java/org/apache/catalina/tribes/UniqueId.java     |  9 +--
 test/org/apache/catalina/tribes/TestUniqueId.java | 67 +++++++++++++++++++++++
 2 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/java/org/apache/catalina/tribes/UniqueId.java b/java/org/apache/catalina/tribes/UniqueId.java
index bbe2d7918f..ceb7baf27d 100644
--- a/java/org/apache/catalina/tribes/UniqueId.java
+++ b/java/org/apache/catalina/tribes/UniqueId.java
@@ -39,12 +39,13 @@ public final class UniqueId implements Serializable {
     }
 
     /**
-     * Constructs a new UniqueId from the given byte array.
+     * Constructs a new UniqueId from the given byte array. A defensive copy of the array is made, so later changes to
+     * the caller's array do not affect this object.
      *
-     * @param id the byte array containing the identifier. The array is retained by reference and must not be modified.
+     * @param id the byte array containing the identifier
      */
     public UniqueId(byte[] id) {
-        this.id = id;
+        this.id = id != null ? id.clone() : null;
     }
 
     /**
@@ -86,7 +87,7 @@ public final class UniqueId implements Serializable {
     }
 
     /**
-     * Returns the raw bytes of this unique identifier.
+     * Returns the raw bytes of this unique identifier. Do not modify the returned array.
      *
      * @return the byte array
      */
diff --git a/test/org/apache/catalina/tribes/TestUniqueId.java b/test/org/apache/catalina/tribes/TestUniqueId.java
new file mode 100644
index 0000000000..966f0bf5c1
--- /dev/null
+++ b/test/org/apache/catalina/tribes/TestUniqueId.java
@@ -0,0 +1,67 @@
+/*
+ * 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
+ *
+ *      http://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.catalina.tribes;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestUniqueId {
+
+    @Test
+    public void testNullId() {
+        UniqueId uid = new UniqueId();
+        Assert.assertNull(uid.getBytes());
+        Assert.assertEquals(new UniqueId(), uid);
+        Assert.assertEquals(new UniqueId().hashCode(), uid.hashCode());
+    }
+
+    @Test
+    public void testByteArrayConstructorDoesNotRetainReference() {
+        byte[] id = new byte[] { 1, 2, 3, 4 };
+        UniqueId uid = new UniqueId(id);
+        Assert.assertNotSame(id, uid.getBytes());
+        Assert.assertArrayEquals(id, uid.getBytes());
+        id[0] = 99;
+        // The UniqueId must be unaffected by the mutation above
+        Assert.assertEquals(1, uid.getBytes()[0]);
+    }
+
+    @Test
+    public void testMapKeySurvivesCallerMutation() {
+        byte[] id = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 };
+        UniqueId uid = new UniqueId(id);
+        Map<UniqueId, String> map = new HashMap<>();
+        map.put(uid, "value");
+        id[0] = 99;
+        Assert.assertEquals("value", map.get(uid));
+        UniqueId equalId = new UniqueId(
+                new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 });
+        Assert.assertTrue(map.containsKey(equalId));
+    }
+
+    @Test
+    public void testRangeConstructorCopies() {
+        byte[] source = new byte[] { 0, 1, 2, 3, 4 };
+        UniqueId uid = new UniqueId(source, 1, 3);
+        Assert.assertNotNull(uid.getBytes());
+        source[1] = 99;
+        Assert.assertEquals(1, uid.getBytes()[0]);
+    }
+}


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org