diff --git a/src/thingset.c b/src/thingset.c index dad32ce..417453b 100644 --- a/src/thingset.c +++ b/src/thingset.c @@ -13,6 +13,7 @@ #include #include +#include LOG_MODULE_REGISTER(thingset, CONFIG_THINGSET_LOG_LEVEL); @@ -194,7 +195,8 @@ int thingset_export_subsets(struct thingset_context *ts, uint8_t *buf, size_t bu thingset_bin_setup(ts, 0); break; default: - return -THINGSET_ERR_NOT_IMPLEMENTED; + ret = -THINGSET_ERR_NOT_IMPLEMENTED; + goto out; } ret = ts->api->serialize_subsets(ts, subsets); @@ -205,6 +207,7 @@ int thingset_export_subsets(struct thingset_context *ts, uint8_t *buf, size_t bu ret = ts->rsp_pos; } +out: k_sem_give(&ts->lock); return ret; @@ -415,6 +418,14 @@ int thingset_import_record(struct thingset_context *ts, const uint8_t *data, siz ts->endpoint = *endpoint; + if (ts->endpoint.object == NULL || ts->endpoint.object->type != THINGSET_TYPE_RECORDS + || ts->endpoint.index < 0 + || ts->endpoint.index >= ts->endpoint.object->data.records->num_records) + { + err = -THINGSET_ERR_NOT_FOUND; + goto out; + } + switch (format) { #ifdef CONFIG_THINGSET_TEXT_MODE case THINGSET_TXT_NAMES_VALUES: @@ -439,6 +450,12 @@ int thingset_import_record(struct thingset_context *ts, const uint8_t *data, siz goto out; } + /* dynamic records keep the data of only one record in memory */ + struct thingset_records *records = ts->endpoint.object->data.records; + size_t record_offset = ts->endpoint.object->detail == THINGSET_DETAIL_DYN_RECORDS + ? 0 + : ts->endpoint.index * records->record_size; + const struct thingset_data_object *item; while ((err = ts->api->deserialize_child(ts, &item)) != -THINGSET_ERR_DESERIALIZATION_FINISHED) { @@ -451,9 +468,7 @@ int thingset_import_record(struct thingset_context *ts, const uint8_t *data, siz goto out; } - struct thingset_records *records = ts->endpoint.object->data.records; - uint8_t *record_ptr = - (uint8_t *)records->records + ts->endpoint.index * records->record_size; + uint8_t *record_ptr = (uint8_t *)records->records + record_offset; err = thingset_common_prepare_record_element(ts, item, record_ptr, deserialize_value_callback); @@ -621,10 +636,18 @@ struct thingset_data_object *thingset_get_object_by_path(struct thingset_context const char *end; uint16_t parent = 0; + if (path_len == 0) { + return NULL; + } + /* maximum depth of 10 assumed */ for (int i = 0; i < 10; i++) { - end = strchr(start, '/'); - if (end == NULL || end >= path + path_len) { + /* + * The path may point directly into a received message and is not null-terminated, so the + * search for the separator has to be limited to the remaining length. + */ + end = memchr(start, '/', path + path_len - start); + if (end == NULL) { /* reached at the end of the path */ if (object != NULL && object->type == THINGSET_TYPE_RECORDS && *start >= '0' && *start <= '9') @@ -639,6 +662,10 @@ struct thingset_data_object *thingset_get_object_by_path(struct thingset_context do { if (*start >= '0' && *start <= '9') { *index = (*index) * 10 + *start - '0'; + if (*index > UINT16_MAX) { + /* record indices are limited to the range of uint16_t */ + return NULL; + } start++; } else { diff --git a/src/thingset_bin.c b/src/thingset_bin.c index 0a8c401..9d26eac 100644 --- a/src/thingset_bin.c +++ b/src/thingset_bin.c @@ -113,8 +113,8 @@ static int bin_serialize_simple_value(zcbor_state_t *encoder, union thingset_dat success = zcbor_int32_put(encoder, *data.i8); break; case THINGSET_TYPE_F32: - if (IS_ENABLED(CONFIG_THINGSET_ENCODE_ZERO_DECIMAL_FLOATS_AS_INTEGERS) && detail == 0) - { /* round to 0 decimals: use int */ + if (IS_ENABLED(CONFIG_THINGSET_ENCODE_ZERO_DECIMAL_FLOATS_AS_INTEGERS) && detail == 0) { + /* round to 0 decimals: use int */ success = zcbor_int32_put(encoder, lroundf(*data.f32)); } else { @@ -710,10 +710,11 @@ static int bin_deserialize_value(struct thingset_context *ts, int index = 0; do { /* using uint8_t pointer for byte-wise pointer arithmetics */ - union thingset_data_pointer data = { .u8 = array->elements.u8 + index * type_size }; + union thingset_data_pointer data = { .u8 = array->elements.u8 + + index * type_size }; - err = bin_deserialize_simple_value(ts, data, array->element_type, array->decimals, - check_only); + err = bin_deserialize_simple_value(ts, data, array->element_type, + array->decimals, check_only); if (err != 0) { break; } @@ -744,15 +745,16 @@ static int bin_deserialize_value(struct thingset_context *ts, while (zcbor_uint32_decode(ts->decoder, &id) && id < UINT16_MAX) { struct thingset_data_object *element = thingset_get_object_by_id(ts, id); - if (element == NULL) { + /* only items belonging to this record may be used */ + if (element == NULL || element->parent_id != object->id) { zcbor_any_skip(ts->decoder, NULL); continue; } union thingset_data_pointer data = { .u8 = ((uint8_t *)records->records) - + (i * records->record_size) - + element->data.offset }; + + (i * records->record_size) + + element->data.offset }; err = bin_deserialize_simple_value(ts, data, element->type, element->detail, - check_only); + check_only); } success = zcbor_map_end_decode(ts->decoder); diff --git a/src/thingset_common.c b/src/thingset_common.c index 1da132f..bdcf410 100644 --- a/src/thingset_common.c +++ b/src/thingset_common.c @@ -111,7 +111,7 @@ int thingset_common_serialize_record(struct thingset_context *ts, size_t record_offset; int err; - if (record_index >= records->num_records) { + if (record_index < 0 || record_index >= records->num_records) { return -THINGSET_ERR_NOT_FOUND; } @@ -328,10 +328,12 @@ int thingset_common_update(struct thingset_context *ts) /* * Test format of simple data types (up to 64-bit) by deserializing the value into a dummy * object of the same type. For string and byte buffers only the size of the buffers is - * checked. + * checked. Types described by a struct don't fit into the dummy buffer, so the data + * pointer of the actual object is used for them. */ uint8_t dummy_data[8]; uint8_t *data = object->type == THINGSET_TYPE_BYTES || object->type == THINGSET_TYPE_ARRAY + || object->type == THINGSET_TYPE_RECORDS ? object->data.u8 : dummy_data; struct thingset_data_object dummy_object = { diff --git a/src/thingset_txt.c b/src/thingset_txt.c index 6cae964..f7ededa 100644 --- a/src/thingset_txt.c +++ b/src/thingset_txt.c @@ -100,6 +100,36 @@ static int txt_serialize_response(struct thingset_context *ts, uint8_t code, con return 0; } +/** + * Append a formatted string at the given position of the buffer. + * + * A negative position is passed through unchanged, so that errors propagate through consecutive + * calls and only have to be checked once at the end. + * + * @returns New position or negative ThingSet reponse code in case of error + */ +static int json_append(char *buf, size_t size, int pos, const char *format, ...) +{ + va_list vargs; + + if (pos < 0) { + return pos; + } + else if ((size_t)pos >= size) { + return -THINGSET_ERR_RESPONSE_TOO_LARGE; + } + + va_start(vargs, format); + int len = vsnprintf(buf + pos, size - pos, format, vargs); + va_end(vargs); + + if (len < 0 || (size_t)len >= size - pos) { + return -THINGSET_ERR_RESPONSE_TOO_LARGE; + } + + return pos + len; +} + /** * @returns Number of serialized bytes or negative ThingSet reponse code in case of error */ @@ -156,6 +186,9 @@ static int json_serialize_simple_value(char *buf, size_t size, union thingset_da break; case THINGSET_TYPE_STRING: #if CONFIG_THINGSET_JSON_STRING_ESCAPING + if (size < 1) { + return -THINGSET_ERR_RESPONSE_TOO_LARGE; + } buf[0] = '"'; pos = 1; for (int data_pos = 0; data_pos < detail; data_pos++) { @@ -203,8 +236,14 @@ static int json_serialize_simple_value(char *buf, size_t size, union thingset_da buf[pos++] = data.str[data_pos]; } } - buf[pos++] = '"'; - buf[pos++] = ','; + if ((size_t)pos + 2 < size) { + buf[pos++] = '"'; + buf[pos++] = ','; + } + else { + /* indicate that the buffer is too small (similar to snprintf) */ + pos += 2; + } #else pos = snprintf(buf, size, "\"%s\",", data.str); #endif @@ -212,6 +251,9 @@ static int json_serialize_simple_value(char *buf, size_t size, union thingset_da #if CONFIG_THINGSET_BYTES_TYPE_SUPPORT case THINGSET_TYPE_BYTES: { size_t strlen; + if (size < 4) { + return -THINGSET_ERR_RESPONSE_TOO_LARGE; + } int err = base64_encode((uint8_t *)buf + 1, size - 4, &strlen, data.bytes->bytes, data.bytes->num_bytes); if (err == 0) { @@ -251,79 +293,88 @@ static int txt_serialize_value(struct thingset_context *ts, if (pos == -THINGSET_ERR_UNSUPPORTED_FORMAT) { /* not a simple value */ if (object->type == THINGSET_TYPE_GROUP) { - pos = snprintf(buf, size, "null,"); + pos = json_append(buf, size, 0, "null,"); } else if (object->type == THINGSET_TYPE_RECORDS) { if (IS_ENABLED(CONFIG_THINGSET_REPORT_RECORD_SERIALIZATION) && ts->rsp[0] == THINGSET_TXT_REPORT) { - pos = snprintf(buf, size, "["); - ts->rsp_pos++; + pos = json_append(buf, size, 0, "["); + if (pos < 0) { + ts->rsp_pos = 0; + return pos; + } + ts->rsp_pos += pos; + + /* the records are serialized directly into the response buffer, each of them + already terminated with a comma by the map end */ for (unsigned int i = 0; i < object->data.records->num_records; i++) { ret = thingset_common_serialize_record(ts, object, i); - pos = ts->rsp_pos; - buf[pos++] = ','; + if (ret != 0) { + ts->rsp_pos = 0; + return ret; + } } - /* thingset_common_serialize_record has already incremeneted rsp_pos, so we reset - the length of pos here, so that when we increment at the end, we are only - incrementing the small delta below */ - buf = ts->rsp + ts->rsp_pos; - pos = 0; + if (object->data.records->num_records > 0) { - pos--; /* remove trailing comma */ + ts->rsp_pos--; /* overwrite trailing comma */ } - pos += snprintf(buf + pos, size - pos, "],"); + buf = ts->rsp + ts->rsp_pos; + size = ts->rsp_size - ts->rsp_pos; + pos = json_append(buf, size, 0, "],"); } else { - pos = snprintf(buf, size, "%d,", object->data.records->num_records); + pos = json_append(buf, size, 0, "%d,", object->data.records->num_records); } } else if (object->type == THINGSET_TYPE_FN_VOID || object->type == THINGSET_TYPE_FN_I32) { - pos = snprintf(buf, size, "["); + pos = json_append(buf, size, 0, "["); for (unsigned int i = 0; i < ts->num_objects; i++) { if (ts->data_objects[i].parent_id == object->id) { - pos += snprintf(buf + pos, size - pos, "\"%s\",", ts->data_objects[i].name); + pos = json_append(buf, size, pos, "\"%s\",", ts->data_objects[i].name); } } if (pos > 1) { pos--; /* remove trailing comma */ } - pos += snprintf(buf + pos, size - pos, "],"); + pos = json_append(buf, size, pos, "],"); } else if (object->type == THINGSET_TYPE_SUBSET) { - pos = snprintf(buf, size, "["); + pos = json_append(buf, size, 0, "["); for (unsigned int i = 0; i < ts->num_objects; i++) { if (ts->data_objects[i].subsets & object->data.subset) { - buf[pos++] = '"'; + pos = json_append(buf, size, pos, "\""); + if (pos < 0) { + break; + } ret = thingset_get_path(ts, buf + pos, size - pos, &ts->data_objects[i]); if (ret <= 0) { ts->rsp_pos = 0; return ret; } - pos += ret; - buf[pos++] = '"'; - buf[pos++] = ','; + pos = json_append(buf, size, pos + ret, "\","); } } if (pos > 1) { pos--; /* remove trailing comma */ } - pos += snprintf(buf + pos, size - pos, "],"); + pos = json_append(buf, size, pos, "],"); } else if (object->type == THINGSET_TYPE_ARRAY && object->data.array != NULL) { struct thingset_array *array = object->data.array; - pos = snprintf(buf, size, "["); + pos = json_append(buf, size, 0, "["); size_t type_size = thingset_type_size(array->element_type); - for (int i = 0; i < array->num_elements; i++) { + for (int i = 0; i < array->num_elements && pos >= 0; i++) { /* using uint8_t pointer for byte-wise pointer arithmetics */ union thingset_data_pointer data = { .u8 = array->elements.u8 + i * type_size }; - pos += json_serialize_simple_value(buf + pos, size - pos, data, array->element_type, - array->decimals); + ret = json_serialize_simple_value(buf + pos, size - pos, data, array->element_type, + array->decimals); + pos = ret < 0 ? ret : pos + ret; } - if (array->num_elements > 0) { + if (pos > 1) { pos--; /* remove trailing comma */ } - pos += snprintf(buf + pos, size - pos, "],"); + pos = json_append(buf, size, pos, "],"); } else { ts->rsp_pos = 0; @@ -424,8 +475,12 @@ static int txt_serialize_name_value(struct thingset_context *ts, static void txt_serialize_finish(struct thingset_context *ts) { /* remove the trailing comma or space (in case of no payload) and terminate string */ - ts->rsp_pos--; - ts->rsp[ts->rsp_pos] = '\0'; + if (ts->rsp_pos > 0) { + ts->rsp_pos--; + } + if (ts->rsp_pos < ts->rsp_size) { + ts->rsp[ts->rsp_pos] = '\0'; + } } /** @@ -697,8 +752,13 @@ static int txt_serialize_subsets(struct thingset_context *ts, uint16_t subsets) { struct thingset_data_object *ancestors[2]; int depth = 0; + int pos; - ts->rsp[ts->rsp_pos++] = '{'; + pos = json_append(ts->rsp, ts->rsp_size, ts->rsp_pos, "{"); + if (pos < 0) { + return pos; + } + ts->rsp_pos = pos; for (unsigned int i = 0; i < ts->num_objects; i++) { if (ts->data_objects[i].subsets & subsets) { @@ -720,7 +780,11 @@ static int txt_serialize_subsets(struct thingset_context *ts, uint16_t subsets) || parent_id == 0)) /* return to root */ { ts->rsp[ts->rsp_pos - 1] = '}'; /* overwrite comma */ - ts->rsp[ts->rsp_pos++] = ','; + pos = json_append(ts->rsp, ts->rsp_size, ts->rsp_pos, ","); + if (pos < 0) { + return pos; + } + ts->rsp_pos = pos; depth--; } @@ -729,25 +793,40 @@ static int txt_serialize_subsets(struct thingset_context *ts, uint16_t subsets) struct thingset_data_object *grandparent = thingset_get_object_by_id(ts, parent->parent_id); if (grandparent != NULL) { - ts->rsp_pos += snprintf(ts->rsp + ts->rsp_pos, ts->rsp_size - ts->rsp_pos, - "\"%s\":{", grandparent->name); + pos = json_append(ts->rsp, ts->rsp_size, ts->rsp_pos, "\"%s\":{", + grandparent->name); + if (pos < 0) { + return pos; + } + ts->rsp_pos = pos; ancestors[depth++] = grandparent; } } - ts->rsp_pos += snprintf(ts->rsp + ts->rsp_pos, ts->rsp_size - ts->rsp_pos, - "\"%s\":{", parent->name); + pos = json_append(ts->rsp, ts->rsp_size, ts->rsp_pos, "\"%s\":{", parent->name); + if (pos < 0) { + return pos; + } + ts->rsp_pos = pos; ancestors[depth++] = parent; } else if (depth > 0 && parent_id != ancestors[depth - 1]->id) { if (parent != NULL) { - ts->rsp_pos += snprintf(ts->rsp + ts->rsp_pos, ts->rsp_size - ts->rsp_pos, - "\"%s\":{", parent->name); + pos = json_append(ts->rsp, ts->rsp_size, ts->rsp_pos, "\"%s\":{", parent->name); + if (pos < 0) { + return pos; + } + ts->rsp_pos = pos; ancestors[depth++] = parent; } } - ts->rsp_pos += ts->api->serialize_key_value(ts, &ts->data_objects[i]); + + /* serialize_key_value advances rsp_pos itself */ + int err = ts->api->serialize_key_value(ts, &ts->data_objects[i]); + if (err != 0) { + return err; + } } - if (ts->rsp_pos >= ts->rsp_size - 1 - depth) { + if (ts->rsp_pos + 1 + depth >= ts->rsp_size) { return -THINGSET_ERR_RESPONSE_TOO_LARGE; } } @@ -766,11 +845,13 @@ static int txt_serialize_subsets(struct thingset_context *ts, uint16_t subsets) static int txt_serialize_report_header(struct thingset_context *ts, const char *path) { - ts->rsp_pos = snprintf(ts->rsp, ts->rsp_size, "#%s ", path); - if (ts->rsp_pos < 0 || ts->rsp_pos > ts->rsp_size) { - return -THINGSET_ERR_RESPONSE_TOO_LARGE; + int pos = json_append(ts->rsp, ts->rsp_size, 0, "#%s ", path); + if (pos < 0) { + ts->rsp_pos = 0; + return pos; } else { + ts->rsp_pos = pos; return 0; } } diff --git a/tests/common/data.c b/tests/common/data.c index dbfacd8..00f6e78 100644 --- a/tests/common/data.c +++ b/tests/common/data.c @@ -158,7 +158,7 @@ THINGSET_DEFINE_RECORD_FLOAT_ARRAY(f32_array_record, 1, struct test_struct, f32_ THINGSET_DEFINE_RECORD_RECORDS(nested_records_obj, struct test_struct, nested); /* Dynamic record */ -static struct test_dyn_struct dyn_records = { +struct test_dyn_struct dyn_records = { .index = 0, }; @@ -266,7 +266,8 @@ THINGSET_ADD_ITEM_FLOAT(0x500, 0x503, "wMfrOnly", &access_item, 2, THINGSET_ANY_ 0); /* Records */ -THINGSET_ADD_RECORDS(THINGSET_ID_ROOT, 0x600, "Records", &records_obj, THINGSET_ANY_R, SUBSET_LIVE); +THINGSET_ADD_RECORDS(THINGSET_ID_ROOT, 0x600, "Records", &records_obj, THINGSET_ANY_RW, + SUBSET_LIVE); THINGSET_ADD_RECORD_ITEM_UINT32(0x600, 0x601, "t_s", struct test_struct, timestamp); THINGSET_ADD_RECORD_ITEM_BOOL(0x600, 0x602, "wBool", struct test_struct, b); THINGSET_ADD_RECORD_ITEM_UINT8(0x600, 0x603, "wU8", struct test_struct, u8); @@ -379,7 +380,8 @@ struct thingset_data_object data_objects[] = { 0), /* Records */ - THINGSET_RECORDS(THINGSET_ID_ROOT, 0x600, "Records", &records_obj, THINGSET_ANY_R, SUBSET_LIVE), + THINGSET_RECORDS(THINGSET_ID_ROOT, 0x600, "Records", &records_obj, THINGSET_ANY_RW, + SUBSET_LIVE), THINGSET_RECORD_ITEM_UINT32(0x600, 0x601, "t_s", struct test_struct, timestamp), THINGSET_RECORD_ITEM_BOOL(0x600, 0x602, "wBool", struct test_struct, b), THINGSET_RECORD_ITEM_UINT8(0x600, 0x603, "wU8", struct test_struct, u8), diff --git a/tests/common/data.h b/tests/common/data.h index 1b90c6a..c1adf31 100644 --- a/tests/common/data.h +++ b/tests/common/data.h @@ -105,4 +105,6 @@ struct test_dyn_struct uint32_t index; }; +extern struct test_dyn_struct dyn_records; + #endif /* TESTS_COMMON_DATA_H_ */ diff --git a/tests/protocol/src/bin.c b/tests/protocol/src/bin.c index 3f51e8d..38eb4ab 100644 --- a/tests/protocol/src/bin.c +++ b/tests/protocol/src/bin.c @@ -1083,6 +1083,103 @@ ZTEST(thingset_bin, test_import_record) records[1].f32_arr[2] = 7.89F; } +/* An UPDATE of a whole records object must not treat the dummy check buffer as a record struct. */ +ZTEST(thingset_bin, test_update_records_empty) +{ + /* = {0x600: [{},{}]} */ +#if CONFIG_THINGSET_BINARY_MAX_DEPTH >= 5 + THINGSET_ASSERT_REQUEST_HEX("07 00 A1 19 0600 82 A0 A0", "84 F6 F6"); +#else + /* the record map cannot be entered with the default nesting depth */ + THINGSET_ASSERT_REQUEST_HEX("07 00 A1 19 0600 82 A0 A0", "AF F6 F6"); +#endif + + zassert_equal(records[0].timestamp, 1); + zassert_equal(records[1].timestamp, 2); +} + +/* + * The following tests deserialize the content of a record map, which requires a nesting depth of + * 3 (outer map, list of records, record map) and thus CONFIG_THINGSET_BINARY_MAX_DEPTH >= 5. + */ +#if CONFIG_THINGSET_BINARY_MAX_DEPTH >= 5 + +/* An UPDATE of a whole records object must write through the actual records descriptor. */ +ZTEST(thingset_bin, test_update_records_values) +{ + /* = {0x600: [{0x601: 5},{0x601: 6}]} */ + THINGSET_ASSERT_REQUEST_HEX("07 00 A1 19 0600 82 A1 19 0601 05 A1 19 0601 06", "84 F6 F6"); + + zassert_equal(records[0].timestamp, 5); + zassert_equal(records[1].timestamp, 6); + + records[0].timestamp = 1; + records[1].timestamp = 2; +} + +/* Items of a nested record must be addressed by IDs belonging to that record. */ +ZTEST(thingset_bin, test_import_record_foreign_id) +{ + struct thingset_endpoint endpoint; + uint8_t data[THINGSET_TEST_BUF_SIZE]; + int err; + + /* 0x502 is Access/wItem, a float item that does not belong to record 0x610 */ + const char data_hex[] = + "A1 " + "19 06 10 " /* Records/Nested */ + "82 " /* two records */ + "A1 19 05 02 FA 3F800000 " /* {Access/wItem: 1.0} -> must be ignored */ + "A0"; /* {} */ + int data_len = hex2bin_spaced(data_hex, data, sizeof(data)); + + err = thingset_endpoint_by_path(&ts, &endpoint, "Records/1", strlen("Records/1")); + zassert_equal(err, 0); + + uint32_t child_u32_bak = records[1].nested[0].child_u32; + float child_f32_bak = records[1].nested[0].child_f32; + + err = thingset_import_record(&ts, data, data_len, &endpoint, THINGSET_BIN_IDS_VALUES); + zassert_equal(err, 0, "act: 0x%X", -err); + + /* the foreign item must have been skipped without touching the record */ + zassert_equal(records[1].nested[0].child_u32, child_u32_bak); + zassert_equal(records[1].nested[0].child_f32, child_f32_bak); +} + +/* Items that do belong to the nested record must still be written. */ +ZTEST(thingset_bin, test_import_record_nested) +{ + struct thingset_endpoint endpoint; + uint8_t data[THINGSET_TEST_BUF_SIZE]; + int err; + + const char data_hex[] = + "A1 " + "19 06 10 " /* Records/Nested */ + "82 " /* two records */ + "A1 19 06 11 0A " /* {Nested/wU32: 10} */ + "A1 19 06 11 14"; /* {Nested/wU32: 20} */ + int data_len = hex2bin_spaced(data_hex, data, sizeof(data)); + + err = thingset_endpoint_by_path(&ts, &endpoint, "Records/1", strlen("Records/1")); + zassert_equal(err, 0); + + uint32_t bak0 = records[1].nested[0].child_u32; + uint32_t bak1 = records[1].nested[1].child_u32; + + err = thingset_import_record(&ts, data, data_len, &endpoint, THINGSET_BIN_IDS_VALUES); + zassert_equal(err, 0, "act: 0x%X", -err); + + zassert_equal(records[1].nested[0].child_u32, 10); + zassert_equal(records[1].nested[1].child_u32, 20); + + records[1].nested[0].child_u32 = bak0; + records[1].nested[1].child_u32 = bak1; +} + +#endif /* CONFIG_THINGSET_BINARY_MAX_DEPTH >= 5 */ + static void *thingset_setup(void) { thingset_init_global(&ts); diff --git a/tests/protocol/src/common.c b/tests/protocol/src/common.c index c26dfa6..8ca02ea 100644 --- a/tests/protocol/src/common.c +++ b/tests/protocol/src/common.c @@ -10,6 +10,7 @@ #include "../../src/thingset_internal.h" +#include "data.h" #include "test_utils.h" static struct thingset_context ts; @@ -114,6 +115,127 @@ ZTEST(thingset_common, test_serialize_path) zassert_mem_equal(buf, "Nested/Obj2/rItem1_V", len); } +/* Paths are not null-terminated, so parsing must not read beyond the given length. */ +static char path_without_termination[] = { 'T', 'y', 'p', 'e', 's', '/', 'w', 'B', 'o', 'o', 'l' }; + +ZTEST(thingset_common, test_endpoint_from_path_without_termination) +{ + struct thingset_endpoint endpoint; + int err; + + err = thingset_endpoint_by_path(&ts, &endpoint, path_without_termination, + sizeof(path_without_termination)); + zassert_equal(err, 0, "act: 0x%X", -err); + zassert_equal(endpoint.object->id, 0x201); + + /* a shorter length must select the group instead of the item */ + err = thingset_endpoint_by_path(&ts, &endpoint, path_without_termination, 5); + zassert_equal(err, 0, "act: 0x%X", -err); + zassert_equal(endpoint.object->id, 0x200); + + /* an empty path selects the root object */ + err = thingset_endpoint_by_path(&ts, &endpoint, path_without_termination, 0); + zassert_equal(err, 0, "act: 0x%X", -err); + zassert_equal(endpoint.object->id, 0); + + /* an empty path must not be dereferenced when looked up directly */ + int index; + zassert_is_null(thingset_get_object_by_path(&ts, path_without_termination, 0, &index)); +} + +/* A record index in a path must be limited to the range of valid indices. */ +ZTEST(thingset_common, test_endpoint_from_path_record_index) +{ + struct thingset_endpoint endpoint; + char *path; + int err; + + path = "Records/1"; + err = thingset_endpoint_by_path(&ts, &endpoint, path, strlen(path)); + zassert_equal(err, 0, "act: 0x%X", -err); + zassert_equal(endpoint.index, 1); + + path = "Records/65535"; + err = thingset_endpoint_by_path(&ts, &endpoint, path, strlen(path)); + zassert_equal(err, 0, "act: 0x%X", -err); + zassert_equal(endpoint.index, 65535); + + /* an index above the range of uint16_t must be rejected instead of overflowing */ + path = "Records/65536"; + err = thingset_endpoint_by_path(&ts, &endpoint, path, strlen(path)); + zassert_equal(err, -THINGSET_ERR_NOT_FOUND, "act: 0x%X", -err); + + path = "Records/99999999999999999999"; + err = thingset_endpoint_by_path(&ts, &endpoint, path, strlen(path)); + zassert_equal(err, -THINGSET_ERR_NOT_FOUND, "act: 0x%X", -err); +} + +/* Importing a record must reject indices outside of the records array. */ +ZTEST(thingset_common, test_import_record_invalid_index) +{ + struct thingset_endpoint endpoint; + uint8_t data[THINGSET_TEST_BUF_SIZE]; + int err; + + /* {0x602: false} */ + int data_len = hex2bin_spaced("A1 19 06 02 F4", data, sizeof(data)); + + err = thingset_endpoint_by_path(&ts, &endpoint, "Records/1", strlen("Records/1")); + zassert_equal(err, 0, "act: 0x%X", -err); + + /* only two records exist */ + endpoint.index = 2; + err = thingset_import_record(&ts, data, data_len, &endpoint, THINGSET_BIN_IDS_VALUES); + zassert_equal(err, -THINGSET_ERR_NOT_FOUND, "act: 0x%X", -err); + + endpoint.index = THINGSET_ENDPOINT_INDEX_NONE; + err = thingset_import_record(&ts, data, data_len, &endpoint, THINGSET_BIN_IDS_VALUES); + zassert_equal(err, -THINGSET_ERR_NOT_FOUND, "act: 0x%X", -err); + + /* an endpoint that is not a records object must be rejected as well */ + err = thingset_endpoint_by_path(&ts, &endpoint, "Types", strlen("Types")); + zassert_equal(err, 0, "act: 0x%X", -err); + endpoint.index = 0; + err = thingset_import_record(&ts, data, data_len, &endpoint, THINGSET_BIN_IDS_VALUES); + zassert_equal(err, -THINGSET_ERR_NOT_FOUND, "act: 0x%X", -err); +} + +/* Dynamic records keep only one record in memory, so the index must not be used as an offset. */ +ZTEST(thingset_common, test_import_dyn_record) +{ + struct thingset_endpoint endpoint; + uint8_t data[THINGSET_TEST_BUF_SIZE]; + int err; + + /* {0x681: 42} */ + int data_len = hex2bin_spaced("A1 19 06 81 18 2A", data, sizeof(data)); + + err = thingset_endpoint_by_path(&ts, &endpoint, "DynRecords/7", strlen("DynRecords/7")); + zassert_equal(err, 0, "act: 0x%X", -err); + zassert_equal(endpoint.index, 7); + + err = thingset_import_record(&ts, data, data_len, &endpoint, THINGSET_BIN_IDS_VALUES); + zassert_equal(err, 0, "act: 0x%X", -err); + + zassert_equal(dyn_records.index, 42); + + dyn_records.index = 0; +} + +/* Rejecting an unsupported format must not leave the context locked. */ +ZTEST(thingset_common, test_export_subsets_unsupported_format) +{ + uint8_t buf[THINGSET_TEST_BUF_SIZE]; + int err; + + err = thingset_export_subsets(&ts, buf, sizeof(buf), SUBSET_LIVE, THINGSET_BIN_IDS_ONLY); + zassert_equal(err, -THINGSET_ERR_NOT_IMPLEMENTED, "act: 0x%X", -err); + + /* a leaked lock would make this time out and fail with an internal server error */ + err = thingset_export_subsets(&ts, buf, sizeof(buf), SUBSET_LIVE, THINGSET_BIN_IDS_VALUES); + zassert_true(err > 0, "act: 0x%X", -err); +} + static void *thingset_setup(void) { thingset_init_global(&ts); diff --git a/tests/protocol/src/txt.c b/tests/protocol/src/txt.c index 00b4d77..923a376 100644 --- a/tests/protocol/src/txt.c +++ b/tests/protocol/src/txt.c @@ -294,6 +294,51 @@ ZTEST(thingset_txt, test_update_escaped_string) #endif /* CONFIG_THINGSET_JSON_STRING_ESCAPING */ +/* + * Process the request with a response buffer of the given size, which is followed by a guard + * pattern to detect writes beyond the end of the buffer. + */ +static void assert_response_within_buffer(const char *req, size_t rsp_size) +{ + uint8_t buf[THINGSET_TEST_BUF_SIZE]; + const uint8_t guard = 0xAA; + + memset(buf, guard, sizeof(buf)); + + int len = thingset_process_message(&ts, req, strlen(req), buf, rsp_size); + + for (size_t i = rsp_size; i < sizeof(buf); i++) { + if (buf[i] != guard) { + zassert_unreachable("req \"%s\" with buffer size %u wrote %u bytes beyond the end", req, + (unsigned int)rsp_size, (unsigned int)(i - rsp_size + 1)); + } + } + + zassert_true(len <= (int)rsp_size, "req \"%s\" returned %d for a buffer size of %u", req, len, + (unsigned int)rsp_size); +} + +/* Serializing into a buffer that is too small must not write beyond the end of the buffer. */ +ZTEST(thingset_txt, test_serialize_buffer_too_small) +{ + static const char *const reqs[] = { + "?Arrays/wF32", /* array */ + "?mLive", /* subset of paths */ + "?Exec", /* group containing functions */ + "?Types/wString", /* string */ + "?Types", /* group of simple values */ + "?Records/1", /* record */ + "?", /* whole node */ + }; + + for (unsigned int i = 0; i < ARRAY_SIZE(reqs); i++) { + /* 4 bytes is the minimum size accepted by thingset_process_message() */ + for (size_t size = 4; size <= 128; size++) { + assert_response_within_buffer(reqs[i], size); + } + } +} + ZTEST(thingset_txt, test_update_timestamp_zero) { THINGSET_ASSERT_REQUEST_TXT("= {\"t_s\":0}", ":84"); diff --git a/tests/protocol/testcase.yaml b/tests/protocol/testcase.yaml index 8b81d8a..14a7f02 100644 --- a/tests/protocol/testcase.yaml +++ b/tests/protocol/testcase.yaml @@ -20,3 +20,11 @@ tests: extra_args: EXTRA_CFLAGS=-Werror extra_configs: - CONFIG_THINGSET_OBJECT_LOOKUP_MAP=y + # deserializing records needs a nesting depth of 3, which the default max depth cannot provide + thingset.protocol.nested_records: + integration_platforms: + - native_sim + - native_sim/native/64 + extra_args: EXTRA_CFLAGS=-Werror + extra_configs: + - CONFIG_THINGSET_BINARY_MAX_DEPTH=5 diff --git a/tests/report/src/report.c b/tests/report/src/report.c index 3715303..1265368 100644 --- a/tests/report/src/report.c +++ b/tests/report/src/report.c @@ -140,6 +140,29 @@ ZTEST(thingset_report, test_report_txt) THINGSET_ASSERT_REPORT_TXT("mLive", rpt_exp, strlen(rpt_exp)); } +/* Generating a report into a buffer that is too small must not write beyond the end. */ +ZTEST(thingset_report, test_report_buffer_too_small) +{ + uint8_t buf[THINGSET_TEST_BUF_SIZE]; + const uint8_t guard = 0xAA; + + for (size_t rpt_size = 1; rpt_size <= 256; rpt_size++) { + memset(buf, guard, sizeof(buf)); + + int len = thingset_report_path(&ts, buf, rpt_size, "mLive", THINGSET_TXT_NAMES_VALUES); + + for (size_t i = rpt_size; i < sizeof(buf); i++) { + if (buf[i] != guard) { + zassert_unreachable("buffer size %u: wrote %u bytes beyond the end", + (unsigned int)rpt_size, (unsigned int)(i - rpt_size + 1)); + } + } + + zassert_true(len <= (int)rpt_size, "buffer size %u: returned %d", (unsigned int)rpt_size, + len); + } +} + static void *thingset_setup(void) { thingset_init_global(&ts);