[PATCH v2 0/2] reftable: widen allocation size arguments to uintmax_t

["Matthew Suozzo via GitGitGadget" <gitgitgadget@gmail.com>]

Changes since v1:

 * Fix the truncation in reftable_alloc_size() and reftable_calloc() instead
   of checking the count in reftable_obj_record_decode() (Patrick, Aniket).
 * Test reftable_alloc_size() directly instead of the decoder (Aniket).
 * Move the Makefile change into its own patch (Patrick).
 * Rewrite the commit messages (Patrick).

Matthew Suozzo (2):
  Makefile: build the reftable record unit tests
  reftable/basics: widen allocation size arguments to uintmax_t

 Makefile                         |  1 +
 reftable/basics.c                |  9 +++++----
 reftable/basics.h                |  5 +++--
 t/unit-tests/u-reftable-basics.c | 18 ++++++++++++++++++
 4 files changed, 27 insertions(+), 6 deletions(-)


base-commit: 6de20f6092dcf9bdb1c8efe03db4b70c82b423dd
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2255%2Fmsuozzo%2Freftable-obj-record-count-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2255/msuozzo/reftable-obj-record-count-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2255

Range-diff vs v1:

 1:  5fb73ec0a8 ! 1:  0d083bc81f reftable/record: bound an object record's offset count by its length
     @@ Metadata
      Author: Matthew Suozzo <m@suozzo.co>
      
       ## Commit message ##
     -    reftable/record: bound an object record's offset count by its length
     +    Makefile: build the reftable record unit tests
      
     -    An object record's count of offsets is a 64-bit varint from the table,
     -    and reftable_obj_record_decode() allocates the offsets array from it
     -    through REFTABLE_ALLOC_ARRAY(), which takes a size_t. Where size_t is
     -    32 bits, a count of 2^32 + 1 allocates room for one offset, and the
     -    loop, which runs to the full count, writes an offset from the table
     -    for each varint left in the block past the end of the array. Where
     -    size_t is 64 bits, a large count only makes a large allocation that
     -    the decode then fails to fill.
     +    The Meson build includes the reftable record unit tests, but the
     +    Makefile does not. The suite was dropped from the Makefile when the
     +    test was converted to the clar framework in 2596bef5841f (2025-07-24).
      
     -    Nothing in Git reaches this decoder from a repository: a block of a
     -    type other than the one wanted is refused before its records are
     -    decoded, and the object index, the only reader of object blocks, is
     -    used only by the unit tests. The decoder is still wrong.
     +    Include the reftable record unit test suite in the Makefile test
     +    target.
      
     -    Each offset takes at least a byte, so refuse a count larger than what
     -    is left of the record.
     -
     -    The Makefile did not build the reftable record unit tests (the Meson
     -    build does). Add the suite there so that the new test runs in both.
     -
     -    Assisted-by: Claude Opus 5.5, Claude Fable 5.1
     +    Assisted-by: Claude Fable 5.1
          Signed-off-by: Matthew Suozzo <m@suozzo.co>
      
       ## Makefile ##
     @@ Makefile: CLAR_TEST_SUITES += u-reftable-block
       CLAR_TEST_SUITES += u-reftable-stack
       CLAR_TEST_SUITES += u-reftable-table
       CLAR_TEST_SUITES += u-reftable-tree
     -
     - ## reftable/record.c ##
     -@@ reftable/record.c: static int reftable_obj_record_decode(void *rec, struct reftable_buf key,
     - 	if (count == 0)
     - 		return start.len - in.len;
     - 
     -+	/* each offset takes at least a byte */
     -+	if (count > in.len)
     -+		return REFTABLE_FORMAT_ERROR;
     -+
     - 	REFTABLE_ALLOC_ARRAY(r->offsets, count);
     - 	if (!r->offsets)
     - 		return REFTABLE_OUT_OF_MEMORY_ERROR;
     -
     - ## t/unit-tests/u-reftable-record.c ##
     -@@ t/unit-tests/u-reftable-record.c: void test_reftable_record__obj_record_roundtrip(void)
     - 	reftable_buf_release(&scratch);
     - }
     - 
     -+void test_reftable_record__obj_record_decode_count_larger_than_record(void)
     -+{
     -+	struct reftable_buf key = REFTABLE_BUF_INIT, scratch = REFTABLE_BUF_INIT;
     -+	struct reftable_record rec = { .type = REFTABLE_BLOCK_TYPE_OBJ };
     -+	uint8_t buffer[32];
     -+	struct string_view dest = { .buf = buffer, .len = sizeof(buffer) }, in;
     -+	int n;
     -+
     -+	/*
     -+	 * A count of offsets that a 32-bit size_t truncates to one,
     -+	 * followed by two offsets.
     -+	 */
     -+	n = put_var_int(&dest, ((uint64_t)1 << 32) + 1);
     -+	cl_assert_gt_i(n, 0);
     -+	string_view_consume(&dest, n);
     -+	n = put_var_int(&dest, 1);
     -+	cl_assert_gt_i(n, 0);
     -+	string_view_consume(&dest, n);
     -+	n = put_var_int(&dest, 1);
     -+	cl_assert_gt_i(n, 0);
     -+	string_view_consume(&dest, n);
     -+
     -+	in.buf = buffer;
     -+	in.len = sizeof(buffer) - dest.len;
     -+
     -+	cl_assert_equal_i(reftable_buf_addstr(&key, "\x01\x02"), 0);
     -+	cl_assert_equal_i(reftable_record_decode(&rec, key, 0, in,
     -+						 REFTABLE_HASH_SIZE_SHA1, &scratch),
     -+			  REFTABLE_FORMAT_ERROR);
     -+
     -+	reftable_record_release(&rec);
     -+	reftable_buf_release(&key);
     -+	reftable_buf_release(&scratch);
     -+}
     -+
     - void test_reftable_record__index_record_comparison(void)
     - {
     - 	struct reftable_record in[3] = {
 -:  ---------- > 2:  8f4a8389d0 reftable/basics: widen allocation size arguments to uintmax_t

-- 
gitgitgadget