diff --git a/Makefile b/Makefile index b1b0e02..9dc4b21 100644 --- a/Makefile +++ b/Makefile @@ -3,7 +3,7 @@ JSON_C_LDFLAGS=$(shell pkg-config --libs json-c) include config.mk -all: test-json-c-get-a test-json-update test-json-filter test-json-subpath test-json-c-array-root +all: test-json-c-get-a test-json-update test-json-filter test-json-subpath test-json-c-array-root test-json-filter-and-missing-key test-json-get-array-big-index bench: make -C bench @@ -30,12 +30,20 @@ test-json-filter: tests/json-c/filter.c csonpath_json-c.h csonpath.h csonpath_do test-json-subpath: tests/json-c/subpath.c csonpath_json-c.h csonpath.h csonpath_do.h $(CC) tests/json-c/subpath.c $(EXTRA_FILES) $(JSON_C_CFLAGS) $(CFLAGS) -Wno-format -I./ -o test-json-subpath $(JSON_C_LDFLAGS) $(LDFLAGS) -tests-c: test-json-c-get-a test-json-update test-json-filter test-json-subpath test-json-c-array-root +test-json-filter-and-missing-key: tests/json-c/filter-and-missing-key.c csonpath_json-c.h csonpath.h csonpath_do.h + $(CC) tests/json-c/filter-and-missing-key.c $(EXTRA_FILES) $(JSON_C_CFLAGS) $(CFLAGS) -Wno-format -I./ -o test-json-filter-and-missing-key $(JSON_C_LDFLAGS) $(LDFLAGS) + +test-json-get-array-big-index: tests/json-c/get-array-big-index.c csonpath_json-c.h csonpath.h csonpath_do.h + $(CC) tests/json-c/get-array-big-index.c $(EXTRA_FILES) $(JSON_C_CFLAGS) $(CFLAGS) -Wno-format -I./ -o test-json-get-array-big-index $(JSON_C_LDFLAGS) $(LDFLAGS) + +tests-c: test-json-c-get-a test-json-update test-json-filter test-json-subpath test-json-c-array-root test-json-filter-and-missing-key test-json-get-array-big-index ./test-json-c-get-a ./test-json-update ./test-json-filter ./test-json-subpath ./test-json-c-array-root + ./test-json-filter-and-missing-key + ./test-json-get-array-big-index pip-dev: pip install -e .[dev] --force-reinstall @@ -46,5 +54,5 @@ tests-py: pip-dev tests: tests-py tests-c clean: - rm -rvf test-json-c-get-a test-json-update test-json-filter test-json-subpath + rm -rvf test-json-c-get-a test-json-update test-json-filter test-json-subpath test-json-c-array-root test-json-filter-and-missing-key test-json-get-array-big-index diff --git a/csonpath.h b/csonpath.h index a252375..f3fb250 100644 --- a/csonpath.h +++ b/csonpath.h @@ -918,9 +918,9 @@ need_reloop_in = 0; #define CSONPATH_NONE_FOUND_RET CSONPATH_NULL #define CSONPATH_GETTER_ERR(args...) do { \ - fprintf(stderr, args); \ - return CSONPATH_NULL; \ - } while (0) + CSONPATH_FORMAT_EXCEPTION(args); \ + return CSONPATH_NULL; \ + } while (0) /* Find First */ @@ -980,10 +980,10 @@ need_reloop_in = 0; #define CSONPATH_NONE_FOUND_RET 0 -#define CSONPATH_GETTER_ERR(args...) do { \ - fprintf(stderr, args); \ - return -1; \ - } while (0) +#define CSONPATH_GETTER_ERR(args...) do { \ + CSONPATH_FORMAT_EXCEPTION(args); \ + return -1; \ + } while (0) #define CSONPATH_DO_ON_FOUND @@ -1104,7 +1104,8 @@ static int csonpath_sync_root_array(CSONPATH_JSON parent, CSONPATH_JSON to_updat CSONPATH_ARRAY_CLEAR(parent); CSONPATH_FOREACH_ARRAY(to_update, child, idx) { - CSONPATH_APPEND_AT(parent, idx, child); + if (CSONPATH_APPEND_AT(parent, idx, child) < 0) + return -1; } return 1; } @@ -1116,7 +1117,8 @@ static int csonpath_sync_root_obj(CSONPATH_JSON parent, CSONPATH_JSON to_update) CSONPATH_OBJ_CLEAR(parent); CSONPATH_FOREACH_OBJ(to_update, child, key) { - CSONPATH_APPEND_AT(parent, key, child); + if (CSONPATH_APPEND_AT(parent, key, child) < 0) + return -1; } return 1; } @@ -1129,7 +1131,7 @@ static int csonpath_sync_root_obj(CSONPATH_JSON parent, CSONPATH_JSON to_update) } else if (CSONPATH_IS_ARRAY(origin) && CSONPATH_IS_ARRAY(to_update)) { \ return csonpath_sync_root_array(origin, to_update); \ } else { \ - CSONPATH_EXCEPTION("can't upate root ($)\n"); \ + CSONPATH_EXCEPTION("can't update root ($)\n"); \ } \ } @@ -1141,7 +1143,7 @@ static int csonpath_sync_root_obj(CSONPATH_JSON parent, CSONPATH_JSON to_update) to_check = *check_at; \ } while (to_check == CSONPATH_INST_GET_ALL || to_check == CSONPATH_INST_FIND_ALL); \ if (to_check == CSONPATH_INST_END || to_check == CSONPATH_INST_OR) { \ - CSONPATH_APPEND_AT(ctx, this_idx, to_update); \ + if (CSONPATH_APPEND_AT(ctx, this_idx, to_update) < 0) return -1; \ return 1; \ } @@ -1215,7 +1217,7 @@ static int csonpath_sync_root_obj(CSONPATH_JSON parent, CSONPATH_JSON to_update) #define CSONPATH_PRE_GET_ROOT \ int to_check = *walker; \ if (to_check == CSONPATH_INST_END || to_check == CSONPATH_INST_OR) { \ - CSONPATH_GETTER_ERR("can't upate root ($)\n"); \ + CSONPATH_GETTER_ERR("can't update root ($)\n"); \ return CSONPATH_NONE_FOUND_RET; \ } diff --git a/csonpath_do.h b/csonpath_do.h index 6893655..14a96ac 100644 --- a/csonpath_do.h +++ b/csonpath_do.h @@ -161,7 +161,7 @@ static CSONPATH_DO_RET_TYPE csonpath_do_dotdot(const struct csonpath cjp[const s CSONPATH_DO_FIND_ALL; } } - } else { + } else if (CSONPATH_IS_ARRAY(tmp)) { CSONPATH_FOREACH_ARRAY(tmp, el, key_idx) { CSONPATH_DO_FOREACH_PRE_SET; if (CSONPATH_IS_OBJ(el) || CSONPATH_IS_ARRAY(el)) { @@ -212,9 +212,15 @@ static CSONPATH_DO_RET_TYPE csonpath_do_internal(const struct csonpath cjp[const (void) foreach_idx; CSONPATH_DO_FILTER_PRE_LOOP; + if (!CSONPATH_IS_ARRAY(tmp)) { + CSONPATH_DO_FILTER_OUT; + break; + } CSONPATH_FOREACH_ARRAY(tmp, el, foreach_idx) { intptr_t key_idx = foreach_idx; + filter_next = filter_next_in; + operation = operation_in; (void)key_idx; owalker = next; match_and_or_or: @@ -226,7 +232,7 @@ static CSONPATH_DO_RET_TYPE csonpath_do_internal(const struct csonpath cjp[const if (csonpath_make_match(cjp, origin, el2, &owalker, operation)) { if (*owalker == CSONPATH_INST_FILTER_AND) { - ++owalker; /* skip next */ + ++owalker; /* skip and */ operation = *owalker; ++owalker; filter_next = *owalker; @@ -237,9 +243,6 @@ static CSONPATH_DO_RET_TYPE csonpath_do_internal(const struct csonpath cjp[const csonpath_do_internal(cjp, origin, el, tmp, owalker CSONPATH_DO_EXTRA_ARGS_NEESTED); CSONPATH_DO_FILTER_FIND; - } else { - operation = operation_in; - filter_next = filter_next_in; } } } @@ -385,7 +388,7 @@ static CSONPATH_DO_RET_TYPE csonpath_do_internal(const struct csonpath cjp[const this_idx = to_num.n; CSONPATH_PRE_GET(this_idx); tmp = CSONPATH_AT(tmp, to_num.n); - walker += 5; + walker += 4; if (tmp == CSONPATH_NULL) { CSONPATH_DO_GET_NOTFOUND(this_idx); } diff --git a/csonpath_python.c b/csonpath_python.c index b3fda41..cc7ec9b 100644 --- a/csonpath_python.c +++ b/csonpath_python.c @@ -264,6 +264,8 @@ static PyObject *find_all(PyCsonPathObject *self, PyObject* args) BAD_ARG(); PyObject *ret = csonpath_find_all(self->cp, json); + if (PyErr_Occurred()) + return NULL; return ret; } @@ -276,6 +278,8 @@ static PyObject *find_first(PyCsonPathObject *self, PyObject* args) BAD_ARG(); PyObject *ret = csonpath_find_first(self->cp, json); + if (PyErr_Occurred()) + return NULL; Py_INCREF(ret); return ret; @@ -284,7 +288,7 @@ static PyObject *find_first(PyCsonPathObject *self, PyObject* args) static PyObject *print_instructions(PyCsonPathObject *self, PyObject *args, PyObject *kwds) { csonpath_print_instruction(self->cp); - return Py_None; + Py_RETURN_NONE; } static PyObject *callback(PyCsonPathObject *self, PyObject* args) @@ -294,6 +298,8 @@ static PyObject *callback(PyCsonPathObject *self, PyObject* args) if (!PyArg_ParseTuple(args, "OO|O", &json, &callback, &udata)) BAD_ARG(); int ret = csonpath_callback(self->cp, json, callback, udata); + if (PyErr_Occurred()) + return NULL; return PyLong_FromLong(ret); } @@ -304,6 +310,8 @@ static PyObject *do_remove(PyCsonPathObject *self, PyObject* args) if (!PyArg_ParseTuple(args, "O", &json)) BAD_ARG(); int ret = csonpath_remove(self->cp, json); + if (PyErr_Occurred()) + return NULL; return PyLong_FromLong(ret); } @@ -313,10 +321,11 @@ static PyObject *update_or_create(PyCsonPathObject *self, PyObject* args) PyObject *value; if (!PyArg_ParseTuple(args, "OO", &json, &value)) - BAD_ARG(); + BAD_ARG(); int ret = csonpath_update_or_create(self->cp, json, value); - if (ret < 0) + if (PyErr_Occurred() || ret < 0) { return NULL; + } return PyLong_FromLong(ret); } @@ -327,6 +336,9 @@ static PyObject *update_or_create_callback(PyCsonPathObject *self, PyObject* arg if (!PyArg_ParseTuple(args, "OO|O", &json, &callback, &udata)) BAD_ARG(); int ret = csonpath_update_or_create_callback(self->cp, json, callback, udata); + if (PyErr_Occurred() || ret < 0) { + return NULL; + } return PyLong_FromLong(ret); } diff --git a/pyproject.toml b/pyproject.toml index 1d60c4d..d5d279b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "csonpath" -version = "0.12.1" +version = "0.13.1" readme = "README.md" [tool.setuptools] @@ -27,7 +27,7 @@ test-command = "pytest {project}/tests/python" before-build = "choco install mingw -y" [tool.bumpversion] -current_version = "0.12.1" +current_version = "0.13.1" parse = "(?P\\d+)\\.(?P\\d+)\\.(?P\\d+)" serialize = ["{major}.{minor}.{patch}"] search = "{current_version}" diff --git a/tests/json-c/filter-and-missing-key.c b/tests/json-c/filter-and-missing-key.c new file mode 100644 index 0000000..5cac14b --- /dev/null +++ b/tests/json-c/filter-and-missing-key.c @@ -0,0 +1,48 @@ +#include +#include +#include "csonpath_json-c.h" + +/* + * Regression test for a bug in compound filter (&) handling. + * + * When iterating over an array where some elements lack the key used + * in the second filter condition, the internal `filter_next` offset + * was not reset between array elements. This caused the filter walker + * to read past the end of the first filter's bytecode and hit an + * unexpected instruction (e.g. FILTER_OPERAND_STR). + * + * In the Python binding this manifests as a segfault because + * CSONPATH_NULL is Py_None (not NULL), so the corrupted walker + * dereferences a NULL el2 inside PyUnicode_Check. + * + * In the json-c binding the same logical corruption happens but + * CSONPATH_NULL == NULL, so the code safely returns "no match" + * after printing an error message. This test therefore verifies + * the *correct* behaviour (exactly one match, no error output). + */ +int main(void) +{ + const char *json_str = "{" + "\"hardware\": [" + "{\"type\": \"fb\", \"status\": \"healthy\"}," + "{\"type\": \"fb\"}" + "]" + "}"; + + struct json_object *jobj = json_tokener_parse(json_str); + assert(jobj); + + struct csonpath *cp = csonpath_new("$.hardware[?type = \"fb\" & status = \"healthy\"]"); + assert(cp); + + struct json_object *ret = csonpath_find_all(cp, jobj); + assert(ret); + + /* Must return exactly one element (the first one). */ + assert(json_object_array_length(ret) == 1); + + json_object_put(jobj); + csonpath_destroy(cp); + + return 0; +} diff --git a/tests/json-c/get-array-big-index.c b/tests/json-c/get-array-big-index.c new file mode 100644 index 0000000..0423791 --- /dev/null +++ b/tests/json-c/get-array-big-index.c @@ -0,0 +1,44 @@ +#include +#include +#include +#include "csonpath_json-c.h" + +/* + * Regression test for a bug in GET_ARRAY_BIG (index >= 100) handling. + * + * The old code used "walker += 5" for GET_ARRAY_BIG. Because the loop + * also does "++walker" at "next_inst:", the walker over-advanced by one + * byte, skipping the next instruction entirely. + * + * This test uses an existing array with 101 elements so that + * update_or_create does not need to create intermediate containers + * (avoiding reference-count issues in the json-c binding). + */ +int main(void) +{ + /* Build { "a": [ { "b": "wrong" }, ..., { "b": "hello" } ] } */ + struct json_object *root = json_object_new_object(); + struct json_object *arr = json_object_new_array(); + for (int i = 0; i < 101; ++i) { + struct json_object *obj = json_object_new_object(); + if (i == 100) + json_object_object_add(obj, "b", json_object_new_string("hello")); + else + json_object_object_add(obj, "b", json_object_new_string("wrong")); + json_object_array_add(arr, obj); + } + json_object_object_add(root, "a", arr); + + struct csonpath *cp = csonpath_new("$.a[100].b"); + assert(cp); + + struct json_object *ret = csonpath_find_first(cp, root); + assert(ret != NULL); + assert(!strcmp(json_object_get_string(ret), "hello")); + + csonpath_destroy(cp); + json_object_put(root); + + printf("OK\n"); + return 0; +} diff --git a/tests/python/test_error.py b/tests/python/test_error.py index 5701152..8167052 100644 --- a/tests/python/test_error.py +++ b/tests/python/test_error.py @@ -8,18 +8,87 @@ def broken_function(): raise Exception("This is broken") -class CsonPathError(unittest.TestCase): - def test_error(self): - with self.assertRaises(ValueError): - o = csonpath.CsonPath("$.[]") +class CsonPathUpdateOrCreateError(unittest.TestCase): + + # --- root --- + def test_uoc_root_scalar(self): + o = csonpath.CsonPath("$") + for inp in (42, "hello", 1): + with self.subTest(inp=inp): + with self.assertRaises(ValueError) as cm: + o.update_or_create(inp, "x") + self.assertIn("can't update root ($)", str(cm.exception)) def test_update_self(self): o = csonpath.CsonPath("$") d = {"c": "not-useful"} e = "hej hej" - with self.assertRaises(ValueError): + with self.assertRaises(ValueError) as cm: o.update_or_create(d, e) + self.assertIn("can't update root ($)", str(cm.exception)) + + # --- parents scalaires intermédiaires --- + def test_uoc_scalar_then_dict_access(self): + o = csonpath.CsonPath("$.a.b") + with self.assertRaises(ValueError) as cm: + o.update_or_create({"a": 1}, "x") + self.assertIn("Dict expected", str(cm.exception)) + + def test_uoc_scalar_then_array_access(self): + o = csonpath.CsonPath("$.a[0]") + with self.assertRaises(ValueError) as cm: + o.update_or_create({"a": 1}, "x") + self.assertIn("List expected", str(cm.exception)) + + def test_uoc_string_then_dict_access(self): + o = csonpath.CsonPath("$.a.b") + with self.assertRaises(ValueError) as cm: + o.update_or_create({"a": "x"}, "x") + self.assertIn("Dict expected", str(cm.exception)) + + def test_uoc_string_then_array_access(self): + o = csonpath.CsonPath("$.a[0]") + with self.assertRaises(ValueError) as cm: + o.update_or_create({"a": "x"}, "x") + self.assertIn("List expected", str(cm.exception)) + + def test_uoc_nested_scalar(self): + o = csonpath.CsonPath("$.a.b.c") + with self.assertRaises(ValueError) as cm: + o.update_or_create({"a": {"b": 1}}, "x") + self.assertIn("Dict expected", str(cm.exception)) + + def test_uoc_list_parent_for_obj(self): + o = csonpath.CsonPath("$.a.b") + with self.assertRaises(ValueError) as cm: + o.update_or_create({"a": []}, "x") + self.assertIn("Dict expected", str(cm.exception)) + + # --- subpath --- + def test_uoc_subpath_returns_array(self): + o = csonpath.CsonPath("$.a[$.c]") + with self.assertRaises(ValueError) as cm: + o.update_or_create({"a": [1], "c": [1]}, "x") + self.assertEqual( + "GET_SUBPATH return need to be either number or str", + str(cm.exception), + ) + + def test_uoc_subpath_returns_dict(self): + o = csonpath.CsonPath("$.a[$.c]") + with self.assertRaises(ValueError) as cm: + o.update_or_create({"a": [0,1], "c": {"x": 1}}, "x") + self.assertEqual( + "GET_SUBPATH return need to be either number or str", + str(cm.exception), + ) + + +class CsonPathError(unittest.TestCase): + def test_error(self): + with self.assertRaises(ValueError): + o = csonpath.CsonPath("$.[]") def test_wrong_path(self): path = "$.a.b.c" diff --git a/tests/python/test_filter.py b/tests/python/test_filter.py index 6dc5a95..7d851b1 100644 --- a/tests/python/test_filter.py +++ b/tests/python/test_filter.py @@ -94,3 +94,15 @@ def test_multiple_filters(): ret = cp.find_first(dict) assert ret is None + + +def test_filter_on_none(): + cp = csonpath.CsonPath('$.ar[?(@.a==1)]') + assert cp.find_first(None) is None + assert cp.find_all(None) is None + + +def test_filter_on_scalar(): + cp = csonpath.CsonPath('$.ar[?(@.a==1)]') + assert cp.find_first(42) is None + assert cp.find_all(42) is None