Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,44 @@ public class UnsignedByteArray implements Destroyable {
private final List<UnsignedByte> unsignedBytes;
private boolean destroyed;

/**
* Required-args constructor.
*
* <p>Note: {@link UnsignedByte} is mutable (see {@link UnsignedByte#destroy()}), so this constructor defensively
* copies both the supplied {@link List} and every {@link UnsignedByte} in it. This guarantees that destroying this
* array can never mutate an {@link UnsignedByte} owned by the caller (e.g., a shared static constant).
*
* @param unsignedBytes A {@link List} of {@link UnsignedByte}s.
*/
public UnsignedByteArray(final List<UnsignedByte> unsignedBytes) {
Objects.requireNonNull(unsignedBytes);
this.unsignedBytes = unsignedBytes;
this.unsignedBytes = copyOf(unsignedBytes);
}

/**
* Private constructor that takes ownership of {@code unsignedBytes} without copying. Only for use by
* factories in this class that have just allocated fresh {@link UnsignedByte} instances.
*
* @param unsignedBytes A freshly allocated {@link List} of freshly allocated {@link UnsignedByte}s.
* @param ignored Disambiguates this constructor from the public one.
*/
private UnsignedByteArray(final List<UnsignedByte> unsignedBytes, final boolean ignored) {
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
this.unsignedBytes = Objects.requireNonNull(unsignedBytes);
}

/**
* Deep-copies a {@link List} of {@link UnsignedByte}s so the result shares no instances with the input.
*
* @param source The {@link List} to copy.
*
* @return A new {@link List} containing new {@link UnsignedByte} instances with equal values.
*/
private static List<UnsignedByte> copyOf(final List<UnsignedByte> source) {
final List<UnsignedByte> copy = new ArrayList<>(source.size());
for (UnsignedByte unsignedByte : source) {
copy.add(UnsignedByte.of(unsignedByte));
}
return copy;
}

/**
Expand All @@ -59,22 +94,25 @@ public static UnsignedByteArray of(final byte[] bytes) {
for (int i = 0; i < bytes.length; i++) {
unsignedBytes.add(i, UnsignedByte.of(bytes[i]));
}
return new UnsignedByteArray(unsignedBytes);
return new UnsignedByteArray(unsignedBytes, true); // <-- all instances freshly allocated above.
}

/**
* Creates an {@link UnsignedByteArray} from one or more {@link UnsignedByte} values.
* Creates an {@link UnsignedByteArray} from one or more {@link UnsignedByte} values. The supplied values are copied,
* so the returned array never aliases the caller's instances.
*
* @param first An {@link UnsignedByte} to include in the resulting {@link UnsignedByteArray}.
* @param rest Other {@link UnsignedByte}s to include in the resulting {@link UnsignedByteArray}.
*
* @return An {@link UnsignedByteArray} with the given {@link UnsignedByte}s.
*/
public static UnsignedByteArray of(UnsignedByte first, UnsignedByte... rest) {
List<UnsignedByte> unsignedBytes = new ArrayList<>();
unsignedBytes.add(first);
Collections.addAll(unsignedBytes, rest);
return new UnsignedByteArray(unsignedBytes);
List<UnsignedByte> unsignedBytes = new ArrayList<>(1 + rest.length);
unsignedBytes.add(UnsignedByte.of(first));
for (UnsignedByte unsignedByte : rest) {
unsignedBytes.add(UnsignedByte.of(unsignedByte));
}
return new UnsignedByteArray(unsignedBytes, true); // <-- all instances freshly copied above.
}

/**
Expand All @@ -83,7 +121,7 @@ public static UnsignedByteArray of(UnsignedByte first, UnsignedByte... rest) {
* @return An empty {@link UnsignedByteArray}.
*/
public static UnsignedByteArray empty() {
return new UnsignedByteArray(new ArrayList<>());
return new UnsignedByteArray(new ArrayList<>(), true);
}

/**
Expand All @@ -94,7 +132,7 @@ public static UnsignedByteArray empty() {
* @return An {@link UnsignedByteArray} of the requested size with all 0 {@link UnsignedByte}s
*/
public static UnsignedByteArray ofSize(int size) {
return new UnsignedByteArray(fill(size));
return new UnsignedByteArray(fill(size), true); // <-- `fill` allocates fresh instances.
}

/**
Expand All @@ -107,7 +145,7 @@ public static UnsignedByteArray ofSize(int size) {
public static UnsignedByteArray fromHex(String hex) {
Objects.requireNonNull(hex);
List<UnsignedByte> unsignedBytes = ByteUtils.parse(hex.toUpperCase(Locale.ENGLISH));
return new UnsignedByteArray(unsignedBytes);
return new UnsignedByteArray(unsignedBytes, true); // <-- `parse` allocates fresh instances.
}

protected static List<UnsignedByte> fill(int amount) {
Expand Down Expand Up @@ -172,38 +210,40 @@ public UnsignedByte get(int index) {
}

/**
* Appends an {@link UnsignedByte} to this {@link UnsignedByteArray}.
* Appends a copy of an {@link UnsignedByte} to this {@link UnsignedByteArray}. Note: this method mutates the instance
* and returns the same instance (mainly for call chaining convenience).
*
* @param unsignedByte An {@link UnsignedByte} to append.
* @param unsignedByte An {@link UnsignedByte} to append (copied; the caller's instance is never aliased).
*
* @return This {@link UnsignedByteArray}, with the given {@link UnsignedByte} appended.
*/
public UnsignedByteArray append(UnsignedByte unsignedByte) {
unsignedBytes.add(unsignedByte);
unsignedBytes.add(UnsignedByte.of(unsignedByte));
return this;
}

/**
* Appends the given bytes to the end of this array. Note: this method mutates the instance and returns the same
* instance (mainly for call chaining convenience).
* Appends copies of the given bytes to the end of this array. Note: this method mutates the instance and returns the
* same instance (mainly for call chaining convenience). The elements of {@code array} are copied, so the two arrays
* never share {@link UnsignedByte} instances.
*
* @param array An {@link UnsignedByteArray} to append to this {@link UnsignedByteArray}.
*
* @return the same instance.
*/
public UnsignedByteArray append(UnsignedByteArray array) {
unsignedBytes.addAll(array.getUnsignedBytes());
unsignedBytes.addAll(copyOf(array.getUnsignedBytes()));
return this;
}

/**
* Sets the value at the given index to the given {@link UnsignedByte}.
* Sets the value at the given index to a copy of the given {@link UnsignedByte}.
*
* @param index The index to set.
* @param value The {@link UnsignedByte} to set at the given index.
* @param value The {@link UnsignedByte} to set at the given index (copied; the caller's instance is never aliased).
*/
public void set(int index, UnsignedByte value) {
unsignedBytes.set(index, value);
unsignedBytes.set(index, UnsignedByte.of(value));
}

/**
Expand All @@ -212,10 +252,10 @@ public void set(int index, UnsignedByte value) {
* @param startIndex start index (inclusive)
* @param endIndex end index (exclusive)
*
* @return An {@link UnsignedByteArray} containing the sliced elements.
* @return A new, independent {@link UnsignedByteArray} containing copies of the sliced elements.
*/
public UnsignedByteArray slice(int startIndex, int endIndex) {
return new UnsignedByteArray(unsignedBytes.subList(startIndex, endIndex));
return new UnsignedByteArray(unsignedBytes.subList(startIndex, endIndex)); // <-- public ctor copies.
}

/**
Expand All @@ -226,9 +266,9 @@ public UnsignedByteArray slice(int startIndex, int endIndex) {
*/
@Beta
public UnsignedByteArray reverse() {
List<UnsignedByte> reversed = new ArrayList<>(unsignedBytes);
List<UnsignedByte> reversed = copyOf(unsignedBytes);
Collections.reverse(reversed);
return new UnsignedByteArray(reversed);
return new UnsignedByteArray(reversed, true); // <-- `copyOf` allocates fresh instances.
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,4 +223,93 @@ void destroy() {
assertThat(uba.hexValue()).isEqualTo("");
assertThat(uba.hashCode()).isEqualTo(32);
}
/////////
// Defensive copying: an UnsignedByteArray must never alias UnsignedByte instances it did not create, because
// UnsignedByte is mutable (destroy() zeroes it) and callers may pass shared/static constants.
/////////

@Test
void constructorCopiesListAndElements() {
UnsignedByte shared = UnsignedByte.of(0xED);
List<UnsignedByte> list = new ArrayList<>();
list.add(shared);
UnsignedByteArray array = new UnsignedByteArray(list);

assertThat(array.get(0)).isNotSameAs(shared);
array.destroy();
assertThat(shared.asInt()).isEqualTo(0xED);
assertThat(shared.isDestroyed()).isFalse();
assertThat(list).containsExactly(shared); // caller's list untouched
}

@Test
void ofUnsignedBytesCopiesElements() {
UnsignedByte first = UnsignedByte.of(0xED);
UnsignedByte second = UnsignedByte.of(0x01);
UnsignedByteArray array = UnsignedByteArray.of(first, second);

assertThat(array.get(0)).isNotSameAs(first);
assertThat(array.get(1)).isNotSameAs(second);
array.destroy();
assertThat(first.asInt()).isEqualTo(0xED);
assertThat(first.isDestroyed()).isFalse();
assertThat(second.asInt()).isEqualTo(0x01);
assertThat(second.isDestroyed()).isFalse();
}

@Test
void appendUnsignedByteCopiesElement() {
UnsignedByte shared = UnsignedByte.of(0xED);
UnsignedByteArray array = UnsignedByteArray.empty().append(shared);

assertThat(array.get(0)).isNotSameAs(shared);
array.destroy();
assertThat(shared.asInt()).isEqualTo(0xED);
assertThat(shared.isDestroyed()).isFalse();
}

@Test
void appendUnsignedByteArrayCopiesElements() {
UnsignedByteArray source = UnsignedByteArray.of(new byte[] {(byte) 0xED, 0x01});
UnsignedByteArray array = UnsignedByteArray.empty().append(source);

assertThat(array.get(0)).isNotSameAs(source.get(0));
array.destroy();
assertThat(source.hexValue()).isEqualTo("ED01");
assertThat(source.get(0).isDestroyed()).isFalse();
assertThat(source.isDestroyed()).isFalse();
}

@Test
void setCopiesElement() {
UnsignedByte shared = UnsignedByte.of(0xED);
UnsignedByteArray array = UnsignedByteArray.ofSize(1);
array.set(0, shared);

assertThat(array.get(0)).isNotSameAs(shared);
array.destroy();
assertThat(shared.asInt()).isEqualTo(0xED);
assertThat(shared.isDestroyed()).isFalse();
}

@Test
void sliceDoesNotAliasParent() {
UnsignedByteArray parent = UnsignedByteArray.of(new byte[] {0, 8, 9, 1});
UnsignedByteArray slice = parent.slice(1, 3);

slice.destroy();
assertThat(parent.hexValue()).isEqualTo("00080901");
assertThat(parent.length()).isEqualTo(4);
assertThat(parent.isDestroyed()).isFalse();
}

@Test
void reverseDoesNotAliasSource() {
UnsignedByteArray source = UnsignedByteArray.of(new byte[] {1, 2});
UnsignedByteArray reversed = source.reverse();

reversed.destroy();
assertThat(source.hexValue()).isEqualTo("0102");
assertThat(source.isDestroyed()).isFalse();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package org.xrpl.xrpl4j.codec.addresses;

import java.lang.reflect.Field;

/**
* Test-only helpers for {@link UnsignedByte}.
*/
public final class UnsignedByteTestUtils {

private UnsignedByteTestUtils() {
}

/**
* Forcibly resets a (possibly shared/static) {@link UnsignedByte} to the supplied value and clears its destroyed
* flag. Used as a safety net so that a regression in defensive copying cannot poison every other test in the fork.
*
* @param target The {@link UnsignedByte} to reset.
* @param value The value to restore.
*/
public static void reset(final UnsignedByte target, final int value) {
try {
Field valueField = UnsignedByte.class.getDeclaredField("value");
valueField.setAccessible(true);
valueField.setInt(target, value);
Field destroyedField = UnsignedByte.class.getDeclaredField("destroyed");
destroyedField.setAccessible(true);
destroyedField.setBoolean(target, false);
} catch (ReflectiveOperationException e) {
throw new IllegalStateException(e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -164,4 +164,19 @@ private void assertInvalidMptId(String invalidId) {
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("mpt_issuance_id must be a 48-character hexadecimal string");
}

/**
* Regression: MPT issues embed the static ACCOUNT_ONE bytes. Destroying one serialized value must not corrupt the
* shared constant and thereby every later MPT serialization.
*/
@Test
void destroyingMptIssueValueDoesNotCorruptLaterSerializations() throws JsonProcessingException {
JsonNode mptIssue = objectMapper.createObjectNode()
.set("mpt_issuance_id", new TextNode("00000002AE123A8556F3CF91154711376AFB0F894F832B3D"));

String expectedHex = codec.fromJson(mptIssue).toHex();
codec.fromJson(mptIssue).value().destroy();

assertThat(codec.fromJson(mptIssue).toHex()).isEqualTo(expectedHex);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package org.xrpl.xrpl4j.crypto.keys;

import static org.assertj.core.api.Assertions.assertThat;

import com.google.common.io.BaseEncoding;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
import org.xrpl.xrpl4j.codec.addresses.KeyType;
import org.xrpl.xrpl4j.codec.addresses.UnsignedByteArray;
import org.xrpl.xrpl4j.codec.addresses.UnsignedByteTestUtils;
import org.xrpl.xrpl4j.crypto.TestConstants;

/**
* Regression tests proving that destroying the result of {@link PrivateKey#prefixedBytes()} cannot mutate the shared
* {@link PrivateKey#ED2559_PREFIX} / {@link PrivateKey#SECP256K1_PREFIX} constants.
*/
class PrivateKeyPrefixDestroyTest {

/**
* Safety net: if defensive copying ever regresses, don't let it poison every other test in this JVM fork.
*/
@AfterEach
void restorePrefixes() {
UnsignedByteTestUtils.reset(PrivateKey.ED2559_PREFIX, 0xED);
UnsignedByteTestUtils.reset(PrivateKey.SECP256K1_PREFIX, 0x00);
}

@Test
void prefixedBytesDoesNotShareEd25519PrefixInstance() {
UnsignedByteArray exported = TestConstants.getEdPrivateKey().prefixedBytes();
assertThat(exported.get(0)).isNotSameAs(PrivateKey.ED2559_PREFIX);
assertThat(exported.get(0)).isEqualTo(PrivateKey.ED2559_PREFIX);
}

@Test
void prefixedBytesDoesNotShareSecp256k1PrefixInstance() {
UnsignedByteArray exported = TestConstants.getEcPrivateKey().prefixedBytes();
assertThat(exported.get(0)).isNotSameAs(PrivateKey.SECP256K1_PREFIX);
assertThat(exported.get(0)).isEqualTo(PrivateKey.SECP256K1_PREFIX);
}

@Test
void destroyingEd25519ExportLeavesSharedPrefixIntact() {
UnsignedByteArray exported = TestConstants.getEdPrivateKey().prefixedBytes();
exported.destroy();

assertThat(PrivateKey.ED2559_PREFIX.asInt()).isEqualTo(0xED);
assertThat(PrivateKey.ED2559_PREFIX.isDestroyed()).isFalse();

// A fresh export still carries the correct prefix...
assertThat(TestConstants.getEdPrivateKey().prefixedBytes().hexValue()).startsWith("ED");

// ...and fromPrefixedBytes still classifies correctly.
UnsignedByteArray realEdBytes = UnsignedByteArray.of(
BaseEncoding.base16().decode(TestConstants.ED_PRIVATE_KEY_WITH_PREFIX_HEX));
assertThat(PrivateKey.fromPrefixedBytes(realEdBytes).keyType()).isEqualTo(KeyType.ED25519);
UnsignedByteArray realEcBytes = UnsignedByteArray.of(
BaseEncoding.base16().decode(TestConstants.EC_PRIVATE_KEY_WITH_PREFIX_HEX));
assertThat(PrivateKey.fromPrefixedBytes(realEcBytes).keyType()).isEqualTo(KeyType.SECP256K1);
}

@Test
void destroyingSecp256k1ExportLeavesSharedPrefixIntact() {
UnsignedByteArray exported = TestConstants.getEcPrivateKey().prefixedBytes();
exported.destroy();

assertThat(PrivateKey.SECP256K1_PREFIX.asInt()).isEqualTo(0x00);
assertThat(PrivateKey.SECP256K1_PREFIX.isDestroyed()).isFalse();
}
}
Loading
Loading