104 no pass though (#105)

Signed-off-by: jasperpotts <jasperpotts@users.noreply.github.com>
Signed-off-by: Richard Bair <rbair23@users.noreply.github.com>
Co-authored-by: jasperpotts <jasperpotts@users.noreply.github.com>
Co-authored-by: Richard Bair <rbair23@users.noreply.github.com>

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

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 {

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.
      *
+     * <p>All data returned will always be a copy otherwise the underlying data could be modified.
+     *
      * <p>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.

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} */

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));
     }
 

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.
      *
+     * <p>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

pbj-core/pbj-runtime/src/test/java/com/hedera/pbj/runtime/io/buffer/BufferedDataTest.java

@@ -1,12 +1,14 @@
 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;
 import com.hedera.pbj.runtime.io.ReadableTestBase;
 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 {