PJK / libcbor

CBOR protocol implementation for C
MIT License
332 stars 95 forks source link

test_serialize_4b_bytestring / test_serialize_8b_bytestring tests fail on 32-bit systems (size_t overflow) #263

Closed trofi closed 1 year ago

trofi commented 1 year ago

Describe the bug

On i686-linux targets libcbor tests fail as:

[ RUN      ] test_serialize_4b_bytestring
[  ERROR   ] --- 0 != 0x100000004
[   LINE   ] --- libcbor/test/cbor_serialize_test.c:...: error: Failure!
[  FAILED  ] test_serialize_4b_bytestring
...
[  FAILED  ] test_serialize_8b_bytestring

I think it happens in this code: https://github.com/PJK/libcbor/blob/master/test/cbor_serialize_test.c#L161-L183

Annotated:

static void test_serialize_4b_bytestring(void **_CBOR_UNUSED(_state)) {
  cbor_item_t *item = cbor_new_definite_bytestring();

  // Fake having a huge chunk of data
  unsigned char *data = malloc(1);
  cbor_bytestring_set_handle(item, data, UINT32_MAX);

  assert_int_equal(cbor_serialize(item, buffer, 512), 0); // fails to serialze due to size_t overflow
  assert_int_equal(cbor_serialized_size(item), (uint64_t)UINT32_MAX + 5);
    // fails: cbor_serialized_size() returns size_t, can't return more than UINT32_MAX
    // `(uint64_t)UINT32_MAX + 5` always exceeds UINT32_MAX.
  cbor_decref(&item);
}

Aside: assert_int_equal() is a bit suspicious as it uses 32-bit int comparison. Is is intentional?

To Reproduce

On i686-linux run cmake .. && make test.

Expected behavior

tests should pass

Environment

PJK commented 1 year ago

Indeed, you're right, that test is not portable. Thank you for the prompt bug report!

264 should prevent future regressions

trofi commented 1 year ago

master does pass all tests for my environment as well. Thank you!