diff --git a/pbj-core/gradle.properties b/pbj-core/gradle.properties index 710515d0..7ac00018 100644 --- a/pbj-core/gradle.properties +++ b/pbj-core/gradle.properties @@ -1,5 +1,5 @@ # Version number -version=0.7.0-SNAPSHOT +version=0.7.5-SNAPSHOT # Need increased heap for running Gradle itself, or SonarQube will run the JVM out of metaspace org.gradle.jvmargs=-Xmx2048m diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/ProtoParserTools.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/ProtoParserTools.java index 8fce63b5..4d3525ec 100644 --- a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/ProtoParserTools.java +++ b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/ProtoParserTools.java @@ -225,8 +225,7 @@ public static String readString(final ReadableSequentialData input) throws IOExc * Read a Bytes field from data input * * @param input the input to read from - * @return read Bytes object, this can be a copy or a direct reference to inputs data. So it has same life span - * of InputData + * @return read Bytes object, this will be a copy of the data * @throws IOException If there was a problem reading */ public static Bytes readBytes(final ReadableSequentialData input) throws IOException { diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/ReadableSequentialData.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/ReadableSequentialData.java index 286cafb1..eb0323a4 100644 --- a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/ReadableSequentialData.java +++ b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/ReadableSequentialData.java @@ -180,6 +180,8 @@ default long readBytes(@NonNull final BufferedData dst) { * the read data. The returned bytes will be immutable. The {@link #position()} of this sequence will be * incremented by {@code length} bytes. * + *

All data returned will always be a copy otherwise the underlying data could be modified. + * *

Bytes are read from the sequence one at a time. If there are not {@code length} bytes remaining in this * sequence, then a {@link BufferUnderflowException} will be thrown. The {@link #position()} will be * incremented by the number of bytes read prior to the exception. diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/BufferedData.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/BufferedData.java index 01846864..19bae4bd 100644 --- a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/BufferedData.java +++ b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/BufferedData.java @@ -367,17 +367,16 @@ public long getBytes(final long offset, @NonNull final BufferedData dst) { return len; } + /** {@inheritDoc} */ @NonNull @Override public Bytes getBytes(long offset, long length) { final var len = Math.toIntExact(length); - if (direct) { - final var copy = new byte[len]; - buffer.get(Math.toIntExact(offset), copy, 0, len); - return Bytes.wrap(copy); - } else { - return Bytes.wrap(buffer.array(), Math.toIntExact(buffer.arrayOffset() + offset), len); - } + if(len < 0) throw new IllegalArgumentException("Length cannot be negative"); + // It is vital that we always copy here, we can never assume ownership of the underlying buffer + final var copy = new byte[len]; + buffer.get(Math.toIntExact(offset), copy, 0, len); + return Bytes.wrap(copy); } /** {@inheritDoc} */ diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/Bytes.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/Bytes.java index e1b245b7..18c67443 100644 --- a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/Bytes.java +++ b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/Bytes.java @@ -332,7 +332,7 @@ public int read() throws IOException { @Override @NonNull public String toString() { - return HexFormat.of().formatHex(buffer, start, length); + return HexFormat.of().formatHex(buffer, start, start + length); } /** @@ -486,6 +486,7 @@ public Bytes getBytes(long offset, long length) { return Bytes.EMPTY; } + // Our buffer is assumed to be immutable, so we can just return a new Bytes object that wraps the same buffer return new Bytes(buffer, Math.toIntExact(start + offset), Math.toIntExact(length)); } diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/RandomAccessData.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/RandomAccessData.java index fe319958..c78e39c8 100644 --- a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/RandomAccessData.java +++ b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/RandomAccessData.java @@ -142,6 +142,8 @@ default long getBytes(final long offset, @NonNull final BufferedData dst) { * Get {@code length} bytes starting at the given {@code offset} from this buffer. The returned bytes will * be immutable. The returned {@link Bytes} will have exactly {@code length} bytes. * + *

All data returned will always be a copy otherwise the underlying data could be modified. + * * @param offset The offset into data to begin reading bytes * @param length The non-negative length in bytes to read * @return new {@link Bytes} containing the read data diff --git a/pbj-core/pbj-runtime/src/test/java/com/hedera/pbj/runtime/io/buffer/BufferedDataTest.java b/pbj-core/pbj-runtime/src/test/java/com/hedera/pbj/runtime/io/buffer/BufferedDataTest.java index 846e26a0..c6f5fb0b 100644 --- a/pbj-core/pbj-runtime/src/test/java/com/hedera/pbj/runtime/io/buffer/BufferedDataTest.java +++ b/pbj-core/pbj-runtime/src/test/java/com/hedera/pbj/runtime/io/buffer/BufferedDataTest.java @@ -1,5 +1,6 @@ package com.hedera.pbj.runtime.io.buffer; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import com.hedera.pbj.runtime.io.ReadableSequentialData; @@ -7,6 +8,7 @@ import com.hedera.pbj.runtime.io.WritableSequentialData; import com.hedera.pbj.runtime.io.WritableTestBase; import edu.umd.cs.findbugs.annotations.NonNull; +import java.util.Arrays; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -93,6 +95,21 @@ void sliceThenReadBytes() { assertEquals((byte) (i + START), slicedBytes.getByte(i)); } } + @Test + @DisplayName("readBytes() does always copy") + void readBytesCopies() { + byte[] bytes = new byte[] {1, 2, 3, 4, 5}; + byte[] bytesOrig = bytes.clone(); + final var buf = BufferedData.wrap(bytes); + final Bytes readBytes = buf.readBytes(5); + assertArrayEquals(bytesOrig, readBytes.toByteArray()); + bytes[0] = 127; + bytes[1] = 127; + bytes[2] = 127; + bytes[3] = 127; + bytes[4] = 127; + assertArrayEquals(bytesOrig, readBytes.toByteArray()); + } @Nested final class ReadableSequentialDataTest extends ReadableTestBase {