Skip to content

Commit 8f16aba

Browse files
committed
numpy_parser: reject undersized vector payloads too, add test coverage
The memcpy fast-path guard in unpack_row() only rejected oversized payloads (buf.size > arr.stride), which prevents the buffer-overflow case but let undersized payloads slip through, leaving the remainder of the row filled with uninitialized/stale heap bytes instead of raising. Tighten the check to buf.size != arr.stride so both directions are rejected, and update the error message accordingly, including the column name for easier debugging. Add tests covering both the oversized and undersized payload cases, since neither had test coverage before. Also revert the arrays = [] / .append() -> [None]*n / index-assign change in make_arrays(), which was functionally identical diff noise unrelated to the VectorType/2D-array feature. Restore ShortType (smallint) to _cqltype_to_numpy: an earlier fix on this PR removed it from that table entirely to stop the VectorType fast-path from assuming a fixed 2-byte stride for vint-prefixed vector elements, but the table is shared with the plain scalar top-level column dispatch in make_array(), where smallint genuinely *is* fixed-width (no length prefix). Removing the entry regressed scalar smallint columns onto the slow object-array fallback. Fix: keep ShortType in the shared table, and instead guard the VectorType.subtype call site with `subtype.serial_size() is not None` (the same check VectorType.serialize()/deserialize() use) before consulting the table, so only the vector-specific dispatch excludes non-fixed-width subtypes. This also protects any subtype added to the table in the future. Add tests for the restored scalar smallint fast path (both make_array() directly and an end-to-end parse_rows() round-trip). Also fix tests/unit/test_numpy_parser.py's HAVE_NUMPY gating: a bare `except ImportError` around the numpy_parser/bytesio/etc. imports could not distinguish "numpy genuinely not installed" from "the numpy_parser Cython extension is broken despite numpy being present", silently skipping the whole test module in both cases. Gate instead on cassandra.cython_deps.HAVE_CYTHON and HAVE_NUMPY (plus the VERIFY_CYTHON override), matching the `numpytest` convention in tests/unit/cython/utils.py, and import unconditionally once those are satisfied so a broken extension surfaces as a real test failure.
1 parent 349014d commit 8f16aba

2 files changed

Lines changed: 216 additions & 28 deletions

File tree

cassandra/numpy_parser.pyx

Lines changed: 58 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -67,20 +67,41 @@ _cqltype_to_numpy = {
6767
cqltypes.LongType: np.dtype('>i8'),
6868
cqltypes.CounterColumnType: np.dtype('>i8'),
6969
cqltypes.Int32Type: np.dtype('>i4'),
70+
cqltypes.ShortType: np.dtype('>i2'),
7071
cqltypes.FloatType: np.dtype('>f4'),
7172
cqltypes.DoubleType: np.dtype('>f8'),
7273
}
73-
# Note: ShortType (smallint) is intentionally absent from this table. Unlike
74-
# LongType/Int32Type/FloatType/DoubleType/CounterColumnType, ShortType does
75-
# not override serial_size() in cassandra.cqltypes, so it has no fixed
76-
# per-element width when serialized *inside* a VectorType: the server
77-
# vint-prefixes each smallint element rather than encoding it as a plain
78-
# 2-byte big-endian field. Mapping it here would make the vector fast-path
79-
# assume a fixed 2-byte stride, which does not match the actual wire format
80-
# and causes a crash (see unpack_row's stride check) instead of falling back
81-
# to the safe, slower object-array path used for other subtypes without a
82-
# fixed width (e.g. UTF8Type). See also ByteType (tinyint), which has the
83-
# same issue and is likewise absent from this table.
74+
# This table is consulted from two different call sites in make_array()
75+
# below, and the two call sites do NOT mean the same thing by "fixed width":
76+
#
77+
# - Scalar top-level columns (`_cqltype_to_numpy[coltype]`): here ShortType
78+
# (smallint) genuinely is fixed-width on the wire (a plain 2-byte
79+
# big-endian field, no length prefix), so it belongs in this table and
80+
# gets the fast masked-array path, exactly like Int32Type/LongType/etc.
81+
#
82+
# - VectorType.subtype dispatch (`_cqltype_to_numpy[subtype]` in the
83+
# VectorType branch below): Cassandra/Scylla vint-prefixes each element
84+
# *inside* a vector unless the subtype overrides serial_size() with a
85+
# fixed value (see cqltypes.VectorType.serialize/deserialize, which
86+
# branch on exactly this). ShortType (and ByteType/tinyint) do not
87+
# override serial_size() -- it returns None from the _CassandraType
88+
# base -- so inside a vector they are NOT fixed-width, even though
89+
# ShortType *is* fixed-width as a scalar column. Using this table
90+
# directly for a vector subtype without checking serial_size() would
91+
# make the vector fast-path assume a fixed 2-byte stride that does not
92+
# match the actual wire format, causing a crash (see unpack_row's
93+
# stride check) instead of falling back to the safe, slower
94+
# object-array path used for other subtypes without a fixed width
95+
# (e.g. UTF8Type).
96+
#
97+
# Rather than removing ShortType from this shared table (which would also
98+
# silently break the scalar smallint fast path, since both call sites read
99+
# from the same dict), the VectorType call site below guards with
100+
# `subtype.serial_size() is not None` before consulting this table at all.
101+
# That mirrors the check VectorType itself uses to decide its own wire
102+
# encoding, so it is the general, authoritative test -- and it also
103+
# protects any subtype added to this table in the future without requiring
104+
# every caller to separately remember the vector-vs-scalar distinction.
84105

85106
obj_dtype = np.dtype('O')
86107

@@ -123,7 +144,7 @@ def make_arrays(ParseDesc desc, array_size):
123144
(e.g. this can be fed into pandas.DataFrame)
124145
"""
125146
array_descs = np.empty((desc.rowsize,), arrDescDtype)
126-
arrays = [None] * desc.rowsize
147+
arrays = []
127148

128149
for i, coltype in enumerate(desc.coltypes):
129150
arr = make_array(coltype, array_size)
@@ -136,7 +157,7 @@ def make_arrays(ParseDesc desc, array_size):
136157
except AttributeError:
137158
array_descs[i]['mask_ptr'] = 0
138159
array_descs[i]['mask_stride'] = 1
139-
arrays[i] = arr
160+
arrays.append(arr)
140161

141162
return array_descs, arrays
142163

@@ -151,13 +172,26 @@ def make_array(coltype, array_size):
151172
# VectorType - create 2D array (rows x vector_dimension)
152173
vector_size = coltype.vector_size
153174
subtype = coltype.subtype
154-
try:
155-
dtype = _cqltype_to_numpy[subtype]
156-
a = np.ma.empty((array_size, vector_size), dtype=dtype)
157-
a.mask = np.zeros((array_size, vector_size), dtype=bool)
158-
except KeyError:
159-
# Unsupported vector subtype - fall back to object array
160-
a = np.empty((array_size,), dtype=obj_dtype)
175+
# Only use the fixed-width fast path if the subtype actually has a
176+
# fixed per-element wire size *inside a vector*. subtype.serial_size()
177+
# is exactly the check VectorType.serialize()/deserialize() use to
178+
# decide whether elements are plain fixed-width fields or
179+
# vint-length-prefixed (e.g. ShortType/ByteType return None here,
180+
# even though ShortType has its own, unrelated entry in
181+
# _cqltype_to_numpy for the scalar-column case below). Checking this
182+
# first -- rather than only catching KeyError on _cqltype_to_numpy --
183+
# also protects any subtype added to that table in the future.
184+
if subtype.serial_size() is not None:
185+
try:
186+
dtype = _cqltype_to_numpy[subtype]
187+
a = np.ma.empty((array_size, vector_size), dtype=dtype)
188+
a.mask = np.zeros((array_size, vector_size), dtype=bool)
189+
return a
190+
except KeyError:
191+
pass
192+
# Unsupported vector subtype, or one without a fixed per-element
193+
# wire width - fall back to object array.
194+
a = np.empty((array_size,), dtype=obj_dtype)
161195
return a
162196

163197
# Scalar types
@@ -189,10 +223,11 @@ cdef inline int unpack_row(
189223
Py_INCREF(val)
190224
(<PyObject **> arr.buf_ptr)[0] = <PyObject *> val
191225
elif buf.size >= 0:
192-
if buf.size > arr.stride:
226+
if buf.size != arr.stride:
193227
raise ValueError(
194-
"Column %d: received %d bytes but array stride is %d" %
195-
(i, buf.size, arr.stride))
228+
"Column %d (%r): received %d bytes but array stride is %d "
229+
"(payload must exactly match the expected element size)" %
230+
(i, desc.colnames[i], buf.size, arr.stride))
196231
memcpy(<char *> arr.buf_ptr, buf.ptr, buf.size)
197232
else:
198233
memset(<char *>arr.mask_ptr, 1, arr.mask_stride)

tests/unit/test_numpy_parser.py

Lines changed: 158 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,44 @@
1515
import struct
1616
import unittest
1717

18+
from cassandra.cython_deps import HAVE_CYTHON, HAVE_NUMPY
19+
1820
try:
21+
from tests import VERIFY_CYTHON
22+
except ImportError:
23+
VERIFY_CYTHON = False
24+
25+
# cassandra.numpy_parser is a Cython extension (gated on HAVE_CYTHON), not a
26+
# pure-Python module -- HAVE_NUMPY (from cassandra.cython_deps) only tells us
27+
# whether the *numpy package itself* is importable, not whether numpy_parser
28+
# built/imports correctly. A single broad `except ImportError` around all of
29+
# these imports would conflate two very different situations:
30+
# - numpy genuinely not installed (expected in a numpy-less environment;
31+
# should skip cleanly), vs.
32+
# - the Cython extension being broken or missing despite numpy being
33+
# present (an unexpected regression that should FAIL the test run
34+
# rather than hide behind a silently skipped test -- this is exactly
35+
# how a real numpy_parser regression could slip through a green CI run).
36+
#
37+
# So gate on HAVE_CYTHON and HAVE_NUMPY together (the same compound
38+
# condition as the `numpytest` decorator in tests/unit/cython/utils.py,
39+
# including its VERIFY_CYTHON override for CI configurations that require
40+
# Cython support to be present); only skip when Cython and/or numpy support
41+
# is genuinely absent, and otherwise import unconditionally so a broken
42+
# extension surfaces as a real ImportError/test failure.
43+
HAVE_NUMPY_PARSER = (HAVE_CYTHON and HAVE_NUMPY) or VERIFY_CYTHON
44+
45+
if HAVE_NUMPY_PARSER:
1946
import numpy as np
2047
from cassandra.numpy_parser import NumpyParser
2148
from cassandra.bytesio import BytesIOReader
2249
from cassandra.parsing import ParseDesc
2350
from cassandra.deserializers import obj_array, make_deserializers
2451

25-
HAVE_NUMPY = True
26-
except ImportError:
27-
HAVE_NUMPY = False
28-
2952
from cassandra import cqltypes
3053

3154

32-
@unittest.skipUnless(HAVE_NUMPY, "NumPy not available")
55+
@unittest.skipUnless(HAVE_NUMPY_PARSER, "NumPy/Cython extension not available")
3356
class TestNumpyParserVectorType(unittest.TestCase):
3457
"""Tests for VectorType support in NumpyParser"""
3558

@@ -476,6 +499,72 @@ def test_null_vector_data_bytes_are_zeroed(self):
476499
null_row_bytes = arr.data[1].tobytes()
477500
self.assertEqual(null_row_bytes, b"\x00" * len(null_row_bytes))
478501

502+
def test_oversized_vector_payload_raises(self):
503+
"""Test that a vector payload larger than the array stride is rejected.
504+
505+
Guards against a memcpy buffer overflow: unpack_row() must not trust
506+
an oversized buf.size and copy past the end of the preallocated row.
507+
"""
508+
vector_size = 3
509+
vector_type = self._create_vector_type(cqltypes.FloatType, vector_size)
510+
511+
buffer = bytearray()
512+
buffer.extend(struct.pack(">i", 1)) # row count
513+
514+
# Claim 4 floats (16 bytes) worth of payload for a 3-float (12 byte)
515+
# column stride.
516+
buffer.extend(struct.pack(">i", 16))
517+
buffer.extend(struct.pack(">4f", 1.0, 2.0, 3.0, 4.0))
518+
519+
parser = NumpyParser()
520+
reader = BytesIOReader(bytes(buffer))
521+
522+
desc = ParseDesc(
523+
colnames=["vec"],
524+
coltypes=[vector_type],
525+
column_encryption_policy=None,
526+
coldescs=None,
527+
deserializers=obj_array([None]),
528+
protocol_version=5,
529+
)
530+
531+
with self.assertRaisesRegex(ValueError, r"'vec'"):
532+
parser.parse_rows(reader, desc)
533+
534+
def test_undersized_vector_payload_raises(self):
535+
"""Test that a vector payload smaller than the array stride is rejected.
536+
537+
An undersized payload must not be silently accepted: without a
538+
strict equality check, memcpy would only fill part of the row,
539+
leaving the remaining bytes uninitialized/stale (a correctness and
540+
potential info-leak issue), rather than raising.
541+
"""
542+
vector_size = 4
543+
vector_type = self._create_vector_type(cqltypes.FloatType, vector_size)
544+
545+
buffer = bytearray()
546+
buffer.extend(struct.pack(">i", 1)) # row count
547+
548+
# Claim only 3 floats (12 bytes) worth of payload for a 4-float
549+
# (16 byte) column stride.
550+
buffer.extend(struct.pack(">i", 12))
551+
buffer.extend(struct.pack(">3f", 1.0, 2.0, 3.0))
552+
553+
parser = NumpyParser()
554+
reader = BytesIOReader(bytes(buffer))
555+
556+
desc = ParseDesc(
557+
colnames=["vec"],
558+
coltypes=[vector_type],
559+
column_encryption_policy=None,
560+
coldescs=None,
561+
deserializers=obj_array([None]),
562+
protocol_version=5,
563+
)
564+
565+
with self.assertRaisesRegex(ValueError, r"'vec'"):
566+
parser.parse_rows(reader, desc)
567+
479568
def test_unsupported_subtype_falls_back_to_object_array(self):
480569
"""Test that an unsupported vector subtype falls back to an object array"""
481570
vector_size = 2
@@ -533,5 +622,69 @@ def test_unsupported_subtype_falls_back_end_to_end_via_parse_rows(self):
533622
self.assertEqual(list(arr[i]), expected_vector)
534623

535624

625+
@unittest.skipUnless(HAVE_NUMPY_PARSER, "NumPy/Cython extension not available")
626+
class TestNumpyParserScalarType(unittest.TestCase):
627+
"""Regression tests for the plain (non-vector) scalar column fast path.
628+
629+
`_cqltype_to_numpy` in cassandra/numpy_parser.pyx is shared by two call
630+
sites in make_array(): VectorType.subtype dispatch and plain scalar
631+
top-level column dispatch. A fix aimed only at excluding a subtype from
632+
the vector fast path (e.g. ShortType, which is vint-prefixed *inside* a
633+
vector) must not remove that subtype's entry from the shared dict
634+
outright, since that would also silently regress the scalar column fast
635+
path for the exact same type (ShortType/smallint *is* fixed-width as a
636+
scalar column). These tests guard against that regression.
637+
"""
638+
639+
def test_scalar_smallint_uses_fast_masked_int16_array(self):
640+
"""smallint (ShortType) is fixed-width (2 bytes, big-endian, no
641+
length prefix) as a scalar column -- unlike inside a VectorType,
642+
where it is vint-length-prefixed per element. It must still get the
643+
fast masked int16 array here, not fall back to a slow 1D object
644+
array.
645+
"""
646+
from cassandra.numpy_parser import make_array
647+
648+
arr = make_array(cqltypes.ShortType, 5)
649+
650+
self.assertNotEqual(arr.dtype, np.dtype("O"))
651+
self.assertEqual(arr.dtype, np.dtype(">i2"))
652+
self.assertTrue(hasattr(arr, "mask"))
653+
self.assertEqual(arr.shape, (5,))
654+
655+
def test_scalar_smallint_round_trips_end_to_end(self):
656+
"""End-to-end companion: parse actual wire bytes for a smallint
657+
column through NumpyParser.parse_rows() and confirm the result is a
658+
native int16 (masked) array with the correct values, not an object
659+
array.
660+
"""
661+
values = [1, -5, 32767, -32768, 0]
662+
663+
buffer = bytearray()
664+
buffer.extend(struct.pack(">i", len(values))) # row count
665+
for v in values:
666+
buffer.extend(struct.pack(">i", 2)) # smallint byte size
667+
buffer.extend(struct.pack(">h", v))
668+
669+
parser = NumpyParser()
670+
reader = BytesIOReader(bytes(buffer))
671+
672+
desc = ParseDesc(
673+
colnames=["small"],
674+
coltypes=[cqltypes.ShortType],
675+
column_encryption_policy=None,
676+
coldescs=None,
677+
deserializers=obj_array([None]),
678+
protocol_version=5,
679+
)
680+
681+
result = parser.parse_rows(reader, desc)
682+
arr = result["small"]
683+
684+
self.assertNotEqual(arr.dtype, np.dtype("O"))
685+
self.assertEqual(arr.shape, (len(values),))
686+
np.testing.assert_array_equal(arr, np.array(values, dtype=np.int16))
687+
688+
536689
if __name__ == "__main__":
537690
unittest.main()

0 commit comments

Comments
 (0)