Skip to content

Commit 38ab83c

Browse files
mtopolnikclaude
andauthored
fix(qwp): accept empty arrays in the egress result decoder (#50)
* fix(qwp): accept empty arrays in the egress result decoder QuestDB emits a non-null empty array (cardinality 0) on QWP egress inline with a 0-length dimension, distinct from a NULL array (carried in the null bitmap). parseArrayColumn rejected dl < 1, so reading an empty array back failed with "ARRAY dim N must be >= 1". Accept dl >= 0, keeping nDims >= 1 and a per-dimension cap so a 0 in one dimension cannot slip an arbitrary value past the element-count cap. Flip the zero-dim hardening case to assert the empty array decodes while a hostile oversized dim is still rejected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Cap ARRAY dim at server's DIM_MAX_LEN The egress decoder bounded each array dimension by MAX_ARRAY_ELEMENTS, the total element-count cap. That value is 128 below DIM_MAX_LEN ((1 << 28) - 1), the server's per-dimension ceiling. For an empty array the 0-length dimension drives the element-count product to 0, so a sibling dimension is bounded only by the per-dimension check. The old cap therefore rejected empty arrays carrying a dimension above MAX_ARRAY_ELEMENTS but within DIM_MAX_LEN -- shapes the server can build and emit, since it checks each dimension against DIM_MAX_LEN at construction and applies no per-dimension cap when encoding egress. Add MAX_ARRAY_DIM_LEN, mirroring the server's DIM_MAX_LEN, and bound each dimension by it. The element-count product guard is unchanged, so the payload still fits the int-addressed buffer and the product stays within long range. A boundary test pins acceptance at DIM_MAX_LEN and rejection one element past it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 2f4d7c7 commit 38ab83c

2 files changed

Lines changed: 76 additions & 20 deletions

File tree

core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpResultBatchDecoder.java

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,16 @@ public class QwpResultBatchDecoder implements QuietCloseable {
5353

5454
private static final int CONN_DICT_INITIAL_BYTES = 4096;
5555
private static final int CONN_DICT_INITIAL_ENTRIES = 512;
56+
/**
57+
* Hard cap on a single ARRAY dimension's length, mirroring the server's
58+
* {@code ArrayView.DIM_MAX_LEN}. It is the largest length the server can
59+
* place on any one dimension when it builds an array, so the decoder accepts
60+
* every dimension up to it. The cap bounds each dimension independently of
61+
* {@link #MAX_ARRAY_ELEMENTS}: an empty array carries a 0-length dimension
62+
* that drives the element-count product to 0, leaving the product guard
63+
* unable to bound a sibling dimension -- this per-dimension cap bounds it.
64+
*/
65+
private static final int MAX_ARRAY_DIM_LEN = (1 << 28) - 1;
5666
/**
5767
* Hard cap on per-row ARRAY element count. 8 bytes per element x this ~ 256 MB max payload,
5868
* which fits in {@code int} once {@code rowEnd - p} is computed. A malicious or buggy
@@ -605,16 +615,17 @@ private long parseArrayColumn(QwpColumnLayout layout, int rowCount, long p, long
605615
long elements = 1;
606616
for (int d = 0; d < nDims; d++) {
607617
int dl = Unsafe.getUnsafe().getInt(p + 1 + 4L * d);
608-
// Require dl >= 1 in every dimension. A dl of 0 in any
609-
// position would zero out {@code elements} and short-circuit
610-
// the {@code MAX_ARRAY_ELEMENTS} cap for the rest of the
611-
// loop, letting subsequent dimensions hold arbitrary values
612-
// unchecked. The encoder side never emits dl == 0 (see
613-
// {@code ColumnType} which asserts nDims >= 1 and treats
614-
// every dimension symmetrically), so reject the
615-
// wire-format inconsistency outright.
616-
if (dl < 1) {
617-
throw new QwpDecodeException("ARRAY dim " + d + " must be >= 1: " + dl);
618+
// A 0-length dimension is a valid empty array (cardinality 0),
619+
// distinct from a NULL array (which the null bitmap carries). A
620+
// dimension is a non-negative int up to MAX_ARRAY_DIM_LEN, the
621+
// same per-dimension ceiling the server applies when it builds an
622+
// array, so every shape the server can emit decodes here. The cap
623+
// also bounds an empty array's other dimensions: a single 0-length
624+
// dimension collapses {@code elements} to 0, leaving the product
625+
// guard below unable to catch an oversized sibling.
626+
if (dl < 0 || dl > MAX_ARRAY_DIM_LEN) {
627+
throw new QwpDecodeException("ARRAY dim " + d + " out of range [0, "
628+
+ MAX_ARRAY_DIM_LEN + "]: " + dl);
618629
}
619630
elements *= dl;
620631
if (elements > MAX_ARRAY_ELEMENTS) {

core/src/test/java/io/questdb/client/test/cutlass/qwp/client/QwpResultBatchDecoderHardeningTest.java

Lines changed: 55 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -52,27 +52,72 @@
5252
public class QwpResultBatchDecoderHardeningTest {
5353

5454
/**
55-
* Regression: an ARRAY column row whose dimension list contains a zero
56-
* must be rejected. Without the fix the multiplicative cap is
57-
* short-circuited the moment {@code elements} hits zero, so subsequent
58-
* dimensions can hold any value (including {@code Integer.MAX_VALUE})
59-
* without the decoder ever firing the {@code MAX_ARRAY_ELEMENTS}
60-
* guard. The encoder side never produces dl == 0 -- the existing
61-
* nDims >= 1 check at the start of {@code parseArrayColumn} confirms
62-
* the design intent that arrays are non-empty in every dimension.
55+
* The per-dimension bound matches the server's {@code DIM_MAX_LEN}
56+
* ({@code (1 << 28) - 1}): an empty array carrying a dimension exactly at
57+
* that ceiling decodes, while one element past it is rejected. Pins the
58+
* boundary so the client accepts every shape the server can emit and no
59+
* more. The 0-length sibling keeps the element-count product at 0, so this
60+
* exercises the per-dimension cap rather than the product cap.
6361
*/
6462
@Test
65-
public void testArrayDimZeroIsRejected() throws Exception {
63+
public void testArrayDimAtServerLimitBoundary() throws Exception {
64+
final int dimMaxLen = (1 << 28) - 1;
6665
TestUtils.assertMemoryLeak(() -> {
6766
QwpResultBatchDecoder decoder = new QwpResultBatchDecoder();
6867
QwpBatchBuffer buffer = new QwpBatchBuffer(256);
6968
long staging = Unsafe.malloc(256, MemoryTag.NATIVE_DEFAULT);
7069
try {
70+
// {0, DIM_MAX_LEN}: empty array (product 0) with a sibling at the
71+
// server's per-dimension ceiling -- decodes without error.
72+
int len = writeArrayResultBatchWithDims(staging, new int[]{0, dimMaxLen});
73+
buffer.copyFromPayload(staging, len);
74+
decoder.decode(buffer);
75+
76+
// {0, DIM_MAX_LEN + 1}: one past the ceiling -- rejected.
77+
len = writeArrayResultBatchWithDims(staging, new int[]{0, dimMaxLen + 1});
78+
buffer.copyFromPayload(staging, len);
79+
try {
80+
decoder.decode(buffer);
81+
Assert.fail("decoder must reject an ARRAY dim above DIM_MAX_LEN");
82+
} catch (QwpDecodeException expected) {
83+
Assert.assertTrue("error must blame the ARRAY dim: " + expected.getMessage(),
84+
expected.getMessage().contains("ARRAY dim"));
85+
}
86+
} finally {
87+
Unsafe.free(staging, 256, MemoryTag.NATIVE_DEFAULT);
88+
buffer.close();
89+
decoder.close();
90+
}
91+
});
92+
}
93+
94+
/**
95+
* A 0-length dimension is a valid empty array (cardinality 0), distinct
96+
* from a NULL array (carried in the null bitmap), so {@code {0, 5}} must
97+
* decode without error. The original concern -- that a 0 collapses the
98+
* element-count product and lets a sibling dimension smuggle an arbitrary
99+
* value past the {@code MAX_ARRAY_ELEMENTS} cap -- is now handled by a
100+
* per-dimension bound, so {@code {0, Integer.MAX_VALUE}} is still rejected.
101+
*/
102+
@Test
103+
public void testArrayDimZeroIsEmptyArrayButHostileDimRejected() throws Exception {
104+
TestUtils.assertMemoryLeak(() -> {
105+
QwpResultBatchDecoder decoder = new QwpResultBatchDecoder();
106+
QwpBatchBuffer buffer = new QwpBatchBuffer(256);
107+
long staging = Unsafe.malloc(256, MemoryTag.NATIVE_DEFAULT);
108+
try {
109+
// {0, 5}: a legitimate empty array -- decodes without error.
71110
int len = writeArrayResultBatchWithDims(staging, new int[]{0, 5});
72111
buffer.copyFromPayload(staging, len);
112+
decoder.decode(buffer);
113+
114+
// {0, Integer.MAX_VALUE}: the per-dimension cap must still fire
115+
// even though the element-count product collapses to 0.
116+
len = writeArrayResultBatchWithDims(staging, new int[]{0, Integer.MAX_VALUE});
117+
buffer.copyFromPayload(staging, len);
73118
try {
74119
decoder.decode(buffer);
75-
Assert.fail("decoder must reject ARRAY with dim == 0");
120+
Assert.fail("decoder must reject a hostile oversized ARRAY dim");
76121
} catch (QwpDecodeException expected) {
77122
Assert.assertTrue("error must blame the ARRAY dim: " + expected.getMessage(),
78123
expected.getMessage().contains("ARRAY dim"));

0 commit comments

Comments
 (0)