Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down
Loading