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..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 @@ -605,16 +615,17 @@ 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). 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_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 073570b1..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 @@ -52,27 +52,72 @@ 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. + * 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 testArrayDimZeroIsRejected() throws Exception { + 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 + * 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 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"));