From cd3686105ac7895f734efc88ac9bc2212b27b4fb Mon Sep 17 00:00:00 2001 From: Pavel Kalvoda Date: Sun, 6 Sep 2020 11:50:50 +0200 Subject: [PATCH 1/7] First stab at reworking NEDATA, remove EBUFFER --- src/cbor.c | 2 - src/cbor/data.h | 2 - src/cbor/streaming.c | 99 +++++++++++++++++----------------- test/cbor_stream_decode_test.c | 29 ++++++++-- 4 files changed, 73 insertions(+), 59 deletions(-) diff --git a/src/cbor.c b/src/cbor.c index f3658c87..76b892c6 100644 --- a/src/cbor.c +++ b/src/cbor.c @@ -82,8 +82,6 @@ cbor_item_t *cbor_load(cbor_data source, size_t source_size, result->error.code = CBOR_ERR_NOTENOUGHDATA; goto error; } - case CBOR_DECODER_EBUFFER: - /* Fallthrough */ case CBOR_DECODER_ERROR: /* Reserved/malformated item */ { diff --git a/src/cbor/data.h b/src/cbor/data.h index 7f703bc0..0934b845 100644 --- a/src/cbor/data.h +++ b/src/cbor/data.h @@ -209,8 +209,6 @@ enum cbor_decoder_status { , CBOR_DECODER_NEDATA /** Not enough data - mismatch with MTB */ , - CBOR_DECODER_EBUFFER /** Buffer manipulation problem */ - , CBOR_DECODER_ERROR /** Malformed or reserved MTB/value */ }; diff --git a/src/cbor/streaming.c b/src/cbor/streaming.c index 9d85e15f..ae131999 100644 --- a/src/cbor/streaming.c +++ b/src/cbor/streaming.c @@ -8,13 +8,12 @@ #include "streaming.h" #include "internal/loaders.h" -bool static _cbor_claim_bytes(size_t required, size_t provided, - struct cbor_decoder_result *result) { +bool static cbor_claim_bytes(size_t required, size_t provided, + struct cbor_decoder_result *result) { if (required > (provided - result->read)) { - /* We need to keep all the metadata if parsing is to be resumed */ + result->required = required + result->read; result->read = 0; result->status = CBOR_DECODER_NEDATA; - result->required = required; return false; } else { result->read += required; @@ -26,14 +25,12 @@ bool static _cbor_claim_bytes(size_t required, size_t provided, struct cbor_decoder_result cbor_stream_decode( cbor_data source, size_t source_size, const struct cbor_callbacks *callbacks, void *context) { - /* If we have no data, we cannot read even the MTB */ - if (source_size < 1) { - return (struct cbor_decoder_result){0, CBOR_DECODER_EBUFFER}; + // Attempt to claim the initial MTB byte + struct cbor_decoder_result result = {.status = CBOR_DECODER_FINISHED}; + if (!cbor_claim_bytes(1, source_size, &result)) { + return result; } - /* If we have a byte, assume it's the MTB */ - struct cbor_decoder_result result = {1, CBOR_DECODER_FINISHED}; - switch (*source) { case 0x00: /* Fallthrough */ case 0x01: /* Fallthrough */ @@ -67,7 +64,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x18: /* One byte unsigned integer */ { - if (_cbor_claim_bytes(1, source_size, &result)) { + if (cbor_claim_bytes(1, source_size, &result)) { callbacks->uint8(context, _cbor_load_uint8(source + 1)); } return result; @@ -75,7 +72,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x19: /* Two bytes unsigned integer */ { - if (_cbor_claim_bytes(2, source_size, &result)) { + if (cbor_claim_bytes(2, source_size, &result)) { callbacks->uint16(context, _cbor_load_uint16(source + 1)); } return result; @@ -83,7 +80,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x1A: /* Four bytes unsigned integer */ { - if (_cbor_claim_bytes(4, source_size, &result)) { + if (cbor_claim_bytes(4, source_size, &result)) { callbacks->uint32(context, _cbor_load_uint32(source + 1)); } return result; @@ -91,7 +88,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x1B: /* Eight bytes unsigned integer */ { - if (_cbor_claim_bytes(8, source_size, &result)) { + if (cbor_claim_bytes(8, source_size, &result)) { callbacks->uint64(context, _cbor_load_uint64(source + 1)); } return result; @@ -135,7 +132,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x38: /* One byte negative integer */ { - if (_cbor_claim_bytes(1, source_size, &result)) { + if (cbor_claim_bytes(1, source_size, &result)) { callbacks->negint8(context, _cbor_load_uint8(source + 1)); } return result; @@ -143,7 +140,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x39: /* Two bytes negative integer */ { - if (_cbor_claim_bytes(2, source_size, &result)) { + if (cbor_claim_bytes(2, source_size, &result)) { callbacks->negint16(context, _cbor_load_uint16(source + 1)); } return result; @@ -151,7 +148,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x3A: /* Four bytes negative integer */ { - if (_cbor_claim_bytes(4, source_size, &result)) { + if (cbor_claim_bytes(4, source_size, &result)) { callbacks->negint32(context, _cbor_load_uint32(source + 1)); } return result; @@ -159,7 +156,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x3B: /* Eight bytes negative integer */ { - if (_cbor_claim_bytes(8, source_size, &result)) { + if (cbor_claim_bytes(8, source_size, &result)) { callbacks->negint64(context, _cbor_load_uint64(source + 1)); } return result; @@ -198,7 +195,7 @@ struct cbor_decoder_result cbor_stream_decode( { size_t length = (size_t)_cbor_load_uint8(source) - 0x40; /* 0x40 offset */ - if (_cbor_claim_bytes(length, source_size, &result)) { + if (cbor_claim_bytes(length, source_size, &result)) { callbacks->byte_string(context, source + 1, length); } return result; @@ -207,9 +204,9 @@ struct cbor_decoder_result cbor_stream_decode( /* One byte length byte string */ // TODO template this? { - if (_cbor_claim_bytes(1, source_size, &result)) { + if (cbor_claim_bytes(1, source_size, &result)) { size_t length = (size_t)_cbor_load_uint8(source + 1); - if (_cbor_claim_bytes(length, source_size, &result)) { + if (cbor_claim_bytes(length, source_size, &result)) { callbacks->byte_string(context, source + 1 + 1, length); } } @@ -218,9 +215,9 @@ struct cbor_decoder_result cbor_stream_decode( case 0x59: /* Two bytes length byte string */ { - if (_cbor_claim_bytes(2, source_size, &result)) { + if (cbor_claim_bytes(2, source_size, &result)) { size_t length = (size_t)_cbor_load_uint16(source + 1); - if (_cbor_claim_bytes(length, source_size, &result)) { + if (cbor_claim_bytes(length, source_size, &result)) { callbacks->byte_string(context, source + 1 + 2, length); } } @@ -229,9 +226,9 @@ struct cbor_decoder_result cbor_stream_decode( case 0x5A: /* Four bytes length byte string */ { - if (_cbor_claim_bytes(4, source_size, &result)) { + if (cbor_claim_bytes(4, source_size, &result)) { size_t length = (size_t)_cbor_load_uint32(source + 1); - if (_cbor_claim_bytes(length, source_size, &result)) { + if (cbor_claim_bytes(length, source_size, &result)) { callbacks->byte_string(context, source + 1 + 4, length); } } @@ -240,9 +237,9 @@ struct cbor_decoder_result cbor_stream_decode( case 0x5B: /* Eight bytes length byte string */ { - if (_cbor_claim_bytes(8, source_size, &result)) { + if (cbor_claim_bytes(8, source_size, &result)) { size_t length = (size_t)_cbor_load_uint64(source + 1); - if (_cbor_claim_bytes(length, source_size, &result)) { + if (cbor_claim_bytes(length, source_size, &result)) { callbacks->byte_string(context, source + 1 + 8, length); } } @@ -287,7 +284,7 @@ struct cbor_decoder_result cbor_stream_decode( { size_t length = (size_t)_cbor_load_uint8(source) - 0x60; /* 0x60 offset */ - if (_cbor_claim_bytes(length, source_size, &result)) { + if (cbor_claim_bytes(length, source_size, &result)) { callbacks->string(context, source + 1, length); } return result; @@ -295,9 +292,9 @@ struct cbor_decoder_result cbor_stream_decode( case 0x78: /* One byte length string */ { - if (_cbor_claim_bytes(1, source_size, &result)) { + if (cbor_claim_bytes(1, source_size, &result)) { size_t length = (size_t)_cbor_load_uint8(source + 1); - if (_cbor_claim_bytes(length, source_size, &result)) { + if (cbor_claim_bytes(length, source_size, &result)) { callbacks->string(context, source + 1 + 1, length); } } @@ -306,9 +303,9 @@ struct cbor_decoder_result cbor_stream_decode( case 0x79: /* Two bytes length string */ { - if (_cbor_claim_bytes(2, source_size, &result)) { + if (cbor_claim_bytes(2, source_size, &result)) { size_t length = (size_t)_cbor_load_uint16(source + 1); - if (_cbor_claim_bytes(length, source_size, &result)) { + if (cbor_claim_bytes(length, source_size, &result)) { callbacks->string(context, source + 1 + 2, length); } } @@ -317,9 +314,9 @@ struct cbor_decoder_result cbor_stream_decode( case 0x7A: /* Four bytes length string */ { - if (_cbor_claim_bytes(4, source_size, &result)) { + if (cbor_claim_bytes(4, source_size, &result)) { size_t length = (size_t)_cbor_load_uint32(source + 1); - if (_cbor_claim_bytes(length, source_size, &result)) { + if (cbor_claim_bytes(length, source_size, &result)) { callbacks->string(context, source + 1 + 4, length); } } @@ -328,9 +325,9 @@ struct cbor_decoder_result cbor_stream_decode( case 0x7B: /* Eight bytes length string */ { - if (_cbor_claim_bytes(8, source_size, &result)) { + if (cbor_claim_bytes(8, source_size, &result)) { size_t length = (size_t)_cbor_load_uint64(source + 1); - if (_cbor_claim_bytes(length, source_size, &result)) { + if (cbor_claim_bytes(length, source_size, &result)) { callbacks->string(context, source + 1 + 8, length); } } @@ -380,7 +377,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x98: /* One byte length array */ { - if (_cbor_claim_bytes(1, source_size, &result)) { + if (cbor_claim_bytes(1, source_size, &result)) { callbacks->array_start(context, (size_t)_cbor_load_uint8(source + 1)); } return result; @@ -388,7 +385,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x99: /* Two bytes length array */ { - if (_cbor_claim_bytes(2, source_size, &result)) { + if (cbor_claim_bytes(2, source_size, &result)) { callbacks->array_start(context, (size_t)_cbor_load_uint16(source + 1)); } @@ -397,7 +394,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x9A: /* Four bytes length array */ { - if (_cbor_claim_bytes(4, source_size, &result)) { + if (cbor_claim_bytes(4, source_size, &result)) { callbacks->array_start(context, (size_t)_cbor_load_uint32(source + 1)); } @@ -406,7 +403,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x9B: /* Eight bytes length array */ { - if (_cbor_claim_bytes(8, source_size, &result)) { + if (cbor_claim_bytes(8, source_size, &result)) { callbacks->array_start(context, (size_t)_cbor_load_uint64(source + 1)); } @@ -456,7 +453,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0xB8: /* One byte length map */ { - if (_cbor_claim_bytes(1, source_size, &result)) { + if (cbor_claim_bytes(1, source_size, &result)) { callbacks->map_start(context, (size_t)_cbor_load_uint8(source + 1)); } return result; @@ -464,7 +461,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0xB9: /* Two bytes length map */ { - if (_cbor_claim_bytes(2, source_size, &result)) { + if (cbor_claim_bytes(2, source_size, &result)) { callbacks->map_start(context, (size_t)_cbor_load_uint16(source + 1)); } return result; @@ -472,7 +469,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0xBA: /* Four bytes length map */ { - if (_cbor_claim_bytes(4, source_size, &result)) { + if (cbor_claim_bytes(4, source_size, &result)) { callbacks->map_start(context, (size_t)_cbor_load_uint32(source + 1)); } return result; @@ -480,7 +477,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0xBB: /* Eight bytes length map */ { - if (_cbor_claim_bytes(8, source_size, &result)) { + if (cbor_claim_bytes(8, source_size, &result)) { callbacks->map_start(context, (size_t)_cbor_load_uint64(source + 1)); } return result; @@ -541,28 +538,28 @@ struct cbor_decoder_result cbor_stream_decode( } case 0xD8: /* 1B tag */ { - if (_cbor_claim_bytes(1, source_size, &result)) { + if (cbor_claim_bytes(1, source_size, &result)) { callbacks->tag(context, _cbor_load_uint8(source + 1)); } return result; } case 0xD9: /* 2B tag */ { - if (_cbor_claim_bytes(2, source_size, &result)) { + if (cbor_claim_bytes(2, source_size, &result)) { callbacks->tag(context, _cbor_load_uint16(source + 1)); } return result; } case 0xDA: /* 4B tag */ { - if (_cbor_claim_bytes(4, source_size, &result)) { + if (cbor_claim_bytes(4, source_size, &result)) { callbacks->tag(context, _cbor_load_uint32(source + 1)); } return result; } case 0xDB: /* 8B tag */ { - if (_cbor_claim_bytes(8, source_size, &result)) { + if (cbor_claim_bytes(8, source_size, &result)) { callbacks->tag(context, _cbor_load_uint64(source + 1)); } return result; @@ -627,7 +624,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0xF9: /* 2B float */ { - if (_cbor_claim_bytes(2, source_size, &result)) { + if (cbor_claim_bytes(2, source_size, &result)) { callbacks->float2(context, _cbor_load_half(source + 1)); } return result; @@ -635,7 +632,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0xFA: /* 4B float */ { - if (_cbor_claim_bytes(4, source_size, &result)) { + if (cbor_claim_bytes(4, source_size, &result)) { callbacks->float4(context, _cbor_load_float(source + 1)); } return result; @@ -643,7 +640,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0xFB: /* 8B float */ { - if (_cbor_claim_bytes(8, source_size, &result)) { + if (cbor_claim_bytes(8, source_size, &result)) { callbacks->float8(context, _cbor_load_double(source + 1)); } return result; diff --git a/test/cbor_stream_decode_test.c b/test/cbor_stream_decode_test.c index 94de21f8..cea2dd4c 100644 --- a/test/cbor_stream_decode_test.c +++ b/test/cbor_stream_decode_test.c @@ -15,6 +15,10 @@ #include "cbor.h" #include "stream_expectations.h" +static void test_no_data(void **state) { + assert_decoder_result_nedata(1, decode(NULL, 0)); +} + unsigned char embedded_uint8_data[] = {0x00, 0x01, 0x05, 0x17}; static void test_uint8_embedded_decoding(void **state) { assert_uint8_eq(0); @@ -41,12 +45,17 @@ static void test_uint8_decoding(void **state) { assert_uint8_eq(0xFF); assert_decoder_result(2, CBOR_DECODER_FINISHED, decode(uint8_data + 2, 2)); + + assert_decoder_result_nedata(2, decode(uint8_data, 1)); } unsigned char uint16_data[] = {0x19, 0x01, 0xf4}; static void test_uint16_decoding(void **state) { assert_uint16_eq(500); assert_decoder_result(3, CBOR_DECODER_FINISHED, decode(uint16_data, 3)); + + assert_decoder_result_nedata(3, decode(uint16_data, 1)); + assert_decoder_result_nedata(3, decode(uint16_data, 2)); } unsigned char uint32_data[] = {0x1a, 0xa5, 0xf7, 0x02, 0xb3}; @@ -127,6 +136,10 @@ static void test_bstring_int16_decoding(void **state) { assert_bstring_mem_eq(bstring_int16_data + 3, 348); assert_decoder_result(3 + 348, CBOR_DECODER_FINISHED, decode(bstring_int16_data, 3 + 348)); + + assert_decoder_result_nedata(3, decode(bstring_int16_data, 1)); + assert_decoder_result_nedata(3, decode(bstring_int16_data, 2)); + assert_decoder_result_nedata(351, decode(bstring_int16_data, 3)); } unsigned char bstring_int32_data[] = {0x5A, 0x00, 0x10, 0x10, @@ -390,15 +403,21 @@ static void test_indef_map_decoding_1(void **state) { decode(indef_map_data_1 + 4, 1)); } -unsigned char map_nedata[] = {0xBB, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x01, 0x19, 0x01, 0xf4, 0x01}; +unsigned char map_nedata[] = {0xBB, // 8B map + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x01, // 1 + 0x19, 0x01, 0xf4, // uint16(500) + 0x01}; static void test_nedata_map_decoding(void **state) { - assert_decoder_result_nedata(8, decode(map_nedata, 1)); + for (int i = 1; i < 9; i++) { + assert_decoder_result_nedata(9, decode(map_nedata, i)); + } assert_map_start(1); assert_decoder_result(9, CBOR_DECODER_FINISHED, decode(map_nedata, 9)); - assert_decoder_result_nedata(2, decode(map_nedata + 9, 1)); + assert_decoder_result_nedata(3, decode(map_nedata + 9, 1)); + assert_decoder_result_nedata(3, decode(map_nedata + 9, 2)); assert_uint16_eq(500); assert_decoder_result(3, CBOR_DECODER_FINISHED, decode(map_nedata + 9, 3)); @@ -489,6 +508,8 @@ static void test_undef_decoding(void **state) { int main(void) { set_decoder(&cbor_stream_decode); const struct CMUnitTest tests[] = { + cmocka_unit_test(test_no_data), + cmocka_unit_test(test_uint8_embedded_decoding), cmocka_unit_test(test_uint8_decoding), cmocka_unit_test(test_uint16_decoding), From 8369960c6a7d32eb50f6810742913bd4a862ea41 Mon Sep 17 00:00:00 2001 From: Pavel Kalvoda Date: Sun, 6 Sep 2020 12:32:26 +0200 Subject: [PATCH 2/7] Add NEDATA test coverage --- test/assertions.c | 7 +++ test/assertions.h | 8 +++ test/cbor_stream_decode_test.c | 93 ++++++++++++++++++++++------------ test/stream_expectations.h | 3 +- 4 files changed, 77 insertions(+), 34 deletions(-) diff --git a/test/assertions.c b/test/assertions.c index 807cd5a5..16fb4cda 100644 --- a/test/assertions.c +++ b/test/assertions.c @@ -6,6 +6,7 @@ */ #include "assertions.h" +#include "stream_expectations.h" void assert_uint8(cbor_item_t* item, uint8_t num) { assert_true(cbor_isa_uint(item)); @@ -44,3 +45,9 @@ void assert_decoder_result_nedata(size_t required, assert_true(CBOR_DECODER_NEDATA == result.status); assert_int_equal((int)required, (int)result.required); } + +void assert_minimum_input_size(size_t expected, cbor_data data) { + for (size_t available = 1; available < expected; available++) { + assert_decoder_result_nedata(expected, decode(data, 1)); + } +} diff --git a/test/assertions.h b/test/assertions.h index 24d36d74..b52d2dce 100644 --- a/test/assertions.h +++ b/test/assertions.h @@ -16,6 +16,14 @@ void assert_uint64(cbor_item_t* item, uint64_t num); void assert_decoder_result(size_t, enum cbor_decoder_status, struct cbor_decoder_result); + +// TODO: Docs void assert_decoder_result_nedata(size_t, struct cbor_decoder_result); +/** + * Check that the streaming decoder will returns a correct CBOR_DECODER_NEDATA + * result for all inputs from data[0..1] through data[0..(expected-1)]. + */ +void assert_minimum_input_size(size_t expected, cbor_data data); + #endif diff --git a/test/cbor_stream_decode_test.c b/test/cbor_stream_decode_test.c index cea2dd4c..b78b252a 100644 --- a/test/cbor_stream_decode_test.c +++ b/test/cbor_stream_decode_test.c @@ -46,7 +46,7 @@ static void test_uint8_decoding(void **state) { assert_uint8_eq(0xFF); assert_decoder_result(2, CBOR_DECODER_FINISHED, decode(uint8_data + 2, 2)); - assert_decoder_result_nedata(2, decode(uint8_data, 1)); + assert_minimum_input_size(2, uint8_data); } unsigned char uint16_data[] = {0x19, 0x01, 0xf4}; @@ -54,14 +54,15 @@ static void test_uint16_decoding(void **state) { assert_uint16_eq(500); assert_decoder_result(3, CBOR_DECODER_FINISHED, decode(uint16_data, 3)); - assert_decoder_result_nedata(3, decode(uint16_data, 1)); - assert_decoder_result_nedata(3, decode(uint16_data, 2)); + assert_minimum_input_size(3, uint16_data); } unsigned char uint32_data[] = {0x1a, 0xa5, 0xf7, 0x02, 0xb3}; static void test_uint32_decoding(void **state) { assert_uint32_eq((uint32_t)2784428723UL); assert_decoder_result(5, CBOR_DECODER_FINISHED, decode(uint32_data, 5)); + + assert_minimum_input_size(5, uint32_data); } unsigned char uint64_data[] = {0x1b, 0xa5, 0xf7, 0x02, 0xb3, @@ -69,6 +70,8 @@ unsigned char uint64_data[] = {0x1b, 0xa5, 0xf7, 0x02, 0xb3, static void test_uint64_decoding(void **state) { assert_uint64_eq(11959030306112471731ULL); assert_decoder_result(9, CBOR_DECODER_FINISHED, decode(uint64_data, 9)); + + assert_minimum_input_size(9, uint64_data); } unsigned char embedded_negint8_data[] = {0x20, 0x21, 0x25, 0x37}; @@ -97,18 +100,24 @@ static void test_negint8_decoding(void **state) { assert_negint8_eq(0xFF); assert_decoder_result(2, CBOR_DECODER_FINISHED, decode(negint8_data + 2, 2)); + + assert_minimum_input_size(2, negint8_data); } unsigned char negint16_data[] = {0x39, 0x01, 0xf4}; static void test_negint16_decoding(void **state) { assert_negint16_eq(500); assert_decoder_result(3, CBOR_DECODER_FINISHED, decode(negint16_data, 3)); + + assert_minimum_input_size(3, negint16_data); } unsigned char negint32_data[] = {0x3a, 0xa5, 0xf7, 0x02, 0xb3}; static void test_negint32_decoding(void **state) { assert_negint32_eq((uint32_t)2784428723UL); assert_decoder_result(5, CBOR_DECODER_FINISHED, decode(negint32_data, 5)); + + assert_minimum_input_size(5, negint32_data); } unsigned char negint64_data[] = {0x3b, 0xa5, 0xf7, 0x02, 0xb3, @@ -116,6 +125,8 @@ unsigned char negint64_data[] = {0x3b, 0xa5, 0xf7, 0x02, 0xb3, static void test_negint64_decoding(void **state) { assert_negint64_eq(11959030306112471731ULL); assert_decoder_result(9, CBOR_DECODER_FINISHED, decode(negint64_data, 9)); + + assert_minimum_input_size(9, negint64_data); } unsigned char bstring_embedded_int8_data[] = {0x41, 0xFF}; @@ -123,12 +134,18 @@ static void test_bstring_embedded_int8_decoding(void **state) { assert_bstring_mem_eq(bstring_embedded_int8_data + 1, 1); assert_decoder_result(2, CBOR_DECODER_FINISHED, decode(bstring_embedded_int8_data, 2)); + + assert_minimum_input_size(2, bstring_embedded_int8_data); } +// TODO: Add tests with actual bstring/string chunks + unsigned char bstring_int8_data[] = {0x58, 0x00}; static void test_bstring_int8_decoding(void **state) { assert_bstring_mem_eq(bstring_int8_data + 2, 0); assert_decoder_result(2, CBOR_DECODER_FINISHED, decode(bstring_int8_data, 2)); + + assert_minimum_input_size(2, bstring_int8_data); } unsigned char bstring_int16_data[] = {0x59, 0x01, 0x5C /*, [348 bytes] */}; @@ -137,9 +154,8 @@ static void test_bstring_int16_decoding(void **state) { assert_decoder_result(3 + 348, CBOR_DECODER_FINISHED, decode(bstring_int16_data, 3 + 348)); - assert_decoder_result_nedata(3, decode(bstring_int16_data, 1)); - assert_decoder_result_nedata(3, decode(bstring_int16_data, 2)); - assert_decoder_result_nedata(351, decode(bstring_int16_data, 3)); + assert_minimum_input_size(3, bstring_int16_data); + assert_decoder_result_nedata(3 + 348, decode(bstring_int16_data, 3)); } unsigned char bstring_int32_data[] = {0x5A, 0x00, 0x10, 0x10, @@ -148,6 +164,9 @@ static void test_bstring_int32_decoding(void **state) { assert_bstring_mem_eq(bstring_int32_data + 5, 1052688); assert_decoder_result(5 + 1052688, CBOR_DECODER_FINISHED, decode(bstring_int32_data, 5 + 1052688)); + + assert_minimum_input_size(5, bstring_int32_data); + assert_decoder_result_nedata(5 + 1052688, decode(bstring_int32_data, 5)); } #ifdef EIGHT_BYTE_SIZE_T @@ -158,6 +177,9 @@ static void test_bstring_int64_decoding(void **state) { assert_bstring_mem_eq(bstring_int64_data + 9, 4294967296); assert_decoder_result(9 + 4294967296, CBOR_DECODER_FINISHED, decode(bstring_int64_data, 9 + 4294967296)); + + assert_minimum_input_size(9, bstring_int64_data); + assert_decoder_result_nedata(9 + 4294967296, decode(bstring_int64_data, 9)); } #endif @@ -188,6 +210,7 @@ static void test_bstring_indef_decoding_2(void **state) { decode(bstring_indef_2_data + 1, 1)); } +// TODO: Comment formatting seems weird unsigned char bstring_indef_3_data[] = { 0x5F, 0x40 /* Empty byte string */, 0x58, 0x01, 0xFF /* 1B 1 char bytes string */, 0xFF}; @@ -228,6 +251,8 @@ static void test_array_int8_decoding(void **state) { assert_uint8_eq(1); assert_decoder_result(1, CBOR_DECODER_FINISHED, decode(array_int8_data + 3, 1)); + + assert_minimum_input_size(2, array_int8_data); } unsigned char array_int16_data[] = {0x99, 0x00, 0x02, 0x00, 0x01}; @@ -242,6 +267,8 @@ static void test_array_int16_decoding(void **state) { assert_uint8_eq(1); assert_decoder_result(1, CBOR_DECODER_FINISHED, decode(array_int16_data + 4, 1)); + + assert_minimum_input_size(3, array_int16_data); } unsigned char array_int32_data[] = {0x9A, 0x00, 0x00, 0x00, 0x02, 0x00, 0x01}; @@ -256,6 +283,8 @@ static void test_array_int32_decoding(void **state) { assert_uint8_eq(1); assert_decoder_result(1, CBOR_DECODER_FINISHED, decode(array_int32_data + 6, 1)); + + assert_minimum_input_size(5, array_int32_data); } unsigned char array_int64_data[] = {0x9B, 0x00, 0x00, 0x00, 0x00, 0x00, @@ -271,6 +300,8 @@ static void test_array_int64_decoding(void **state) { assert_uint8_eq(1); assert_decoder_result(1, CBOR_DECODER_FINISHED, decode(array_int64_data + 10, 1)); + + assert_minimum_input_size(9, array_int64_data); } unsigned char array_of_arrays_data[] = {0x82, 0x80, 0x80}; @@ -340,6 +371,8 @@ static void test_map_int8_decoding(void **state) { assert_uint8_eq(1); assert_decoder_result(1, CBOR_DECODER_FINISHED, decode(map_int8_data + 3, 1)); + + assert_minimum_input_size(2, map_int8_data); } unsigned char map_int16_data[] = {0xB9, 0x00, 0x01, 0x00, 0x01}; @@ -354,6 +387,8 @@ static void test_map_int16_decoding(void **state) { assert_uint8_eq(1); assert_decoder_result(1, CBOR_DECODER_FINISHED, decode(map_int16_data + 4, 1)); + + assert_minimum_input_size(3, map_int16_data); } unsigned char map_int32_data[] = {0xBA, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01}; @@ -368,6 +403,8 @@ static void test_map_int32_decoding(void **state) { assert_uint8_eq(1); assert_decoder_result(1, CBOR_DECODER_FINISHED, decode(map_int32_data + 6, 1)); + + assert_minimum_input_size(5, map_int32_data); } unsigned char map_int64_data[] = {0xBB, 0x00, 0x00, 0x00, 0x00, 0x00, @@ -383,6 +420,8 @@ static void test_map_int64_decoding(void **state) { assert_uint8_eq(1); assert_decoder_result(1, CBOR_DECODER_FINISHED, decode(map_int64_data + 10, 1)); + + assert_minimum_input_size(9, map_int64_data); } unsigned char indef_map_data_1[] = {0xBF, 0x00, 0x18, 0xFF, 0xFF}; @@ -403,29 +442,6 @@ static void test_indef_map_decoding_1(void **state) { decode(indef_map_data_1 + 4, 1)); } -unsigned char map_nedata[] = {0xBB, // 8B map - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x01, // 1 - 0x19, 0x01, 0xf4, // uint16(500) - 0x01}; -static void test_nedata_map_decoding(void **state) { - for (int i = 1; i < 9; i++) { - assert_decoder_result_nedata(9, decode(map_nedata, i)); - } - - assert_map_start(1); - assert_decoder_result(9, CBOR_DECODER_FINISHED, decode(map_nedata, 9)); - - assert_decoder_result_nedata(3, decode(map_nedata + 9, 1)); - assert_decoder_result_nedata(3, decode(map_nedata + 9, 2)); - - assert_uint16_eq(500); - assert_decoder_result(3, CBOR_DECODER_FINISHED, decode(map_nedata + 9, 3)); - - assert_uint8_eq(1); - assert_decoder_result(1, CBOR_DECODER_FINISHED, decode(map_nedata + 12, 1)); -} - unsigned char embedded_tag_data[] = {0xC1}; static void test_embedded_tag_decoding(void **state) { assert_tag_eq(1); @@ -436,18 +452,24 @@ unsigned char int8_tag_data[] = {0xD8, 0xFE}; static void test_int8_tag_decoding(void **state) { assert_tag_eq(254); assert_decoder_result(2, CBOR_DECODER_FINISHED, decode(int8_tag_data, 2)); + + assert_minimum_input_size(2, int8_tag_data); } unsigned char int16_tag_data[] = {0xD9, 0xFE, 0xFD}; static void test_int16_tag_decoding(void **state) { assert_tag_eq(65277); assert_decoder_result(3, CBOR_DECODER_FINISHED, decode(int16_tag_data, 3)); + + assert_minimum_input_size(3, int16_tag_data); } unsigned char int32_tag_data[] = {0xDA, 0xFE, 0xFD, 0xFC, 0xFB}; static void test_int32_tag_decoding(void **state) { assert_tag_eq(4278058235ULL); assert_decoder_result(5, CBOR_DECODER_FINISHED, decode(int32_tag_data, 5)); + + assert_minimum_input_size(5, int32_tag_data); } unsigned char int64_tag_data[] = {0xDB, 0xFE, 0xFD, 0xFC, 0xFB, @@ -455,6 +477,8 @@ unsigned char int64_tag_data[] = {0xDB, 0xFE, 0xFD, 0xFC, 0xFB, static void test_int64_tag_decoding(void **state) { assert_tag_eq(18374120213919168759ULL); assert_decoder_result(9, CBOR_DECODER_FINISHED, decode(int64_tag_data, 9)); + + assert_minimum_input_size(9, int64_tag_data); } unsigned char bad_tag_data[] = {0xC6}; @@ -464,14 +488,18 @@ static void test_bad_tag_decoding(void **state) { unsigned char float2_data[] = {0xF9, 0x7B, 0xFF}; static void test_float2_decoding(void **state) { - assert_half(65504.0); + assert_half(65504.0f); assert_decoder_result(3, CBOR_DECODER_FINISHED, decode(float2_data, 3)); + + assert_minimum_input_size(3, float2_data); } unsigned char float4_data[] = {0xFA, 0x47, 0xC3, 0x50, 0x00}; static void test_float4_decoding(void **state) { - assert_float(100000.0); + assert_float(100000.0f); assert_decoder_result(5, CBOR_DECODER_FINISHED, decode(float4_data, 5)); + + assert_minimum_input_size(5, float4_data); } unsigned char float8_data[] = {0xFB, 0xC0, 0x10, 0x66, 0x66, @@ -479,6 +507,8 @@ unsigned char float8_data[] = {0xFB, 0xC0, 0x10, 0x66, 0x66, static void test_float8_decoding(void **state) { assert_double(-4.1); assert_decoder_result(9, CBOR_DECODER_FINISHED, decode(float8_data, 9)); + + assert_minimum_input_size(0, float8_data); } unsigned char false_data[] = {0xF4}; @@ -547,7 +577,6 @@ int main(void) { cmocka_unit_test(test_map_int32_decoding), cmocka_unit_test(test_map_int64_decoding), cmocka_unit_test(test_indef_map_decoding_1), - cmocka_unit_test(test_nedata_map_decoding), cmocka_unit_test(test_embedded_tag_decoding), cmocka_unit_test(test_int8_tag_decoding), diff --git a/test/stream_expectations.h b/test/stream_expectations.h index 179588c7..7495b08c 100644 --- a/test/stream_expectations.h +++ b/test/stream_expectations.h @@ -73,15 +73,14 @@ struct test_assertion { }; /* Tested function */ +// TODO: This looks overengineered, we only ever use one (?) in the testsuite typedef struct cbor_decoder_result decoder_t(cbor_data, size_t, const struct cbor_callbacks *, void *); - void set_decoder(decoder_t *); struct cbor_decoder_result decode(cbor_data, size_t); /* Assertions builders */ - void assert_uint8_eq(uint8_t); void assert_uint16_eq(uint16_t); void assert_uint32_eq(uint32_t); From 763117b09b3a679f6a7a215471212739d0a013b7 Mon Sep 17 00:00:00 2001 From: Pavel Kalvoda Date: Sun, 6 Sep 2020 13:12:11 +0200 Subject: [PATCH 3/7] Properly isolate case failures in stream test --- test/cbor_stream_decode_test.c | 108 +++++++++++++++++---------------- test/stream_expectations.c | 75 +++++++++++++---------- test/stream_expectations.c.erb | 27 ++++++--- test/stream_expectations.h | 5 ++ 4 files changed, 124 insertions(+), 91 deletions(-) diff --git a/test/cbor_stream_decode_test.c b/test/cbor_stream_decode_test.c index b78b252a..1d90bc7f 100644 --- a/test/cbor_stream_decode_test.c +++ b/test/cbor_stream_decode_test.c @@ -535,63 +535,65 @@ static void test_undef_decoding(void **state) { assert_decoder_result(1, CBOR_DECODER_FINISHED, decode(undef_data, 1)); } +#define stream_test(f) cmocka_unit_test_teardown(f, clear_stream_assertions) + int main(void) { set_decoder(&cbor_stream_decode); const struct CMUnitTest tests[] = { - cmocka_unit_test(test_no_data), - - cmocka_unit_test(test_uint8_embedded_decoding), - cmocka_unit_test(test_uint8_decoding), - cmocka_unit_test(test_uint16_decoding), - cmocka_unit_test(test_uint32_decoding), - cmocka_unit_test(test_uint64_decoding), - - cmocka_unit_test(test_negint8_embedded_decoding), - cmocka_unit_test(test_negint8_decoding), - cmocka_unit_test(test_negint16_decoding), - cmocka_unit_test(test_negint32_decoding), - cmocka_unit_test(test_negint64_decoding), - - cmocka_unit_test(test_bstring_embedded_int8_decoding), - cmocka_unit_test(test_bstring_int8_decoding), - cmocka_unit_test(test_bstring_int16_decoding), - cmocka_unit_test(test_bstring_int32_decoding), + stream_test(test_no_data), + + stream_test(test_uint8_embedded_decoding), + stream_test(test_uint8_decoding), + stream_test(test_uint16_decoding), + stream_test(test_uint32_decoding), + stream_test(test_uint64_decoding), + + stream_test(test_negint8_embedded_decoding), + stream_test(test_negint8_decoding), + stream_test(test_negint16_decoding), + stream_test(test_negint32_decoding), + stream_test(test_negint64_decoding), + + stream_test(test_bstring_embedded_int8_decoding), + stream_test(test_bstring_int8_decoding), + stream_test(test_bstring_int16_decoding), + stream_test(test_bstring_int32_decoding), #ifdef EIGHT_BYTE_SIZE_T - cmocka_unit_test(test_bstring_int64_decoding), + stream_test(test_bstring_int64_decoding), #endif - cmocka_unit_test(test_bstring_indef_decoding_1), - cmocka_unit_test(test_bstring_indef_decoding_2), - cmocka_unit_test(test_bstring_indef_decoding_3), - - cmocka_unit_test(test_array_embedded_int8_decoding), - cmocka_unit_test(test_array_int8_decoding), - cmocka_unit_test(test_array_int16_decoding), - cmocka_unit_test(test_array_int32_decoding), - cmocka_unit_test(test_array_int64_decoding), - cmocka_unit_test(test_array_of_arrays_decoding), - cmocka_unit_test(test_indef_array_decoding_1), - - cmocka_unit_test(test_map_embedded_int8_decoding), - cmocka_unit_test(test_map_int8_decoding), - cmocka_unit_test(test_map_int16_decoding), - cmocka_unit_test(test_map_int32_decoding), - cmocka_unit_test(test_map_int64_decoding), - cmocka_unit_test(test_indef_map_decoding_1), - - cmocka_unit_test(test_embedded_tag_decoding), - cmocka_unit_test(test_int8_tag_decoding), - cmocka_unit_test(test_int16_tag_decoding), - cmocka_unit_test(test_int32_tag_decoding), - cmocka_unit_test(test_int64_tag_decoding), - cmocka_unit_test(test_bad_tag_decoding), - - cmocka_unit_test(test_float2_decoding), - cmocka_unit_test(test_float4_decoding), - cmocka_unit_test(test_float8_decoding), - - cmocka_unit_test(test_false_decoding), - cmocka_unit_test(test_true_decoding), - cmocka_unit_test(test_null_decoding), - cmocka_unit_test(test_undef_decoding)}; + stream_test(test_bstring_indef_decoding_1), + stream_test(test_bstring_indef_decoding_2), + stream_test(test_bstring_indef_decoding_3), + + stream_test(test_array_embedded_int8_decoding), + stream_test(test_array_int8_decoding), + stream_test(test_array_int16_decoding), + stream_test(test_array_int32_decoding), + stream_test(test_array_int64_decoding), + stream_test(test_array_of_arrays_decoding), + stream_test(test_indef_array_decoding_1), + + stream_test(test_map_embedded_int8_decoding), + stream_test(test_map_int8_decoding), + stream_test(test_map_int16_decoding), + stream_test(test_map_int32_decoding), + stream_test(test_map_int64_decoding), + stream_test(test_indef_map_decoding_1), + + stream_test(test_embedded_tag_decoding), + stream_test(test_int8_tag_decoding), + stream_test(test_int16_tag_decoding), + stream_test(test_int32_tag_decoding), + stream_test(test_int64_tag_decoding), + stream_test(test_bad_tag_decoding), + + stream_test(test_float2_decoding), + stream_test(test_float4_decoding), + stream_test(test_float8_decoding), + + stream_test(test_false_decoding), + stream_test(test_true_decoding), + stream_test(test_null_decoding), + stream_test(test_undef_decoding)}; return cmocka_run_group_tests(tests, NULL, NULL); } diff --git a/test/stream_expectations.c b/test/stream_expectations.c index 3bf0ebe8..890fafa7 100644 --- a/test/stream_expectations.c +++ b/test/stream_expectations.c @@ -1,12 +1,25 @@ #include "stream_expectations.h" +// TODO: The saved keystrokes are not worth the complexity. Get rid of this +// file to prevent confusion, the fundamental structure is unlikely to change +// in the future. + /* Ordered from 0 to queue_size - 1 */ struct test_assertion assertions_queue[MAX_QUEUE_ITEMS]; -size_t queue_size = 0; -size_t current_expectation = 0; -decoder_t* decoder; +int queue_size = 0; +int current_expectation = 0; +decoder_t *decoder; + +void set_decoder(decoder_t *dec) { decoder = dec; } -void set_decoder(decoder_t* dec) { decoder = dec; } +int clear_stream_assertions(void **state) { + if (queue_size != current_expectation) { + return 1; // We have not matched all expectations correctly + } + queue_size = current_expectation = 0; + free(*state); + return 0; +} /* Callbacks */ struct test_assertion current() { @@ -20,7 +33,7 @@ void assert_uint8_eq(uint8_t actual) { UINT8_EQ, (union test_expectation_data){.int8 = actual}}; } -void uint8_callback(void* context, uint8_t actual) { +void uint8_callback(void *context, uint8_t actual) { assert_true(current().expectation == UINT8_EQ); assert_true(current().data.int8 == actual); current_expectation++; @@ -31,7 +44,7 @@ void assert_uint16_eq(uint16_t actual) { UINT16_EQ, (union test_expectation_data){.int16 = actual}}; } -void uint16_callback(void* context, uint16_t actual) { +void uint16_callback(void *context, uint16_t actual) { assert_true(current().expectation == UINT16_EQ); assert_true(current().data.int16 == actual); current_expectation++; @@ -42,7 +55,7 @@ void assert_uint32_eq(uint32_t actual) { UINT32_EQ, (union test_expectation_data){.int32 = actual}}; } -void uint32_callback(void* context, uint32_t actual) { +void uint32_callback(void *context, uint32_t actual) { assert_true(current().expectation == UINT32_EQ); assert_true(current().data.int32 == actual); current_expectation++; @@ -53,7 +66,7 @@ void assert_uint64_eq(uint64_t actual) { UINT64_EQ, (union test_expectation_data){.int64 = actual}}; } -void uint64_callback(void* context, uint64_t actual) { +void uint64_callback(void *context, uint64_t actual) { assert_true(current().expectation == UINT64_EQ); assert_true(current().data.int64 == actual); current_expectation++; @@ -64,7 +77,7 @@ void assert_negint8_eq(uint8_t actual) { NEGINT8_EQ, (union test_expectation_data){.int8 = actual}}; } -void negint8_callback(void* context, uint8_t actual) { +void negint8_callback(void *context, uint8_t actual) { assert_true(current().expectation == NEGINT8_EQ); assert_true(current().data.int8 == actual); current_expectation++; @@ -75,7 +88,7 @@ void assert_negint16_eq(uint16_t actual) { NEGINT16_EQ, (union test_expectation_data){.int16 = actual}}; } -void negint16_callback(void* context, uint16_t actual) { +void negint16_callback(void *context, uint16_t actual) { assert_true(current().expectation == NEGINT16_EQ); assert_true(current().data.int16 == actual); current_expectation++; @@ -86,7 +99,7 @@ void assert_negint32_eq(uint32_t actual) { NEGINT32_EQ, (union test_expectation_data){.int32 = actual}}; } -void negint32_callback(void* context, uint32_t actual) { +void negint32_callback(void *context, uint32_t actual) { assert_true(current().expectation == NEGINT32_EQ); assert_true(current().data.int32 == actual); current_expectation++; @@ -97,7 +110,7 @@ void assert_negint64_eq(uint64_t actual) { NEGINT64_EQ, (union test_expectation_data){.int64 = actual}}; } -void negint64_callback(void* context, uint64_t actual) { +void negint64_callback(void *context, uint64_t actual) { assert_true(current().expectation == NEGINT64_EQ); assert_true(current().data.int64 == actual); current_expectation++; @@ -109,7 +122,7 @@ void assert_bstring_mem_eq(cbor_data address, size_t length) { (union test_expectation_data){.string = {address, length}}}; } -void byte_string_callback(void* context, cbor_data address, size_t length) { +void byte_string_callback(void *context, cbor_data address, size_t length) { assert_true(current().expectation == BSTRING_MEM_EQ); assert_true(current().data.string.address == address); assert_true(current().data.string.length == length); @@ -121,7 +134,7 @@ void assert_bstring_indef_start() { (struct test_assertion){.expectation = BSTRING_INDEF_START}; } -void byte_string_start_callback(void* context) { +void byte_string_start_callback(void *context) { assert_true(current().expectation == BSTRING_INDEF_START); current_expectation++; } @@ -131,7 +144,7 @@ void assert_indef_break() { (struct test_assertion){.expectation = INDEF_BREAK}; } -void indef_break_callback(void* context) { +void indef_break_callback(void *context) { assert_true(current().expectation == INDEF_BREAK); current_expectation++; } @@ -141,7 +154,7 @@ void assert_array_start(size_t length) { (struct test_assertion){ARRAY_START, {.length = length}}; } -void array_start_callback(void* context, size_t length) { +void array_start_callback(void *context, size_t length) { assert_true(current().expectation == ARRAY_START); assert_true(current().data.length == length); current_expectation++; @@ -152,7 +165,7 @@ void assert_indef_array_start() { (struct test_assertion){.expectation = ARRAY_INDEF_START}; } -void indef_array_start_callback(void* context) { +void indef_array_start_callback(void *context) { assert_true(current().expectation == ARRAY_INDEF_START); current_expectation++; } @@ -162,7 +175,7 @@ void assert_map_start(size_t length) { (struct test_assertion){MAP_START, {.length = length}}; } -void map_start_callback(void* context, size_t length) { +void map_start_callback(void *context, size_t length) { assert_true(current().expectation == MAP_START); assert_true(current().data.length == length); current_expectation++; @@ -173,7 +186,7 @@ void assert_indef_map_start() { (struct test_assertion){.expectation = MAP_INDEF_START}; } -void indef_map_start_callback(void* context) { +void indef_map_start_callback(void *context) { assert_true(current().expectation == MAP_INDEF_START); current_expectation++; } @@ -183,7 +196,7 @@ void assert_tag_eq(uint64_t value) { (struct test_assertion){TAG_EQ, {.int64 = value}}; } -void tag_callback(void* context, uint64_t value) { +void tag_callback(void *context, uint64_t value) { assert_true(current().expectation == TAG_EQ); assert_true(current().data.int64 == value); current_expectation++; @@ -194,7 +207,7 @@ void assert_half(float value) { (struct test_assertion){HALF_EQ, {.float2 = value}}; } -void half_callback(void* context, float actual) { +void half_callback(void *context, float actual) { assert_true(current().expectation == HALF_EQ); assert_true(current().data.float2 == actual); current_expectation++; @@ -205,7 +218,7 @@ void assert_float(float value) { (struct test_assertion){FLOAT_EQ, {.float4 = value}}; } -void float_callback(void* context, float actual) { +void float_callback(void *context, float actual) { assert_true(current().expectation == FLOAT_EQ); assert_true(current().data.float4 == actual); current_expectation++; @@ -216,7 +229,7 @@ void assert_double(double value) { (struct test_assertion){DOUBLE_EQ, {.float8 = value}}; } -void double_callback(void* context, double actual) { +void double_callback(void *context, double actual) { assert_true(current().expectation == DOUBLE_EQ); assert_true(current().data.float8 == actual); current_expectation++; @@ -236,18 +249,18 @@ void assert_undef() { (struct test_assertion){.expectation = UNDEF}; } -void bool_callback(void* context, bool actual) { +void bool_callback(void *context, bool actual) { assert_true(current().expectation == BOOL_EQ); assert_true(current().data.boolean == actual); current_expectation++; } -void null_callback(void* context) { +void null_callback(void *context) { assert_true(current().expectation == NIL); current_expectation++; } -void undef_callback(void* context) { +void undef_callback(void *context) { assert_true(current().expectation == UNDEF); current_expectation++; } @@ -293,12 +306,12 @@ const struct cbor_callbacks asserting_callbacks = { .indef_break = &indef_break_callback}; struct cbor_decoder_result decode(cbor_data source, size_t source_size) { + int last_expectation = current_expectation; struct cbor_decoder_result result = decoder(source, source_size, &asserting_callbacks, NULL); - /* Check remaining assertions */ - - assert_true(current_expectation == queue_size); - /* Clean up */ - current_expectation = queue_size = 0; + if (result.status == CBOR_DECODER_FINISHED) { + // Check that we have matched an expectation from the queue + assert_true(last_expectation + 1 == current_expectation); + } return result; } diff --git a/test/stream_expectations.c.erb b/test/stream_expectations.c.erb index 47baee06..88ab40f3 100644 --- a/test/stream_expectations.c.erb +++ b/test/stream_expectations.c.erb @@ -1,9 +1,13 @@ #include "stream_expectations.h" +// TODO: The saved keystrokes are not worth the complexity. Get rid of this +// file to prevent confusion, the fundamental structure is unlikely to change +// in the future. + /* Ordered from 0 to queue_size - 1 */ struct test_assertion assertions_queue[MAX_QUEUE_ITEMS]; -size_t queue_size = 0; -size_t current_expectation = 0; +int queue_size = 0; +int current_expectation = 0; decoder_t * decoder; @@ -12,6 +16,15 @@ void set_decoder(decoder_t * dec) decoder = dec; } +int clear_stream_assertions(void **state) { + if (queue_size != current_expectation) { + return 1; // We have not matched all expectations correctly + } + queue_size = current_expectation = 0; + free(*state); + return 0; +} + /* Callbacks */ struct test_assertion current() { @@ -187,12 +200,12 @@ const struct cbor_callbacks asserting_callbacks = { struct cbor_decoder_result decode(cbor_data source, size_t source_size) { + int last_expectation = current_expectation; struct cbor_decoder_result result = decoder(source, source_size, &asserting_callbacks, NULL); - /* Check remaining assertions */ - - assert_true(current_expectation == queue_size); - /* Clean up */ - current_expectation = queue_size = 0; + if (result.status == CBOR_DECODER_FINISHED) { + // Check that we have matched an expectation from the queue + assert_true(last_expectation + 1 == current_expectation); + } return result; } diff --git a/test/stream_expectations.h b/test/stream_expectations.h index 7495b08c..84970ae9 100644 --- a/test/stream_expectations.h +++ b/test/stream_expectations.h @@ -17,6 +17,8 @@ #include #include "cbor.h" +// TODO: This setup is overengineered, we currently use one assertion at a time +// TOOD: We never ensure that the queue is empty #define MAX_QUEUE_ITEMS 30 enum test_expectation { @@ -80,6 +82,9 @@ typedef struct cbor_decoder_result decoder_t(cbor_data, size_t, void set_decoder(decoder_t *); struct cbor_decoder_result decode(cbor_data, size_t); +/* Test setup */ +int clear_stream_assertions(void **); + /* Assertions builders */ void assert_uint8_eq(uint8_t); void assert_uint16_eq(uint16_t); From 5da72a7cbfc9d388f6f22aabfefda2bc53ce56c8 Mon Sep 17 00:00:00 2001 From: Pavel Kalvoda Date: Sun, 6 Sep 2020 13:19:47 +0200 Subject: [PATCH 4/7] Add inline docs for asserts --- test/assertions.h | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/test/assertions.h b/test/assertions.h index b52d2dce..7a36a4c5 100644 --- a/test/assertions.h +++ b/test/assertions.h @@ -14,14 +14,19 @@ void assert_uint16(cbor_item_t* item, uint16_t num); void assert_uint32(cbor_item_t* item, uint32_t num); void assert_uint64(cbor_item_t* item, uint64_t num); -void assert_decoder_result(size_t, enum cbor_decoder_status, - struct cbor_decoder_result); +/** Assert that result `status` and `read` are equal. */ +void assert_decoder_result(size_t read, enum cbor_decoder_status status, + struct cbor_decoder_result result); -// TODO: Docs -void assert_decoder_result_nedata(size_t, struct cbor_decoder_result); +/** + * Assert that the result is set to CBOR_DECODER_NEDATA with the given + * `cbor_decoder_result.required` value. + */ +void assert_decoder_result_nedata(size_t required, + struct cbor_decoder_result result); /** - * Check that the streaming decoder will returns a correct CBOR_DECODER_NEDATA + * Check that the streaming decoder returns a correct CBOR_DECODER_NEDATA * result for all inputs from data[0..1] through data[0..(expected-1)]. */ void assert_minimum_input_size(size_t expected, cbor_data data); From 15e70662223e8226045d07d1b3fb5375c936dadc Mon Sep 17 00:00:00 2001 From: Pavel Kalvoda Date: Sun, 6 Sep 2020 13:55:26 +0200 Subject: [PATCH 5/7] Add changelog note --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b2c4a64..4219e319 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,9 @@ Next - BUILD BREAKING: Use BUILD_SHARED_LIBS to determine how to build libraries (fixed Windows linkage) [[#148]](https://github.com/PJK/libcbor/pull/148) (by [intelligide@](https://github.com/intelligide)) - BREAKING: Fix `cbor_tag_item` not increasing the reference count on the tagged item reference it returns [[Fixes #109](https://github.com/PJK/libcbor/issues/109)] (discovered bt [JohnGilmour](https://github.com/JohnGilmour)) - If you have previously relied on the broken behavior, you can use `cbor_move` to emulate as long as the returned handle is an "rvalue" +- BREAKING: [`CBOR_DECODER_EBUFFER` removed from `cbor_decoder_status`](https://github.com/PJK/libcbor/pull/156) + - `cbor_stream_decode` will set `CBOR_DECODER_NEDATA` instead if the input buffer is empty +- [Fix `cbor_stream_decode`](https://github.com/PJK/libcbor/pull/156) to set `cbor_decoder_result.required` to the minimum number of input bytes necessary to receive the next callback (as long as at least one byte was passed) (discovered by [woefulwabbit](https://github.com/woefulwabbit)) 0.7.0 (2020-04-25) --------------------- From 26f54aa1bf4ce4a47ac0f923cbcbd0dcf945c7b7 Mon Sep 17 00:00:00 2001 From: Pavel Kalvoda Date: Sun, 6 Sep 2020 14:21:20 +0200 Subject: [PATCH 6/7] Improve docs --- src/cbor/data.h | 45 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/src/cbor/data.h b/src/cbor/data.h index 0934b845..982e6a2c 100644 --- a/src/cbor/data.h +++ b/src/cbor/data.h @@ -199,27 +199,52 @@ struct cbor_pair { struct cbor_load_result { /** Error indicator */ struct cbor_error error; - /** Number of bytes read*/ + /** Number of bytes read */ size_t read; }; /** Streaming decoder result - status */ enum cbor_decoder_status { - CBOR_DECODER_FINISHED /** OK, finished */ - , - CBOR_DECODER_NEDATA /** Not enough data - mismatch with MTB */ - , - CBOR_DECODER_ERROR /** Malformed or reserved MTB/value */ + /** Decoding finished successfully (a callback has been invoked) + * + * Note that this does *not* mean that the buffer has been fully decoded; + * there may still be unread bytes for which no callback has been involved. + */ + CBOR_DECODER_FINISHED, + /** Not enough data to invoke a callback */ + CBOR_DECODER_NEDATA, + /** Bad data (reserved MTB, malformed value, etc.) */ + CBOR_DECODER_ERROR }; /** Streaming decoder result */ struct cbor_decoder_result { - /** Bytes read */ + /** Input bytes read/consumed + * + * If this is less than the size of input buffer, the client will likely + * resume parsing starting at the next byte (e.g. `buffer + result.read`). + * + * Set to 0 if the #status is not #CBOR_DECODER_FINISHED. + */ size_t read; - /** The result */ + + /** The decoding status */ enum cbor_decoder_status status; - /** When status == CBOR_DECODER_NEDATA, - * the minimum number of bytes required to continue parsing */ + + /** Number of bytes in the input buffer needed to resume parsing + * + * Set to 0 unless the result status is #CBOR_DECODER_NEDATA. If it is, then: + * - If at least one byte was passed, #required will be set to the minimum + * number of bytes needed to invoke a decoded callback on the current + * prefix. + * + * For example: Attempting to decode a 1B buffer containing `0x19` will + * set #required to 3 as `0x19` signals a 2B integer item, so we need at + * least 3B to continue (the `0x19` MTB byte and two bytes of data needed + * to invoke #cbor_callbacks.uint16). + * + * - If there was no data at all, #read will always be set to 1 + */ size_t required; }; From b3ce6bbac1df7214826163ba7672fd9cceeb5dba Mon Sep 17 00:00:00 2001 From: Pavel Kalvoda Date: Sun, 6 Sep 2020 14:24:20 +0200 Subject: [PATCH 7/7] Rename claim_bytes (internal helper) --- src/cbor/streaming.c | 88 ++++++++++++++++++++++---------------------- 1 file changed, 44 insertions(+), 44 deletions(-) diff --git a/src/cbor/streaming.c b/src/cbor/streaming.c index ae131999..aa58abb3 100644 --- a/src/cbor/streaming.c +++ b/src/cbor/streaming.c @@ -8,8 +8,8 @@ #include "streaming.h" #include "internal/loaders.h" -bool static cbor_claim_bytes(size_t required, size_t provided, - struct cbor_decoder_result *result) { +bool static claim_bytes(size_t required, size_t provided, + struct cbor_decoder_result *result) { if (required > (provided - result->read)) { result->required = required + result->read; result->read = 0; @@ -27,7 +27,7 @@ struct cbor_decoder_result cbor_stream_decode( const struct cbor_callbacks *callbacks, void *context) { // Attempt to claim the initial MTB byte struct cbor_decoder_result result = {.status = CBOR_DECODER_FINISHED}; - if (!cbor_claim_bytes(1, source_size, &result)) { + if (!claim_bytes(1, source_size, &result)) { return result; } @@ -64,7 +64,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x18: /* One byte unsigned integer */ { - if (cbor_claim_bytes(1, source_size, &result)) { + if (claim_bytes(1, source_size, &result)) { callbacks->uint8(context, _cbor_load_uint8(source + 1)); } return result; @@ -72,7 +72,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x19: /* Two bytes unsigned integer */ { - if (cbor_claim_bytes(2, source_size, &result)) { + if (claim_bytes(2, source_size, &result)) { callbacks->uint16(context, _cbor_load_uint16(source + 1)); } return result; @@ -80,7 +80,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x1A: /* Four bytes unsigned integer */ { - if (cbor_claim_bytes(4, source_size, &result)) { + if (claim_bytes(4, source_size, &result)) { callbacks->uint32(context, _cbor_load_uint32(source + 1)); } return result; @@ -88,7 +88,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x1B: /* Eight bytes unsigned integer */ { - if (cbor_claim_bytes(8, source_size, &result)) { + if (claim_bytes(8, source_size, &result)) { callbacks->uint64(context, _cbor_load_uint64(source + 1)); } return result; @@ -132,7 +132,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x38: /* One byte negative integer */ { - if (cbor_claim_bytes(1, source_size, &result)) { + if (claim_bytes(1, source_size, &result)) { callbacks->negint8(context, _cbor_load_uint8(source + 1)); } return result; @@ -140,7 +140,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x39: /* Two bytes negative integer */ { - if (cbor_claim_bytes(2, source_size, &result)) { + if (claim_bytes(2, source_size, &result)) { callbacks->negint16(context, _cbor_load_uint16(source + 1)); } return result; @@ -148,7 +148,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x3A: /* Four bytes negative integer */ { - if (cbor_claim_bytes(4, source_size, &result)) { + if (claim_bytes(4, source_size, &result)) { callbacks->negint32(context, _cbor_load_uint32(source + 1)); } return result; @@ -156,7 +156,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x3B: /* Eight bytes negative integer */ { - if (cbor_claim_bytes(8, source_size, &result)) { + if (claim_bytes(8, source_size, &result)) { callbacks->negint64(context, _cbor_load_uint64(source + 1)); } return result; @@ -195,7 +195,7 @@ struct cbor_decoder_result cbor_stream_decode( { size_t length = (size_t)_cbor_load_uint8(source) - 0x40; /* 0x40 offset */ - if (cbor_claim_bytes(length, source_size, &result)) { + if (claim_bytes(length, source_size, &result)) { callbacks->byte_string(context, source + 1, length); } return result; @@ -204,9 +204,9 @@ struct cbor_decoder_result cbor_stream_decode( /* One byte length byte string */ // TODO template this? { - if (cbor_claim_bytes(1, source_size, &result)) { + if (claim_bytes(1, source_size, &result)) { size_t length = (size_t)_cbor_load_uint8(source + 1); - if (cbor_claim_bytes(length, source_size, &result)) { + if (claim_bytes(length, source_size, &result)) { callbacks->byte_string(context, source + 1 + 1, length); } } @@ -215,9 +215,9 @@ struct cbor_decoder_result cbor_stream_decode( case 0x59: /* Two bytes length byte string */ { - if (cbor_claim_bytes(2, source_size, &result)) { + if (claim_bytes(2, source_size, &result)) { size_t length = (size_t)_cbor_load_uint16(source + 1); - if (cbor_claim_bytes(length, source_size, &result)) { + if (claim_bytes(length, source_size, &result)) { callbacks->byte_string(context, source + 1 + 2, length); } } @@ -226,9 +226,9 @@ struct cbor_decoder_result cbor_stream_decode( case 0x5A: /* Four bytes length byte string */ { - if (cbor_claim_bytes(4, source_size, &result)) { + if (claim_bytes(4, source_size, &result)) { size_t length = (size_t)_cbor_load_uint32(source + 1); - if (cbor_claim_bytes(length, source_size, &result)) { + if (claim_bytes(length, source_size, &result)) { callbacks->byte_string(context, source + 1 + 4, length); } } @@ -237,9 +237,9 @@ struct cbor_decoder_result cbor_stream_decode( case 0x5B: /* Eight bytes length byte string */ { - if (cbor_claim_bytes(8, source_size, &result)) { + if (claim_bytes(8, source_size, &result)) { size_t length = (size_t)_cbor_load_uint64(source + 1); - if (cbor_claim_bytes(length, source_size, &result)) { + if (claim_bytes(length, source_size, &result)) { callbacks->byte_string(context, source + 1 + 8, length); } } @@ -284,7 +284,7 @@ struct cbor_decoder_result cbor_stream_decode( { size_t length = (size_t)_cbor_load_uint8(source) - 0x60; /* 0x60 offset */ - if (cbor_claim_bytes(length, source_size, &result)) { + if (claim_bytes(length, source_size, &result)) { callbacks->string(context, source + 1, length); } return result; @@ -292,9 +292,9 @@ struct cbor_decoder_result cbor_stream_decode( case 0x78: /* One byte length string */ { - if (cbor_claim_bytes(1, source_size, &result)) { + if (claim_bytes(1, source_size, &result)) { size_t length = (size_t)_cbor_load_uint8(source + 1); - if (cbor_claim_bytes(length, source_size, &result)) { + if (claim_bytes(length, source_size, &result)) { callbacks->string(context, source + 1 + 1, length); } } @@ -303,9 +303,9 @@ struct cbor_decoder_result cbor_stream_decode( case 0x79: /* Two bytes length string */ { - if (cbor_claim_bytes(2, source_size, &result)) { + if (claim_bytes(2, source_size, &result)) { size_t length = (size_t)_cbor_load_uint16(source + 1); - if (cbor_claim_bytes(length, source_size, &result)) { + if (claim_bytes(length, source_size, &result)) { callbacks->string(context, source + 1 + 2, length); } } @@ -314,9 +314,9 @@ struct cbor_decoder_result cbor_stream_decode( case 0x7A: /* Four bytes length string */ { - if (cbor_claim_bytes(4, source_size, &result)) { + if (claim_bytes(4, source_size, &result)) { size_t length = (size_t)_cbor_load_uint32(source + 1); - if (cbor_claim_bytes(length, source_size, &result)) { + if (claim_bytes(length, source_size, &result)) { callbacks->string(context, source + 1 + 4, length); } } @@ -325,9 +325,9 @@ struct cbor_decoder_result cbor_stream_decode( case 0x7B: /* Eight bytes length string */ { - if (cbor_claim_bytes(8, source_size, &result)) { + if (claim_bytes(8, source_size, &result)) { size_t length = (size_t)_cbor_load_uint64(source + 1); - if (cbor_claim_bytes(length, source_size, &result)) { + if (claim_bytes(length, source_size, &result)) { callbacks->string(context, source + 1 + 8, length); } } @@ -377,7 +377,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x98: /* One byte length array */ { - if (cbor_claim_bytes(1, source_size, &result)) { + if (claim_bytes(1, source_size, &result)) { callbacks->array_start(context, (size_t)_cbor_load_uint8(source + 1)); } return result; @@ -385,7 +385,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x99: /* Two bytes length array */ { - if (cbor_claim_bytes(2, source_size, &result)) { + if (claim_bytes(2, source_size, &result)) { callbacks->array_start(context, (size_t)_cbor_load_uint16(source + 1)); } @@ -394,7 +394,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x9A: /* Four bytes length array */ { - if (cbor_claim_bytes(4, source_size, &result)) { + if (claim_bytes(4, source_size, &result)) { callbacks->array_start(context, (size_t)_cbor_load_uint32(source + 1)); } @@ -403,7 +403,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0x9B: /* Eight bytes length array */ { - if (cbor_claim_bytes(8, source_size, &result)) { + if (claim_bytes(8, source_size, &result)) { callbacks->array_start(context, (size_t)_cbor_load_uint64(source + 1)); } @@ -453,7 +453,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0xB8: /* One byte length map */ { - if (cbor_claim_bytes(1, source_size, &result)) { + if (claim_bytes(1, source_size, &result)) { callbacks->map_start(context, (size_t)_cbor_load_uint8(source + 1)); } return result; @@ -461,7 +461,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0xB9: /* Two bytes length map */ { - if (cbor_claim_bytes(2, source_size, &result)) { + if (claim_bytes(2, source_size, &result)) { callbacks->map_start(context, (size_t)_cbor_load_uint16(source + 1)); } return result; @@ -469,7 +469,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0xBA: /* Four bytes length map */ { - if (cbor_claim_bytes(4, source_size, &result)) { + if (claim_bytes(4, source_size, &result)) { callbacks->map_start(context, (size_t)_cbor_load_uint32(source + 1)); } return result; @@ -477,7 +477,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0xBB: /* Eight bytes length map */ { - if (cbor_claim_bytes(8, source_size, &result)) { + if (claim_bytes(8, source_size, &result)) { callbacks->map_start(context, (size_t)_cbor_load_uint64(source + 1)); } return result; @@ -538,28 +538,28 @@ struct cbor_decoder_result cbor_stream_decode( } case 0xD8: /* 1B tag */ { - if (cbor_claim_bytes(1, source_size, &result)) { + if (claim_bytes(1, source_size, &result)) { callbacks->tag(context, _cbor_load_uint8(source + 1)); } return result; } case 0xD9: /* 2B tag */ { - if (cbor_claim_bytes(2, source_size, &result)) { + if (claim_bytes(2, source_size, &result)) { callbacks->tag(context, _cbor_load_uint16(source + 1)); } return result; } case 0xDA: /* 4B tag */ { - if (cbor_claim_bytes(4, source_size, &result)) { + if (claim_bytes(4, source_size, &result)) { callbacks->tag(context, _cbor_load_uint32(source + 1)); } return result; } case 0xDB: /* 8B tag */ { - if (cbor_claim_bytes(8, source_size, &result)) { + if (claim_bytes(8, source_size, &result)) { callbacks->tag(context, _cbor_load_uint64(source + 1)); } return result; @@ -624,7 +624,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0xF9: /* 2B float */ { - if (cbor_claim_bytes(2, source_size, &result)) { + if (claim_bytes(2, source_size, &result)) { callbacks->float2(context, _cbor_load_half(source + 1)); } return result; @@ -632,7 +632,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0xFA: /* 4B float */ { - if (cbor_claim_bytes(4, source_size, &result)) { + if (claim_bytes(4, source_size, &result)) { callbacks->float4(context, _cbor_load_float(source + 1)); } return result; @@ -640,7 +640,7 @@ struct cbor_decoder_result cbor_stream_decode( case 0xFB: /* 8B float */ { - if (cbor_claim_bytes(8, source_size, &result)) { + if (claim_bytes(8, source_size, &result)) { callbacks->float8(context, _cbor_load_double(source + 1)); } return result;