This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 63c4210cac802aa4c278ff34acf19f6c3df6a53a Author: opencode <[email protected]> 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 57b78df5d7..91e018bb56 100644 --- a/java/org/apache/catalina/tribes/UniqueId.java +++ b/java/org/apache/catalina/tribes/UniqueId.java @@ -41,12 +41,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; } /** @@ -88,7 +89,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: [email protected] For additional commands, e-mail: [email protected]
