utf8: address Copilot review comments on PR #677

- pack_utf8_span (utf8_ext.pyx): validate rel is well-formed (rel[0]==0,
  non-decreasing, within data's bounds) before the unchecked C loop,
  instead of trusting the caller silently -- a malformed rel previously
  risked an out-of-bounds read/crash.
- Utf8Factorizer docstring: stop claiming a hash collision's duplicate
  vocabulary entry is "benign because callers merge groups by decoded
  value" -- no downstream consumer does that merge, so a collision would
  actually split a group in two. Still an accepted, vanishingly-rare
  risk with 64-bit hashes, just not a resolved one.
- test_utf8.py: two groupby tests assigned float64 values into the
  int64 `x` column via implicit cast; use integer inputs explicitly so
  the test doesn't rely on that coercion.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Author: FrancescAlted

src/blosc2/utf8_array.py

@@ -669,10 +669,14 @@ class Utf8Factorizer:
     first-appearance order; :meth:`uniques` returns the decoded vocabulary in
     that same order.  No row is ever decoded — only the distinct values.
 
-    A hash collision between a new value and a known one is not resolved
-    (the new value gets a fresh code); the resulting duplicate vocabulary
-    entry is benign because callers merge groups by decoded value, and with
-    64-bit hashes the case is vanishingly rare anyway.
+    A hash collision between a new value and an already-known value of the
+    same byte length is not resolved: the new value always gets a fresh
+    code, even in the (unhandled) case where its decoded string equals an
+    existing vocabulary entry's. No downstream consumer merges groups by
+    decoded value, so such a collision would silently split what should be
+    one group into two rather than being caught -- an accepted, unmitigated
+    risk given how vanishingly rare it is with 64-bit hashes, not a
+    guarantee that it is otherwise resolved.
     """
 
     def __init__(self, arr: Utf8Array) -> None:

src/blosc2/utf8_ext.pyx

@@ -80,6 +80,15 @@ def pack_utf8_span(cnp.ndarray rel not None, cnp.ndarray data not None, cnp.ndar
         raise ValueError("rel, data and out must be C-contiguous")
     if n == 0:
         return
+    # Cheap, vectorized well-formedness checks: a malformed rel (decreasing,
+    # negative, or reaching past the end of data) would otherwise drive the
+    # unchecked pointer arithmetic below out of bounds.
+    if int(rel[0]) != 0:
+        raise ValueError("rel[0] must be 0")
+    if bool((np.diff(rel) < 0).any()):
+        raise ValueError("rel must be non-decreasing")
+    if int(rel[n]) > data.shape[0]:
+        raise ValueError("rel values must not exceed len(data)")
 
     cdef const int64_t* rel_ptr = <const int64_t*>cnp.PyArray_DATA(rel)
     cdef const uint8_t* data_ptr = <const uint8_t*>cnp.PyArray_DATA(data)

tests/ctable/test_utf8.py

@@ -259,6 +259,28 @@ def force_kernel_mode(request, monkeypatch):
     return request.param
 
 
+def test_pack_utf8_span_rejects_malformed_rel():
+    """pack_utf8_span trusts its caller's rel/data invariants for speed, but
+    still validates them cheaply up front so a malformed rel fails with a
+    clear ValueError instead of driving the unchecked C loop out of bounds."""
+    pytest.importorskip("blosc2.utf8_ext")
+    from blosc2 import utf8_ext
+
+    data = np.array([1, 2, 3], dtype=np.uint8)
+    out = np.empty(2, dtype=STRING_DTYPE)
+
+    with pytest.raises(ValueError, match="rel\\[0\\] must be 0"):
+        utf8_ext.pack_utf8_span(np.array([1, 2, 3], dtype=np.int64), data, out)
+    with pytest.raises(ValueError, match="non-decreasing"):
+        utf8_ext.pack_utf8_span(np.array([0, 2, 1], dtype=np.int64), data, out)
+    with pytest.raises(ValueError, match="must not exceed len\\(data\\)"):
+        utf8_ext.pack_utf8_span(np.array([0, 2, 10], dtype=np.int64), data, out)
+
+    # a well-formed rel still works after the added checks
+    utf8_ext.pack_utf8_span(np.array([0, 1, 3], dtype=np.int64), data, out)
+    assert list(out) == ["\x01", "\x02\x03"]
+
+
 def test_utf8_array_bulk_read_kernel_and_fallback(force_kernel_mode):
     from blosc2.utf8_array import Utf8Array
 
@@ -874,7 +896,7 @@ def test_ctable_utf8_groupby_many_byte_lengths_and_non_ascii():
     pool = ["", "a", "bb", "café", "日本語のテキスト", "x" * 2000, "münchen"]
     names = [pool[i] for i in rng.integers(0, len(pool), 3000)]
     t = make_table(names)
-    t.x[:] = np.ones(3000)
+    t.x[:] = np.ones(3000, dtype=np.int64)
     g = t.group_by("name").sum("x")
     got = dict(zip(g["name"][:].tolist(), g["x_sum"][:].tolist(), strict=True))
     exp: dict[str, int] = {}
@@ -927,10 +949,10 @@ class FloatRow:
 
 def test_ctable_utf8_groupby_sum():
     t = make_table(["a", "b", "a", "b", "a"])
-    t.x[:] = [1.0, 2.0, 3.0, 4.0, 5.0]
+    t.x[:] = [1, 2, 3, 4, 5]
     g = t.group_by("name").sum("x")
     rows = dict(zip(g["name"][:].tolist(), g["x_sum"][:].tolist(), strict=False))
-    assert rows == {"a": 9.0, "b": 6.0}
+    assert rows == {"a": 9, "b": 6}
 
 
 def test_ctable_utf8_groupby_size_and_dropna():