Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 58 additions & 22 deletions src/printer_cbor.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ struct cborpr_ctx {
const struct lyd_node *first_leaflist; /**< first printed leaf-list instance, used when printing its metadata/attributes */

cbor_item_t *root_map; /**< root CBOR map */
cbor_item_t *array; /**< currently open CBOR array for leaf-list/list instances */
cbor_item_t **arrays; /**< stack of currently open CBOR arrays, parallel to `open` (one per nesting level) */
uint32_t arrays_size; /**< allocated size of the `arrays` stack */
};

static LY_ERR cbor_print_node(struct cborpr_ctx *pctx, const struct lyd_node *node, cbor_item_t *parent_map);
Expand Down Expand Up @@ -93,12 +94,39 @@ matching_node(const struct lyd_node *node1, const struct lyd_node *node2)
* @return LY_ERR value.
*/
static LY_ERR
cbor_print_array_open(struct cborpr_ctx *pctx, const struct lyd_node *node)
cbor_print_array_open(struct cborpr_ctx *pctx, const struct lyd_node *node, cbor_item_t *array)
{
LY_CHECK_RET(ly_set_add(&pctx->open, (void *)node, 0, NULL));

/* keep the array pointer on a stack parallel to `open` so that nested
* (leaf-)lists each have their own open array instead of a single shared one */
if (pctx->open.count > pctx->arrays_size) {
cbor_item_t **tmp = realloc(pctx->arrays, pctx->open.count * sizeof *pctx->arrays);

LY_CHECK_RET(!tmp, LY_EMEM);
pctx->arrays = tmp;
pctx->arrays_size = pctx->open.count;
}
pctx->arrays[pctx->open.count - 1] = array;

return LY_SUCCESS;
}

/**
* @brief Get the innermost currently open CBOR array.
*
* @param[in] pctx CBOR printer context.
* @return The innermost open array, or NULL if none is open.
*/
static cbor_item_t *
cbor_current_array(struct cborpr_ctx *pctx)
{
if (!pctx->open.count) {
return NULL;
}
return pctx->arrays[pctx->open.count - 1];
}

/**
* @brief Get know if the array for the provided @p node is currently open.
*
Expand Down Expand Up @@ -922,31 +950,35 @@ cbor_print_array_is_last_inst(struct cborpr_ctx *pctx, const struct lyd_node *no
* @return LY_ERR value.
*/
static LY_ERR
cbor_print_leaf_list(struct cborpr_ctx *pctx, const struct lyd_node *node, cbor_item_t *parent_map, cbor_item_t **array_p)
cbor_print_leaf_list(struct cborpr_ctx *pctx, const struct lyd_node *node, cbor_item_t *parent_map)
{
const struct lys_module *wdmod = NULL;
cbor_item_t *value_item = NULL;
cbor_item_t *inner_map = NULL;
cbor_item_t *array;
char *key = NULL;

if (!is_open_array(pctx, node)) {
/* start new array */
*array_p = cbor_new_indefinite_array();
LY_CHECK_RET(!*array_p, LY_EMEM);
cbor_item_t *new_array = cbor_new_indefinite_array();

LY_CHECK_RET(cbor_print_array_open(pctx, node));
LY_CHECK_RET(!new_array, LY_EMEM);
LY_CHECK_ERR_RET(cbor_print_array_open(pctx, node, new_array), cbor_decref(&new_array), LY_EINVAL);
}
/* the array for this (leaf-)list is the innermost open one */
array = cbor_current_array(pctx);

if (node->schema->nodetype == LYS_LIST) {
/* print list's content */
LY_CHECK_RET(cbor_print_inner(pctx, node, &inner_map));
LY_CHECK_RET(cbor_array_push_check(pctx->ctx, *array_p, inner_map));
/* re-read: nested (leaf-)lists may have pushed/popped their own arrays */
LY_CHECK_RET(cbor_array_push_check(pctx->ctx, cbor_current_array(pctx), inner_map));
} else {
assert(node->schema->nodetype == LYS_LEAFLIST);

LY_CHECK_RET(cbor_print_value(pctx, LYD_CTX(node), &((const struct lyd_node_term *)node)->value,
node->schema->module, &value_item));
LY_CHECK_RET(cbor_array_push_check(pctx->ctx, *array_p, value_item));
LY_CHECK_RET(cbor_array_push_check(pctx->ctx, array, value_item));

if (!pctx->first_leaflist) {
if (((node->flags & LYD_DEFAULT) && (pctx->options & (LYD_PRINT_WD_ALL_TAG | LYD_PRINT_WD_IMPL_TAG))) ||
Expand All @@ -962,12 +994,13 @@ cbor_print_leaf_list(struct cborpr_ctx *pctx, const struct lyd_node *node, cbor_
}

if (cbor_print_array_is_last_inst(pctx, node)) {
array = cbor_current_array(pctx);
key = cbor_print_member_name(pctx, node, 0);
/* add completed array to parent map */
if (key) {
struct cbor_pair pair = {
.key = cbor_move(cbor_build_string(key)),
.value = cbor_move(*array_p)
.value = cbor_move(array)
};

free(key);
Expand All @@ -978,11 +1011,11 @@ cbor_print_leaf_list(struct cborpr_ctx *pctx, const struct lyd_node *node, cbor_
return LY_EMEM;
}
} else {
cbor_decref(array_p);
cbor_decref(&array);
cbor_print_array_close(pctx);
return LY_EMEM;
}
cbor_print_array_close(pctx);
*array_p = NULL;
}

return LY_SUCCESS;
Expand Down Expand Up @@ -1091,7 +1124,7 @@ cbor_print_meta_attr_leaflist(struct cborpr_ctx *pctx, cbor_item_t *parent_map)
* @return LY_ERR value.
*/
static LY_ERR
cbor_print_opaq(struct cborpr_ctx *pctx, const struct lyd_node_opaq *node, cbor_item_t *parent_map, cbor_item_t **array_p)
cbor_print_opaq(struct cborpr_ctx *pctx, const struct lyd_node_opaq *node, cbor_item_t *parent_map)
{
ly_bool first = 1, last = 1;
uint32_t hints;
Expand Down Expand Up @@ -1119,9 +1152,11 @@ cbor_print_opaq(struct cborpr_ctx *pctx, const struct lyd_node_opaq *node, cbor_
LY_CHECK_RET(!key, LY_EMEM);

if (hints & (LYD_NODEHINT_LIST | LYD_NODEHINT_LEAFLIST)) {
*array_p = cbor_new_indefinite_array();
LY_CHECK_ERR_RET(!*array_p, free(key), LY_EMEM);
LY_CHECK_ERR_RET(cbor_print_array_open(pctx, &node->node), free(key), LY_EINVAL);
cbor_item_t *new_array = cbor_new_indefinite_array();

LY_CHECK_ERR_RET(!new_array, free(key), LY_EMEM);
LY_CHECK_ERR_RET(cbor_print_array_open(pctx, &node->node, new_array),
cbor_decref(&new_array); free(key), LY_EINVAL);
}
}

Expand All @@ -1131,7 +1166,7 @@ cbor_print_opaq(struct cborpr_ctx *pctx, const struct lyd_node_opaq *node, cbor_
LY_CHECK_ERR_RET(cbor_print_inner(pctx, &node->node, &inner_map), free(key), LY_EINVAL);

if (hints & (LYD_NODEHINT_LIST | LYD_NODEHINT_LEAFLIST)) {
LY_CHECK_RET(cbor_array_push_check(pctx->ctx, *array_p, inner_map));
LY_CHECK_RET(cbor_array_push_check(pctx->ctx, cbor_current_array(pctx), inner_map));
} else {
struct cbor_pair pair = {
.key = cbor_move(cbor_build_string(key)),
Expand Down Expand Up @@ -1176,7 +1211,7 @@ cbor_print_opaq(struct cborpr_ctx *pctx, const struct lyd_node_opaq *node, cbor_
LY_CHECK_ERR_RET(!value_item, free(key), LY_EMEM);

if (hints & (LYD_NODEHINT_LIST | LYD_NODEHINT_LEAFLIST)) {
LY_CHECK_RET(cbor_array_push_check(pctx->ctx, *array_p, value_item));
LY_CHECK_RET(cbor_array_push_check(pctx->ctx, cbor_current_array(pctx), value_item));
} else {
struct cbor_pair pair = {
.key = cbor_move(cbor_build_string(key)),
Expand Down Expand Up @@ -1206,7 +1241,7 @@ cbor_print_opaq(struct cborpr_ctx *pctx, const struct lyd_node_opaq *node, cbor_
if (key) {
struct cbor_pair pair = {
.key = cbor_move(cbor_build_string(key)),
.value = cbor_move(*array_p)
.value = cbor_move(cbor_current_array(pctx))
};

free(key);
Expand All @@ -1218,7 +1253,6 @@ cbor_print_opaq(struct cborpr_ctx *pctx, const struct lyd_node_opaq *node, cbor_
}
}
cbor_print_array_close(pctx);
*array_p = NULL;
}

if (key) {
Expand Down Expand Up @@ -1248,7 +1282,7 @@ cbor_print_node(struct cborpr_ctx *pctx, const struct lyd_node *node, cbor_item_
}

if (!node->schema) {
LY_CHECK_RET(cbor_print_opaq(pctx, (const struct lyd_node_opaq *)node, parent_map, &pctx->array));
LY_CHECK_RET(cbor_print_opaq(pctx, (const struct lyd_node_opaq *)node, parent_map));
} else {
switch (node->schema->nodetype) {
case LYS_RPC:
Expand All @@ -1262,7 +1296,7 @@ cbor_print_node(struct cborpr_ctx *pctx, const struct lyd_node *node, cbor_item_
break;
case LYS_LEAFLIST:
case LYS_LIST:
LY_CHECK_RET(cbor_print_leaf_list(pctx, node, parent_map, &pctx->array));
LY_CHECK_RET(cbor_print_leaf_list(pctx, node, parent_map));
break;
case LYS_ANYDATA:
case LYS_ANYXML:
Expand Down Expand Up @@ -1322,7 +1356,7 @@ cbor_print_data(struct ly_out *out, const struct lyd_node *root, uint32_t option
LY_LIST_FOR(root, node) {
pctx.root = node;
LY_CHECK_ERR_RET(cbor_print_node(&pctx, node, pctx.root_map),
cbor_decref(&pctx.root_map); ly_set_erase(&pctx.open, NULL), LY_EINVAL);
cbor_decref(&pctx.root_map); ly_set_erase(&pctx.open, NULL); free(pctx.arrays), LY_EINVAL);
if (!(options & LYD_PRINT_SIBLINGS)) {
break;
}
Expand All @@ -1334,6 +1368,7 @@ cbor_print_data(struct ly_out *out, const struct lyd_node *root, uint32_t option

if (buffer_size == 0) {
ly_set_erase(&pctx.open, NULL);
free(pctx.arrays);
return LY_EMEM;
}

Expand All @@ -1343,6 +1378,7 @@ cbor_print_data(struct ly_out *out, const struct lyd_node *root, uint32_t option

assert(!pctx.open.count);
ly_set_erase(&pctx.open, NULL);
free(pctx.arrays);

ly_print_flush(out);
return LY_SUCCESS;
Expand Down
31 changes: 31 additions & 0 deletions tests/modules/yang/cbor-test.yang
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,37 @@ module cbor-test {
}
}

/* ===== Nested lists / leaf-lists =====
* Regression coverage for the CBOR printer: a leaf-list and a list
* nested inside another list. Printing such data used to crash because
* the printer kept only a single open-array pointer shared across all
* nesting levels.
*/

list list-nested {
key "id";

leaf id {
type string;
}

leaf-list tags {
type string;
}

list inner {
key "iid";

leaf iid {
type uint32;
}

leaf-list vals {
type int32;
}
}
}

/* ===== Choice / Case ===== */

choice choice-transport {
Expand Down
67 changes: 67 additions & 0 deletions tests/utests/data/test_cbor.c
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,72 @@ test_node(void **state)
}
}

/**
* @brief Regression test: printing a (leaf-)list nested inside a list.
*
* The CBOR printer used to keep a single "currently open array" pointer in its
* context. When a leaf-list (or list) was nested inside a list, printing the
* inner array overwrote that pointer and then reset it to NULL, so the outer
* list pushed into a NULL array and the printer crashed (SIGSEGV in
* cbor_array_push()). This exercises exactly that structure and additionally
* checks the JSON round-trip fidelity.
*/
static void
test_nested_list(void **state)
{
struct lyd_node *tree;
char *buffer, *json;
struct ly_out *out;
size_t i;

const struct test_case tests[] = {
/* leaf-list nested inside a list */
{
"ll-in-list",
"{\"cbor-test:list-nested\":["
"{\"id\":\"a\",\"tags\":[\"t1\",\"t2\"]}"
"]}"
},
/* list nested inside a list, the inner list holding a leaf-list */
{
"list-in-list",
"{\"cbor-test:list-nested\":["
"{\"id\":\"a\",\"inner\":[{\"iid\":1,\"vals\":[10,20]},{\"iid\":2,\"vals\":[30]}]}"
"]}"
},
/* several outer instances mixing nested leaf-lists and lists */
{
"multi-nested",
"{\"cbor-test:list-nested\":["
"{\"id\":\"a\",\"tags\":[\"t1\",\"t2\"],\"inner\":[{\"iid\":1,\"vals\":[10,20]}]},"
"{\"id\":\"b\",\"inner\":[{\"iid\":3,\"vals\":[40]}]}"
"]}"
},
};

for (i = 0; i < sizeof(tests) / sizeof(tests[0]); ++i) {
CHECK_PARSE_LYD_PARAM(tests[i].json, LYD_JSON, LYD_PARSE_STRICT, LYD_VALIDATE_PRESENT, LY_SUCCESS, tree);

assert_int_equal(LY_SUCCESS, ly_out_new_memory(&buffer, 0, &out));
/* this print call is what used to crash */
assert_int_equal(LY_SUCCESS, lyd_print_all(out, tree, LYD_CBOR, 0));

lyd_free_all(tree);

assert_int_equal(LY_SUCCESS, lyd_parse_data_mem_len(((struct utest_context *)*state)->ctx, buffer,
(uint32_t)ly_out_printed(out), LYD_CBOR, LYD_PARSE_ONLY, 0, &tree));

ly_out_free(out, NULL, 0);
free(buffer);

assert_int_equal(LY_SUCCESS, lyd_print_mem(&json, tree, LYD_JSON, LYD_PRINT_SHRINK | LYD_PRINT_SIBLINGS));
assert_string_equal(json, tests[i].json);

free(json);
lyd_free_all(tree);
}
}

static void
test_operations(void **state)
{
Expand Down Expand Up @@ -304,6 +370,7 @@ main(void)
{
const struct CMUnitTest tests[] = {
UTEST(test_node, setup),
UTEST(test_nested_list, setup),
UTEST(test_operations, setup),
UTEST(test_schema_mount, setup),
UTEST(test_opaque, setup),
Expand Down
Loading