Skip to content

Commit 5d5fcc4

Browse files
dfa1claude
andcommitted
refactor(reader): drop dead length < 0 blob check; reuse PTypeIO LE layouts
Two follow-ups: - checkBlobBounds: drop the `length < 0` clause. Every caller passes a u32-masked PostscriptSegment.length() (`bb.getInt(..) & 0xFFFFFFFFL`), always >= 0, so the branch is unreachable — the equivalent mutant pitest flagged. Removing it takes the reader file-structure classes to a 100% mutation score (the only non-killed mutation is a depth-recursion timeout, which pitest counts as detected). The `offset < 0` check stays — offset is u64 and can read back negative. - Tests: reuse the public PTypeIO.LE_INT / LE_SHORT layout constants instead of re-declaring them locally, and inline the single-use I32 dtype (no shared DType constant exists — it is duplicated per test file). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent c9243f9 commit 5d5fcc4

3 files changed

Lines changed: 10 additions & 17 deletions

File tree

reader/src/main/java/io/github/dfa1/vortex/reader/PostscriptParser.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,12 @@ static void validateSegmentSpecs(List<SegmentSpec> specs, long fileSize) {
9090
}
9191

9292
private static void checkBlobBounds(String name, long offset, long length, long fileSize) {
93-
// Same overflow-safe range form as IoBounds.checkRange (no redundant `offset > fileSize`
94-
// clause: length >= 0 makes it implied by the final comparison). Keeps the blob-named
95-
// message that checkRange's generic text would lose.
96-
if (offset < 0 || length < 0 || length > fileSize - offset) {
93+
// Overflow-safe containment in [0, fileSize], keeping the blob-named message that
94+
// IoBounds.checkRange's generic text would lose. Two clauses checkRange carries are
95+
// omitted because they are unreachable here: every caller passes a u32-masked
96+
// PostscriptSegment.length() (always >= 0, so no `length < 0` check), which in turn makes
97+
// an `offset > fileSize` check redundant — it is already implied by the final comparison.
98+
if (offset < 0 || length > fileSize - offset) {
9799
throw new VortexException(
98100
"postscript " + name + " blob out of bounds: offset=" + offset
99101
+ " length=" + length + " fileSize=" + fileSize);

reader/src/test/java/io/github/dfa1/vortex/reader/FlatSegmentDecoderDecodeTest.java

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,9 @@
1111

1212
import java.lang.foreign.Arena;
1313
import java.lang.foreign.MemorySegment;
14-
import java.lang.foreign.ValueLayout;
15-
import java.nio.ByteOrder;
1614
import java.util.List;
1715

16+
import static io.github.dfa1.vortex.encoding.PTypeIO.LE_INT;
1817
import static org.assertj.core.api.Assertions.assertThat;
1918

2019
/// Successful flat-segment decode path — complements [FlatSegmentBoundsSecurityTest] (which only
@@ -27,11 +26,6 @@
2726
/// null node and the decode would not produce an `UnknownArray`.
2827
class FlatSegmentDecoderDecodeTest {
2928

30-
private static final ValueLayout.OfInt LE_INT =
31-
ValueLayout.JAVA_INT_UNALIGNED.withOrder(ByteOrder.LITTLE_ENDIAN);
32-
33-
private static final DType DTYPE = new DType.Primitive(PType.I32, false);
34-
3529
@Test
3630
void decode_unknownEncodingWithBufferPadding_returnsUnknownArray() {
3731
ReadRegistry registry = ReadRegistry.builder().allowUnknown().build();
@@ -49,7 +43,8 @@ void decode_unknownEncodingWithBufferPadding_returnsUnknownArray() {
4943
seg.set(LE_INT, padding + fb.length, fb.length);
5044

5145
// When — encoding index 0 maps to an id no decoder handles
52-
Array result = sut.decode(seg, List.of("vortex.nonexistent"), DTYPE, 0, arena);
46+
Array result = sut.decode(seg, List.of("vortex.nonexistent"),
47+
new DType.Primitive(PType.I32, false), 0, arena);
5348

5449
// Then — the allow-unknown path produced an UnknownArray (proves both the +padding
5550
// walk and the UnknownArrayNode fallback ran)

reader/src/test/java/io/github/dfa1/vortex/reader/TrailerLengthBoundaryTest.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,8 @@
55
import org.junit.jupiter.api.Test;
66

77
import java.lang.foreign.MemorySegment;
8-
import java.lang.foreign.ValueLayout;
9-
import java.nio.ByteOrder;
108

9+
import static io.github.dfa1.vortex.encoding.PTypeIO.LE_SHORT;
1110
import static org.assertj.core.api.Assertions.assertThat;
1211
import static org.assertj.core.api.Assertions.assertThatCode;
1312
import static org.assertj.core.api.Assertions.assertThatThrownBy;
@@ -18,9 +17,6 @@
1817
/// exact-fill case — this pins that edge.
1918
class TrailerLengthBoundaryTest {
2019

21-
private static final ValueLayout.OfShort LE_SHORT =
22-
ValueLayout.JAVA_SHORT_UNALIGNED.withOrder(ByteOrder.LITTLE_ENDIAN);
23-
2420
/// Builds a well-formed 8-byte trailer (`version | postscriptLen | magic`) with the given
2521
/// postscript length, so only the length-vs-body check is under test.
2622
private static MemorySegment trailer(int postscriptLen) {

0 commit comments

Comments
 (0)