Skip to content

Commit dd196f1

Browse files
dfa1claude
andcommitted
fix(reader): honor custom decoders under chunked; harden SPI edges
Review findings on the LayoutDecoder SPI: - ChunkedLayoutDecoder decoded its leaves via a direct static FlatLayoutDecoder call, silently bypassing the registry — a custom decoder registered for a leaf id was not honored under a chunked parent, making the SPI partially decorative. Leaves now route through ctx.decodeChild; the end-to-end test asserts the flat delegator itself fires during a real scan. Integration oracle confirms identical behavior for built-ins (dict leaves under chunked included). - ScanLayoutContext.segmentSpec and DictLayoutDecoder child access now guard malformed indexes/arity with VortexException instead of leaking IndexOutOfBoundsException from untrusted input. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent fc488d0 commit dd196f1

5 files changed

Lines changed: 29 additions & 25 deletions

File tree

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -824,7 +824,13 @@ public Array decodeFlatSegment(SegmentSpec spec, DType dtype, long rowCount) {
824824

825825
@Override
826826
public SegmentSpec segmentSpec(int index) {
827-
return file.footer().segmentSpecs().get(index);
827+
List<SegmentSpec> specs = file.footer().segmentSpecs();
828+
if (index < 0 || index >= specs.size()) {
829+
// Untrusted input: a malformed flat layout may carry any segment index.
830+
throw new VortexException("segment index " + index
831+
+ " out of bounds (segmentSpecs.size=" + specs.size() + ")");
832+
}
833+
return specs.get(index);
828834
}
829835
}
830836

reader/src/main/java/io/github/dfa1/vortex/reader/layout/ChunkedLayoutDecoder.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,15 @@ private static Array decodeChunkedLayout(LayoutDecodeContext ctx, List<Layout> f
7070
throw new VortexException(EncodingId.VORTEX_CHUNKED, "no flat children");
7171
}
7272
if (flats.size() == 1) {
73-
return FlatLayoutDecoder.decodeFlat(ctx, flats.getFirst(), dtype);
73+
return ctx.decodeChild(flats.getFirst(), dtype);
7474
}
7575
// ADR 0012: every primitive ptype gets the zero-copy ChunkedXxxArray shape.
7676
// The concat path is gone.
7777
var chunkArrays = new ArrayList<Array>(flats.size());
7878
for (Layout flat : flats) {
79-
chunkArrays.add(FlatLayoutDecoder.decodeFlat(ctx, flat, dtype));
79+
// Registry dispatch, not a direct decodeFlat call — a custom decoder registered for
80+
// a leaf's layout id must be honored under a chunked parent too.
81+
chunkArrays.add(ctx.decodeChild(flat, dtype));
8082
}
8183
if (dtype instanceof DType.Bool) {
8284
return ChunkedBoolArray.of(dtype, totalRows, chunkArrays);

reader/src/main/java/io/github/dfa1/vortex/reader/layout/DictLayoutDecoder.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,11 @@ public Array decode(LayoutDecodeContext ctx, Layout dictLayout, DType dtype) {
4545
PType codesPType = readDictLayoutCodesPType(rawMeta);
4646

4747
// child[0] = values layout; child[1] = codes layout
48+
if (dictLayout.children().size() < 2) {
49+
// Untrusted input: a malformed dict layout may carry any child count.
50+
throw new VortexException(EncodingId.VORTEX_DICT,
51+
"expected 2 children (values, codes), got " + dictLayout.children().size());
52+
}
4853
Layout valuesLayout = dictLayout.children().get(0);
4954
Layout codesLayout = dictLayout.children().get(1);
5055
long n = codesLayout.rowCount();

reader/src/main/java/io/github/dfa1/vortex/reader/layout/FlatLayoutDecoder.java

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,23 +17,11 @@ public LayoutId layoutId() {
1717

1818
@Override
1919
public Array decode(LayoutDecodeContext ctx, Layout layout, DType dtype) {
20-
return decodeFlat(ctx, layout, dtype);
21-
}
22-
23-
/// Decodes one flat leaf: resolves its single segment index to a [SegmentSpec] and delegates
24-
/// to the file handle. Shared with [ChunkedLayoutDecoder], which decodes each of its collected
25-
/// leaves through exactly this path (not registry dispatch), preserving the original behavior.
26-
///
27-
/// @param ctx the decode context
28-
/// @param flat the flat layout node
29-
/// @param dtype logical type of the decoded array
30-
/// @return the decoded [Array]
31-
static Array decodeFlat(LayoutDecodeContext ctx, Layout flat, DType dtype) {
32-
if (flat.segments().isEmpty()) {
20+
if (layout.segments().isEmpty()) {
3321
throw new VortexException("no segments");
3422
}
35-
int segIdx = flat.segments().getFirst();
23+
int segIdx = layout.segments().getFirst();
3624
SegmentSpec spec = ctx.segmentSpec(segIdx);
37-
return ctx.decodeFlatSegment(spec, dtype, flat.rowCount());
25+
return ctx.decodeFlatSegment(spec, dtype, layout.rowCount());
3826
}
3927
}

reader/src/test/java/io/github/dfa1/vortex/reader/layout/LayoutRegistryTest.java

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,13 @@ void openWithCustomRegistry_reachesTheCustomDecoderDuringScan() throws URISyntax
9494
// Given — a LayoutRegistry whose built-ins are wrapped in counting delegators, so any decode
9595
// dispatch during a real scan is observable. This proves the open(path, reg, layoutReg)
9696
// overload threads the custom registry all the way into ScanIterator's decode path.
97-
AtomicInteger dispatches = new AtomicInteger();
97+
AtomicInteger flatDispatches = new AtomicInteger();
98+
AtomicInteger otherDispatches = new AtomicInteger();
9899
LayoutRegistry custom = LayoutRegistry.builder()
99-
.register(new CountingLayoutDecoder(new FlatLayoutDecoder(), dispatches))
100-
.register(new CountingLayoutDecoder(new ChunkedLayoutDecoder(), dispatches))
101-
.register(new CountingLayoutDecoder(new ZonedLayoutDecoder(), dispatches))
102-
.register(new CountingLayoutDecoder(new DictLayoutDecoder(), dispatches))
100+
.register(new CountingLayoutDecoder(new FlatLayoutDecoder(), flatDispatches))
101+
.register(new CountingLayoutDecoder(new ChunkedLayoutDecoder(), otherDispatches))
102+
.register(new CountingLayoutDecoder(new ZonedLayoutDecoder(), otherDispatches))
103+
.register(new CountingLayoutDecoder(new DictLayoutDecoder(), otherDispatches))
103104
.build();
104105
Path fixture = fixtureFile("primitives.vortex");
105106

@@ -114,9 +115,11 @@ void openWithCustomRegistry_reachesTheCustomDecoderDuringScan() throws URISyntax
114115
}
115116
}
116117

117-
// Then — the scan produced rows and routed every layout decode through the custom registry
118+
// Then — the scan produced rows, and the FLAT delegator itself fired: leaf decode under a
119+
// container layout goes through registry dispatch too (a custom flat decoder is honored),
120+
// not through a direct built-in call that would bypass the registry.
118121
assertThat(rows).isPositive();
119-
assertThat(dispatches.get()).isPositive();
122+
assertThat(flatDispatches.get()).isPositive();
120123
}
121124

122125
// ── helpers ───────────────────────────────────────────────────────────────

0 commit comments

Comments
 (0)