Skip to content

Commit 312c1a6

Browse files
dfa1claude
andcommitted
review: narrow gap catch, guard untrusted pool codes, harden hydrate script
Applied from the PR review: assertStillFails now catches only VortexException plus (narrowly, tied to #215) IndexOutOfBoundsException instead of blanket RuntimeException; pool-validity lookups guard out-of-range codes with VortexException per the untrusted-input rule; the hydrate script rejects a valueless --max-mb, misplaced flags, and an empty slug list (bash 3.2 + set -u would otherwise die cryptically); oracle-abort asymmetry documented on oracleLines. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 88d4031 commit 312c1a6

4 files changed

Lines changed: 47 additions & 7 deletions

File tree

integration/src/test/java/io/github/dfa1/vortex/integration/RaincloudConformanceIntegrationTest.java

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import dev.hardwood.reader.ParquetFileReader;
77
import dev.hardwood.reader.RowReader;
88
import dev.hardwood.schema.ColumnSchema;
9+
import io.github.dfa1.vortex.core.error.VortexException;
910
import io.github.dfa1.vortex.csv.CsvExporter;
1011
import io.github.dfa1.vortex.csv.ExportOptions;
1112
import org.junit.jupiter.api.DynamicTest;
@@ -108,14 +109,16 @@ private static void reportUntriaged(Path vortex, Path parquet) {
108109
}
109110

110111
private static void assertStillFails(Path vortex, Path parquet, String status) {
111-
// Given / When — a decode gap still reproduces when the export itself throws; any
112-
// RuntimeException counts (a gap may surface as IndexOutOfBoundsException before its
113-
// decoder gains proper VortexException bounds guards, e.g. #215). No oracle needed
112+
// Given / When — a decode gap still reproduces when the export itself throws.
113+
// Only VortexException (the contractual untrusted-input failure) and, narrowly,
114+
// IndexOutOfBoundsException count: the latter is itself a bounds-guard bug (#215
115+
// throws it today) and this arm dies with that fix — a blanket RuntimeException
116+
// catch would green unrelated regressions (NPEs, ...). No oracle needed here
114117
// (the oracle may not even read this parquet, e.g. nested columns).
115118
List<String> result;
116119
try {
117120
result = exportVortex(vortex);
118-
} catch (RuntimeException e) {
121+
} catch (VortexException | IndexOutOfBoundsException e) {
119122
return;
120123
} catch (IOException e) {
121124
throw new UncheckedIOException(e);
@@ -145,7 +148,9 @@ private static List<String> exportVortex(Path vortex) throws IOException {
145148

146149
/// Reads the parquet sibling through hardwood; an oracle-side failure (nested
147150
/// columns, unsupported physical type) aborts the slug rather than failing it —
148-
/// it says nothing about vortex-java.
151+
/// it says nothing about vortex-java. Deliberate asymmetry: an `ok` slug whose
152+
/// parquet the oracle cannot read stops being verified (visibly, as skipped) —
153+
/// widen the oracle rather than let unverifiable entries fail the build.
149154
private static List<String> oracleLines(Path parquet) {
150155
StringWriter out = new StringWriter();
151156
try {

reader/src/main/java/io/github/dfa1/vortex/reader/decode/DictEncodingDecoder.java

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,15 +164,15 @@ private static BoolArray rowValidity(DecodeContext ctx, MemorySegment codesBuf,
164164
if (codesCap >= rowCount) {
165165
for (long i = 0; i < rowCount; i++) {
166166
boolean valid = (codesValidity == null || codesValidity.getBoolean(i))
167-
&& poolValidity.getBoolean(readCode(codesBuf, i, codePType));
167+
&& poolValid(poolValidity, readCode(codesBuf, i, codePType));
168168
if (valid) {
169169
setBit(bits, i);
170170
}
171171
}
172172
} else {
173173
for (long i = 0; i < rowCount; i++) {
174174
boolean valid = (codesValidity == null || codesValidity.getBoolean(i))
175-
&& poolValidity.getBoolean(readCode(codesBuf, i % codesCap, codePType));
175+
&& poolValid(poolValidity, readCode(codesBuf, i % codesCap, codePType));
176176
if (valid) {
177177
setBit(bits, i);
178178
}
@@ -181,6 +181,17 @@ private static BoolArray rowValidity(DecodeContext ctx, MemorySegment codesBuf,
181181
return new MaterializedBoolArray(DType.BOOL, rowCount, bits.asReadOnly());
182182
}
183183

184+
/// Untrusted-input guard: a malformed file may carry codes outside the pool, and the
185+
/// validity bitmap lookup must fail as [VortexException], never as a raw JDK
186+
/// IndexOutOfBoundsException.
187+
private static boolean poolValid(BoolArray poolValidity, long code) {
188+
if (code >= poolValidity.length()) {
189+
throw new VortexException(EncodingId.VORTEX_DICT,
190+
"code " + code + " out of range for pool validity of length " + poolValidity.length());
191+
}
192+
return poolValidity.getBoolean(code);
193+
}
194+
184195
private static long readCode(MemorySegment codes, long i, PType codePType) {
185196
return switch (codePType) {
186197
case U8 -> Byte.toUnsignedLong(codes.get(ValueLayout.JAVA_BYTE, i));

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,13 @@ private static BoolArray gatherRowValidity(Array codes, BoolArray codesValidity,
185185
default -> throw new VortexException(EncodingId.VORTEX_DICT,
186186
"layout: invalid codes type: " + codes.getClass().getSimpleName());
187187
};
188+
// Untrusted-input guard: codes outside the pool must fail as VortexException,
189+
// never as a raw JDK IndexOutOfBoundsException.
190+
if (code >= poolValidity.length()) {
191+
throw new VortexException(EncodingId.VORTEX_DICT,
192+
"layout: code " + code + " out of range for pool validity of length "
193+
+ poolValidity.length());
194+
}
188195
boolean valid = (codesValidity == null || codesValidity.getBoolean(i))
189196
&& poolValidity.getBoolean(code);
190197
if (valid) {

scripts/hydrate-raincloud-corpus.sh

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,20 @@ MATRIX="$REPO_ROOT/integration/src/test/resources/raincloud/expected-status.csv"
2929

3030
MAX_MB=200
3131
if [[ "${1:-}" == "--max-mb" ]]; then
32+
if [[ -z "${2:-}" ]]; then
33+
echo "error: --max-mb needs a value" >&2
34+
exit 2
35+
fi
3236
MAX_MB="$2"
3337
shift 2
3438
fi
39+
# after the flag: any remaining --max-mb is misplaced, not a slug
40+
for arg in "$@"; do
41+
if [[ "$arg" == --* ]]; then
42+
echo "error: flags must come before slugs: $arg" >&2
43+
exit 2
44+
fi
45+
done
3546

3647
if [[ ! -d "$VENV" ]]; then
3748
echo "creating venv for raincloud@$RAINCLOUD_TAG"
@@ -49,6 +60,12 @@ else
4960
done < <(grep -v '^#' "$MATRIX" | cut -d, -f1)
5061
fi
5162

63+
# bash 3.2 + set -u: expanding an empty array is an unbound-variable error
64+
if [ ${#SLUGS[@]} -eq 0 ]; then
65+
echo "error: no slugs to hydrate (empty matrix?)" >&2
66+
exit 2
67+
fi
68+
5269
mkdir -p "$(dirname "$MANIFEST")"
5370
printf '%s\n' "${SLUGS[@]}" | MAX_MB="$MAX_MB" MANIFEST="$MANIFEST" "$VENV/bin/python" - <<'PY'
5471
import json

0 commit comments

Comments
 (0)