From 66b226f51d15ed1c5ad4a724d7285f16991ceb67 Mon Sep 17 00:00:00 2001 From: Marko Topolnik Date: Wed, 17 Jun 2026 12:45:03 +0200 Subject: [PATCH 1/2] 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) --- .../qwp/client/QwpResultBatchDecoder.java | 20 +++++++-------- .../QwpResultBatchDecoderHardeningTest.java | 25 +++++++++++-------- 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpResultBatchDecoder.java b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpResultBatchDecoder.java index f1c8d706..e641b5ca 100644 --- a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpResultBatchDecoder.java +++ b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpResultBatchDecoder.java @@ -605,16 +605,16 @@ private long parseArrayColumn(QwpColumnLayout layout, int rowCount, long p, long long elements = 1; for (int d = 0; d < nDims; d++) { int dl = Unsafe.getUnsafe().getInt(p + 1 + 4L * d); - // Require dl >= 1 in every dimension. A dl of 0 in any - // position would zero out {@code elements} and short-circuit - // the {@code MAX_ARRAY_ELEMENTS} cap for the rest of the - // loop, letting subsequent dimensions hold arbitrary values - // unchecked. The encoder side never emits dl == 0 (see - // {@code ColumnType} which asserts nDims >= 1 and treats - // every dimension symmetrically), so reject the - // wire-format inconsistency outright. - if (dl < 1) { - throw new QwpDecodeException("ARRAY dim " + d + " must be >= 1: " + dl); + // A 0-length dimension is a valid empty array (cardinality 0), + // distinct from a NULL array (which the null bitmap carries). + // Reject only a negative length, and bound each dimension + // independently: a single 0 collapses {@code elements} to 0, + // so without a per-dimension cap a 0 in one dimension would let + // another hold an arbitrary value the product check below could + // no longer catch. Matches the server's QwpArrayColumnCursor. + if (dl < 0 || dl > MAX_ARRAY_ELEMENTS) { + throw new QwpDecodeException("ARRAY dim " + d + " out of range [0, " + + MAX_ARRAY_ELEMENTS + "]: " + dl); } elements *= dl; if (elements > MAX_ARRAY_ELEMENTS) { diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/QwpResultBatchDecoderHardeningTest.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/QwpResultBatchDecoderHardeningTest.java index 073570b1..47e31f8c 100644 --- a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/QwpResultBatchDecoderHardeningTest.java +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/QwpResultBatchDecoderHardeningTest.java @@ -52,27 +52,32 @@ public class QwpResultBatchDecoderHardeningTest { /** - * Regression: an ARRAY column row whose dimension list contains a zero - * must be rejected. Without the fix the multiplicative cap is - * short-circuited the moment {@code elements} hits zero, so subsequent - * dimensions can hold any value (including {@code Integer.MAX_VALUE}) - * without the decoder ever firing the {@code MAX_ARRAY_ELEMENTS} - * guard. The encoder side never produces dl == 0 -- the existing - * nDims >= 1 check at the start of {@code parseArrayColumn} confirms - * the design intent that arrays are non-empty in every dimension. + * A 0-length dimension is a valid empty array (cardinality 0), distinct + * from a NULL array (carried in the null bitmap), so {@code {0, 5}} must + * decode without error. The original concern -- that a 0 collapses the + * element-count product and lets a sibling dimension smuggle an arbitrary + * value past the {@code MAX_ARRAY_ELEMENTS} cap -- is now handled by a + * per-dimension bound, so {@code {0, Integer.MAX_VALUE}} is still rejected. */ @Test - public void testArrayDimZeroIsRejected() throws Exception { + public void testArrayDimZeroIsEmptyArrayButHostileDimRejected() throws Exception { TestUtils.assertMemoryLeak(() -> { QwpResultBatchDecoder decoder = new QwpResultBatchDecoder(); QwpBatchBuffer buffer = new QwpBatchBuffer(256); long staging = Unsafe.malloc(256, MemoryTag.NATIVE_DEFAULT); try { + // {0, 5}: a legitimate empty array -- decodes without error. int len = writeArrayResultBatchWithDims(staging, new int[]{0, 5}); buffer.copyFromPayload(staging, len); + decoder.decode(buffer); + + // {0, Integer.MAX_VALUE}: the per-dimension cap must still fire + // even though the element-count product collapses to 0. + len = writeArrayResultBatchWithDims(staging, new int[]{0, Integer.MAX_VALUE}); + buffer.copyFromPayload(staging, len); try { decoder.decode(buffer); - Assert.fail("decoder must reject ARRAY with dim == 0"); + Assert.fail("decoder must reject a hostile oversized ARRAY dim"); } catch (QwpDecodeException expected) { Assert.assertTrue("error must blame the ARRAY dim: " + expected.getMessage(), expected.getMessage().contains("ARRAY dim")); From e8fa51faf870d5b9becd3ab16108c63759b04034 Mon Sep 17 00:00:00 2001 From: Marko Topolnik Date: Wed, 17 Jun 2026 15:44:08 +0200 Subject: [PATCH 2/2] 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) --- .../qwp/client/QwpResultBatchDecoder.java | 27 +++++++++---- .../QwpResultBatchDecoderHardeningTest.java | 40 +++++++++++++++++++ 2 files changed, 59 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpResultBatchDecoder.java b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpResultBatchDecoder.java index e641b5ca..4359e26f 100644 --- a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpResultBatchDecoder.java +++ b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpResultBatchDecoder.java @@ -53,6 +53,16 @@ public class QwpResultBatchDecoder implements QuietCloseable { private static final int CONN_DICT_INITIAL_BYTES = 4096; private static final int CONN_DICT_INITIAL_ENTRIES = 512; + /** + * Hard cap on a single ARRAY dimension's length, mirroring the server's + * {@code ArrayView.DIM_MAX_LEN}. It is the largest length the server can + * place on any one dimension when it builds an array, so the decoder accepts + * every dimension up to it. The cap bounds each dimension independently of + * {@link #MAX_ARRAY_ELEMENTS}: an empty array carries a 0-length dimension + * that drives the element-count product to 0, leaving the product guard + * unable to bound a sibling dimension -- this per-dimension cap bounds it. + */ + private static final int MAX_ARRAY_DIM_LEN = (1 << 28) - 1; /** * Hard cap on per-row ARRAY element count. 8 bytes per element x this ~ 256 MB max payload, * which fits in {@code int} once {@code rowEnd - p} is computed. A malicious or buggy @@ -606,15 +616,16 @@ private long parseArrayColumn(QwpColumnLayout layout, int rowCount, long p, long for (int d = 0; d < nDims; d++) { int dl = Unsafe.getUnsafe().getInt(p + 1 + 4L * d); // A 0-length dimension is a valid empty array (cardinality 0), - // distinct from a NULL array (which the null bitmap carries). - // Reject only a negative length, and bound each dimension - // independently: a single 0 collapses {@code elements} to 0, - // so without a per-dimension cap a 0 in one dimension would let - // another hold an arbitrary value the product check below could - // no longer catch. Matches the server's QwpArrayColumnCursor. - if (dl < 0 || dl > MAX_ARRAY_ELEMENTS) { + // distinct from a NULL array (which the null bitmap carries). A + // dimension is a non-negative int up to MAX_ARRAY_DIM_LEN, the + // same per-dimension ceiling the server applies when it builds an + // array, so every shape the server can emit decodes here. The cap + // also bounds an empty array's other dimensions: a single 0-length + // dimension collapses {@code elements} to 0, leaving the product + // guard below unable to catch an oversized sibling. + if (dl < 0 || dl > MAX_ARRAY_DIM_LEN) { throw new QwpDecodeException("ARRAY dim " + d + " out of range [0, " - + MAX_ARRAY_ELEMENTS + "]: " + dl); + + MAX_ARRAY_DIM_LEN + "]: " + dl); } elements *= dl; if (elements > MAX_ARRAY_ELEMENTS) { diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/QwpResultBatchDecoderHardeningTest.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/QwpResultBatchDecoderHardeningTest.java index 47e31f8c..b1f158f1 100644 --- a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/QwpResultBatchDecoderHardeningTest.java +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/QwpResultBatchDecoderHardeningTest.java @@ -51,6 +51,46 @@ */ public class QwpResultBatchDecoderHardeningTest { + /** + * The per-dimension bound matches the server's {@code DIM_MAX_LEN} + * ({@code (1 << 28) - 1}): an empty array carrying a dimension exactly at + * that ceiling decodes, while one element past it is rejected. Pins the + * boundary so the client accepts every shape the server can emit and no + * more. The 0-length sibling keeps the element-count product at 0, so this + * exercises the per-dimension cap rather than the product cap. + */ + @Test + public void testArrayDimAtServerLimitBoundary() throws Exception { + final int dimMaxLen = (1 << 28) - 1; + TestUtils.assertMemoryLeak(() -> { + QwpResultBatchDecoder decoder = new QwpResultBatchDecoder(); + QwpBatchBuffer buffer = new QwpBatchBuffer(256); + long staging = Unsafe.malloc(256, MemoryTag.NATIVE_DEFAULT); + try { + // {0, DIM_MAX_LEN}: empty array (product 0) with a sibling at the + // server's per-dimension ceiling -- decodes without error. + int len = writeArrayResultBatchWithDims(staging, new int[]{0, dimMaxLen}); + buffer.copyFromPayload(staging, len); + decoder.decode(buffer); + + // {0, DIM_MAX_LEN + 1}: one past the ceiling -- rejected. + len = writeArrayResultBatchWithDims(staging, new int[]{0, dimMaxLen + 1}); + buffer.copyFromPayload(staging, len); + try { + decoder.decode(buffer); + Assert.fail("decoder must reject an ARRAY dim above DIM_MAX_LEN"); + } catch (QwpDecodeException expected) { + Assert.assertTrue("error must blame the ARRAY dim: " + expected.getMessage(), + expected.getMessage().contains("ARRAY dim")); + } + } finally { + Unsafe.free(staging, 256, MemoryTag.NATIVE_DEFAULT); + buffer.close(); + decoder.close(); + } + }); + } + /** * A 0-length dimension is a valid empty array (cardinality 0), distinct * from a NULL array (carried in the null bitmap), so {@code {0, 5}} must