From 5ed5ec6a73ffa9037535aa1f164a7a7bce379450 Mon Sep 17 00:00:00 2001 From: Jonah Beckford <9566106-jonahbeckford@users.noreply.gitlab.com> Date: Thu, 31 Aug 2023 21:22:21 -0700 Subject: [PATCH] Support signed offsets Both Structs and Lists support two's complement "Signed" offsets: > B (30 bits) = Offset, in words, from the end > of the pointer to the start of the struct's > data section. Signed. > B (30 bits) = Offset, in words, from the end > of the pointer to the start of the first > element of the list. Signed. The prior code only supported positive offsets because it used the Java >>> operator, which is an unsigned shift operator. The Java >> operator is the signed shift operator. Audited the remaining uses of the shift operator; they were correct and they are documented as such. Positive offsets are only guaranteed in a Canonical message. --- .../main/java/org/capnproto/FarPointer.java | 4 ++ .../main/java/org/capnproto/ListPointer.java | 3 ++ .../java/org/capnproto/StructPointer.java | 3 ++ .../main/java/org/capnproto/WirePointer.java | 4 +- .../java/org/capnproto/SegmentReaderTest.java | 51 +++++++++++++++++++ 5 files changed, 64 insertions(+), 1 deletion(-) diff --git a/runtime/src/main/java/org/capnproto/FarPointer.java b/runtime/src/main/java/org/capnproto/FarPointer.java index 4f80548d..fd8b1f07 100644 --- a/runtime/src/main/java/org/capnproto/FarPointer.java +++ b/runtime/src/main/java/org/capnproto/FarPointer.java @@ -29,10 +29,14 @@ public static int getSegmentId(long ref) { } public static int positionInSegment(long ref) { + /* The [ref] Offset (the "C" section) is "Unsigned", + so use unsigned >>> operator. */ return WirePointer.offsetAndKind(ref) >>> 3; } public static boolean isDoubleFar(long ref) { + /* The [ref] Offset (the "C" section) is "Unsigned", + so use unsigned >>> operator. */ return ((WirePointer.offsetAndKind(ref) >>> 2) & 1) != 0; } diff --git a/runtime/src/main/java/org/capnproto/ListPointer.java b/runtime/src/main/java/org/capnproto/ListPointer.java index c73f25b8..9d18da1f 100644 --- a/runtime/src/main/java/org/capnproto/ListPointer.java +++ b/runtime/src/main/java/org/capnproto/ListPointer.java @@ -29,6 +29,9 @@ public static byte elementSize(long ref) { } public static int elementCount(long ref) { + /* The [ref] List Size (the "D" section) is not specified + as Signed or Unsigned, but number of elements is inherently non-negative. + So use unsigned >>> operator. */ return WirePointer.upper32Bits(ref) >>> 3; } diff --git a/runtime/src/main/java/org/capnproto/StructPointer.java b/runtime/src/main/java/org/capnproto/StructPointer.java index 855ea74a..8d270f09 100644 --- a/runtime/src/main/java/org/capnproto/StructPointer.java +++ b/runtime/src/main/java/org/capnproto/StructPointer.java @@ -30,6 +30,9 @@ public static int dataSize(long ref) { } public static int ptrCount(long ref) { + /* The [ref] Pointer Section Size (the "D" section) is not specified + as Signed or Unsigned, but section size is inherently non-negative. + So use unsigned >>> operator. */ return WirePointer.upper32Bits(ref) >>> 16; } diff --git a/runtime/src/main/java/org/capnproto/WirePointer.java b/runtime/src/main/java/org/capnproto/WirePointer.java index 4387f8ef..bc375b99 100644 --- a/runtime/src/main/java/org/capnproto/WirePointer.java +++ b/runtime/src/main/java/org/capnproto/WirePointer.java @@ -42,7 +42,9 @@ public static byte kind(long wirePointer) { } public static int target(int offset, long wirePointer) { - return offset + 1 + (offsetAndKind(wirePointer) >>> 2); + /* The [wirePointer] Offset (the "B" section) is "Signed", + so use signed >> operator. */ + return offset + 1 + (offsetAndKind(wirePointer) >> 2); } public static void setKindAndTarget(ByteBuffer buffer, int offset, byte kind, int targetOffset) { diff --git a/runtime/src/test/java/org/capnproto/SegmentReaderTest.java b/runtime/src/test/java/org/capnproto/SegmentReaderTest.java index 404d40c9..f855ea36 100644 --- a/runtime/src/test/java/org/capnproto/SegmentReaderTest.java +++ b/runtime/src/test/java/org/capnproto/SegmentReaderTest.java @@ -1,9 +1,11 @@ package org.capnproto; +import org.capnproto.WireHelpers.FollowFarsResult; import org.hamcrest.MatcherAssert; import org.junit.Test; import java.nio.ByteBuffer; +import java.nio.ByteOrder; import static org.hamcrest.CoreMatchers.is; @@ -29,4 +31,53 @@ public void twoWordsAtLastWordShouldNotBeInBounds() { SegmentReader segmentReader = new SegmentReader(byteBuffer, null); MatcherAssert.assertThat(segmentReader.isInBounds(7, 2), is(false)); } + + @Test + public void validSegmentWithNegativeOffsetShouldBeInBounds() { + int refOffset; + long ref; + int refTarget; + int dataSizeWords; + int wordSize; + + /* + Binary data: + echo -n + 'FAAAAAEAAQDJqtK2cBpJhZ2LUEVMkYblyarStnAaSYWdi1BFTJGG4e3///8CAQAAAgAAAAAAAAD0////AAABAA==' + | base64 -d + + Verify it is valid with: + capnp decode --flat dksdk_std_schema.capnp GenericReturn + where the .capnp comes from + https://gitlab.com/diskuv/dksdk-schema/-/blob/afbf9564a60f2670f6b9dfb3c423fc55dd4c3013/src/dksdk_std_schema.capnp + */ + ByteBuffer byteBuffer = ByteBuffer.wrap(new byte[] { + (byte)0x14, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x01, (byte)0x00, (byte)0x01, (byte)0x00, + (byte)0xC9, (byte)0xAA, (byte)0xD2, (byte)0xB6, (byte)0x70, (byte)0x1A, (byte)0x49, (byte)0x85, + (byte)0x9D, (byte)0x8B, (byte)0x50, (byte)0x45, (byte)0x4C, (byte)0x91, (byte)0x86, (byte)0xE5, + (byte)0xC9, (byte)0xAA, (byte)0xD2, (byte)0xB6, (byte)0x70, (byte)0x1A, (byte)0x49, (byte)0x85, + (byte)0x9D, (byte)0x8B, (byte)0x50, (byte)0x45, (byte)0x4C, (byte)0x91, (byte)0x86, (byte)0xE1, + (byte)0xED, (byte)0xFF, (byte)0xFF, (byte)0xFF, (byte)0x02, (byte)0x01, (byte)0x00, (byte)0x00, + (byte)0x02, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, + (byte)0xF4, (byte)0xFF, (byte)0xFF, (byte)0xFF, (byte)0x00, (byte)0x00, (byte)0x01, (byte)0x00 + }).order(ByteOrder.LITTLE_ENDIAN); + SegmentReader segment = new SegmentReader(byteBuffer, null); + + /* Read root Struct: GenericReturn. */ + refOffset = 0; /* At the root STRUCT POINTER */ + ref = segment.get(refOffset); + refTarget = WirePointer.target(refOffset, ref); + dataSizeWords = StructPointer.dataSize(ref); + wordSize = dataSizeWords + StructPointer.ptrCount(ref); + MatcherAssert.assertThat(segment.isInBounds(refTarget, wordSize), is(true)); + + /* Read inner Struct: ComObject. */ + refOffset = refTarget + dataSizeWords; /* At the inner STRUCT POINTER */ + ref = segment.get(refOffset); + refTarget = WirePointer.target(refOffset, ref); + dataSizeWords = StructPointer.dataSize(ref); + wordSize = dataSizeWords + StructPointer.ptrCount(ref); + MatcherAssert.assertThat(segment.isInBounds(refTarget, wordSize), is(true)); + } + } \ No newline at end of file