From bb432b4eea2bd19d034dd729fc8caa376f0cab83 Mon Sep 17 00:00:00 2001 From: tongkunkun Date: Mon, 29 Aug 2022 18:16:42 +0800 Subject: [PATCH] json: fix parsing json problems when json config is invalid. Add parsing json as invalid cases: 1.json content that not enclosed in {}, it should be parsed as invalid, e.g. "abc":"not encloesed in {}" 2.json content that 'subsystems' not associate with array, it will report error and return failure, e.g. {"subsystems":"123"} 3.handle other invalid json formats, report and return failure, e.g. duplicate keys. Added `spdk_json_find` API return errcode: EPROTOTYPE - json not enclosed in {}. json config with content: 1."not enclosed in {}" 2."'subsystems' not be an array" 3."duplicate key in json" and some other invaild cases will be regarded as invalid json config, and will fail to start app. Fixes #2599 Signed-off-by: tongkunkun Change-Id: I02574c9acd7671e336d4c589ebbff8ed21eb3681 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/13754 Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: GangCao Reviewed-by: Aleksey Marchuk Reviewed-by: Tomasz Zawadzki Reviewed-by: Jim Harris --- CHANGELOG.md | 11 +++++++++++ include/spdk/json.h | 1 + lib/init/json_config.c | 18 +++++++++++++++--- lib/json/json_util.c | 12 +++++++++--- test/bdev/blockdev.sh | 9 +++++++++ test/bdev/nonarray.json | 1 + test/bdev/nonenclosed.json | 1 + 7 files changed, 47 insertions(+), 6 deletions(-) create mode 100644 test/bdev/nonarray.json create mode 100644 test/bdev/nonenclosed.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 6386a618c..746c4f23c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ ## v22.09: (Upcoming Release) +### json + +Added `spdk_json_find` API return errcode: EPROTOTYPE - json not enclosed in {}. +`spdk_json_find` now returns -EPROTOTYPE instead of -ENOENT if the object parameter +does not point to a JSON object (i.e. is not enclosed with {}). + +### init + +`spdk_subsystem_init_from_json_config` now fails if the JSON configuration file is not +an object with an array named "subsystems". + ### bdev New RPCs `bdev_xnvme_create` and `bdev_xnvme_delete` were added to support the xNVMe bdev. diff --git a/include/spdk/json.h b/include/spdk/json.h index b20820153..a3b8d9985 100644 --- a/include/spdk/json.h +++ b/include/spdk/json.h @@ -266,6 +266,7 @@ int spdk_json_write_named_object_begin(struct spdk_json_write_ctx *w, const char * -EINVAL - json object is invalid * -ENOENT - key not found * -EDOM - key exists but value type mismatch. + * -EPROTOTYPE - json not enclosed in {}. */ int spdk_json_find(struct spdk_json_val *object, const char *key_name, struct spdk_json_val **key, struct spdk_json_val **val, enum spdk_json_val_type type); diff --git a/lib/init/json_config.c b/lib/init/json_config.c index 0db8e06d8..f1fcaa5f1 100644 --- a/lib/init/json_config.c +++ b/lib/init/json_config.c @@ -566,14 +566,26 @@ spdk_subsystem_init_from_json_config(const char *json_config_file, const char *r /* Capture subsystems array */ rc = spdk_json_find_array(ctx->values, "subsystems", NULL, &ctx->subsystems); - if (rc) { - SPDK_WARNLOG("No 'subsystems' key JSON configuration file.\n"); - } else { + switch (rc) { + case 0: /* Get first subsystem */ ctx->subsystems_it = spdk_json_array_first(ctx->subsystems); if (ctx->subsystems_it == NULL) { SPDK_NOTICELOG("'subsystems' configuration is empty\n"); } + break; + case -EPROTOTYPE: + SPDK_ERRLOG("Invalid JSON configuration: not enclosed in {}.\n"); + goto fail; + case -ENOENT: + SPDK_WARNLOG("No 'subsystems' key JSON configuration file.\n"); + break; + case -EDOM: + SPDK_ERRLOG("Invalid JSON configuration: 'subsystems' should be an array.\n"); + goto fail; + default: + SPDK_ERRLOG("Failed to parse JSON configuration.\n"); + goto fail; } /* If rpc_addr is not an Unix socket use default address as prefix. */ diff --git a/lib/json/json_util.c b/lib/json/json_util.c index a81feb9b9..2ca5fe14a 100644 --- a/lib/json/json_util.c +++ b/lib/json/json_util.c @@ -530,11 +530,17 @@ spdk_json_find(struct spdk_json_val *object, const char *key_name, struct spdk_j { struct spdk_json_val *_key = NULL; struct spdk_json_val *_val = NULL; - struct spdk_json_val *it; + struct spdk_json_val *it_first, *it; assert(object != NULL); - for (it = json_first(object, SPDK_JSON_VAL_ARRAY_BEGIN | SPDK_JSON_VAL_OBJECT_BEGIN); + it_first = json_first(object, SPDK_JSON_VAL_OBJECT_BEGIN); + if (!it_first) { + SPDK_JSON_DEBUG("Not enclosed in {}\n"); + return -EPROTOTYPE; + } + + for (it = it_first; it != NULL; it = spdk_json_next(it)) { if (it->type != SPDK_JSON_VAL_NAME) { @@ -553,7 +559,7 @@ spdk_json_find(struct spdk_json_val *object, const char *key_name, struct spdk_j _key = it; _val = json_value(_key); - if (type != SPDK_JSON_VAL_INVALID && (_val->type & type) == 0) { + if (type != SPDK_JSON_VAL_ANY && (_val->type & type) == 0) { SPDK_JSON_DEBUG("key '%s' type is %#x but expected one of %#x\n", key_name, _val->type, type); return -EDOM; } diff --git a/test/bdev/blockdev.sh b/test/bdev/blockdev.sh index 42a5b771c..6a310172a 100755 --- a/test/bdev/blockdev.sh +++ b/test/bdev/blockdev.sh @@ -7,6 +7,9 @@ source $testdir/nbd_common.sh rpc_py=rpc_cmd conf_file="$testdir/bdev.json" +nonenclosed_conf_file="$testdir/nonenclosed.json" +nonarray_conf_file="$testdir/nonarray.json" + # Make sure the configuration is clean : > "$conf_file" @@ -566,6 +569,12 @@ trap "cleanup" SIGINT SIGTERM EXIT run_test "bdev_verify" $testdir/bdevperf/bdevperf --json "$conf_file" -q 128 -o 4096 -w verify -t 5 -C -m 0x3 "$env_ctx" run_test "bdev_write_zeroes" $testdir/bdevperf/bdevperf --json "$conf_file" -q 128 -o 4096 -w write_zeroes -t 1 "$env_ctx" +# test json config not enclosed with {} +run_test "bdev_json_nonenclosed" $testdir/bdevperf/bdevperf --json "$nonenclosed_conf_file" -q 128 -o 4096 -w write_zeroes -t 1 "$env_ctx" || true + +# test json config "subsystems" not with array +run_test "bdev_json_nonarray" $testdir/bdevperf/bdevperf --json "$nonarray_conf_file" -q 128 -o 4096 -w write_zeroes -t 1 "$env_ctx" || true + if [[ $test_type == bdev ]]; then run_test "bdev_qos" qos_test_suite "$env_ctx" run_test "bdev_qd_sampling" qd_sampling_test_suite "$env_ctx" diff --git a/test/bdev/nonarray.json b/test/bdev/nonarray.json new file mode 100644 index 000000000..5a3899731 --- /dev/null +++ b/test/bdev/nonarray.json @@ -0,0 +1 @@ +{"subsystems": "test non array"} diff --git a/test/bdev/nonenclosed.json b/test/bdev/nonenclosed.json new file mode 100644 index 000000000..e9397a1c5 --- /dev/null +++ b/test/bdev/nonenclosed.json @@ -0,0 +1 @@ +"subsystems" : "test nonenclosed json"