Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix SegmentReader bugs #134

Merged
merged 2 commits into from
Sep 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
4 changes: 4 additions & 0 deletions runtime/src/main/java/org/capnproto/FarPointer.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
3 changes: 3 additions & 0 deletions runtime/src/main/java/org/capnproto/ListPointer.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
9 changes: 5 additions & 4 deletions runtime/src/main/java/org/capnproto/SegmentReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,10 @@ public final long get(int index) {
* Verify that the `size`-long (in words) range starting at word index
* `start` is within bounds.
*/
public final boolean isInBounds(int start, int size) {
if (start < 0 || size < 0) return false;
long sizeInWords = (long) size * Constants.BYTES_PER_WORD;
return start + sizeInWords <= this.buffer.capacity();
public final boolean isInBounds(int startInWords, int sizeInWords) {
if (startInWords < 0 || sizeInWords < 0) return false;
long startInBytes = (long) startInWords * Constants.BYTES_PER_WORD;
long sizeInBytes = (long) sizeInWords * Constants.BYTES_PER_WORD;
return startInBytes + sizeInBytes <= this.buffer.capacity();
}
}
3 changes: 3 additions & 0 deletions runtime/src/main/java/org/capnproto/StructPointer.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
4 changes: 3 additions & 1 deletion runtime/src/main/java/org/capnproto/WirePointer.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
65 changes: 65 additions & 0 deletions runtime/src/test/java/org/capnproto/SegmentReaderTest.java
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -15,4 +17,67 @@ public void in_boundsCalculationShouldNotOverflow() {
SegmentReader segmentReader = new SegmentReader(byteBuffer, null);
MatcherAssert.assertThat(segmentReader.isInBounds(0, Integer.MAX_VALUE), is(false));
}

@Test
public void oneWordAtLastWordShouldBeInBounds() {
ByteBuffer byteBuffer = ByteBuffer.allocate(64);
SegmentReader segmentReader = new SegmentReader(byteBuffer, null);
MatcherAssert.assertThat(segmentReader.isInBounds(7, 1), is(true));
}

@Test
public void twoWordsAtLastWordShouldNotBeInBounds() {
ByteBuffer byteBuffer = ByteBuffer.allocate(64);
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));
}

}
Loading