Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Variadic Object Traversal #688

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
7 changes: 7 additions & 0 deletions doc/apiref.rst
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,13 @@ allowed in object keys.
Get a value corresponding to *key* from *object*. Returns *NULL* if
*key* is not found and on error.

.. function:: json_t *json_object_get_path(const json_t* object, size_t depth, ...)

.. refcounting:: borrow

Get a value corresponding to the path from *object*. Returns *NULL* if
the object is not found and on error.

.. function:: json_t *json_object_getn(const json_t *object, const char *key, size_t key_len)

.. refcounting:: borrow
Expand Down
1 change: 1 addition & 0 deletions src/jansson.def
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ EXPORTS
json_object_size
json_object_get
json_object_getn
json_object_get_path
json_object_set_new
json_object_setn_new
json_object_set_new_nocheck
Expand Down
1 change: 1 addition & 0 deletions src/jansson.h
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ void json_object_seed(size_t seed);
size_t json_object_size(const json_t *object);
json_t *json_object_get(const json_t *object, const char *key)
JANSSON_ATTRS((warn_unused_result));
json_t *json_object_get_path(const json_t* object, size_t depth, ...);
json_t *json_object_getn(const json_t *object, const char *key, size_t key_len)
JANSSON_ATTRS((warn_unused_result));
int json_object_set_new(json_t *object, const char *key, json_t *value);
Expand Down
25 changes: 25 additions & 0 deletions src/value.c
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,31 @@ json_t *json_object_get(const json_t *json, const char *key) {
return json_object_getn(json, key, strlen(key));
}

json_t *json_object_get_path(json_t* object, size_t depth, ...){
va_list path;
size_t i = 0;

va_start(path, depth);

for (i = 0; i < depth; i++){
if(object->type == JSON_ARRAY){
object = json_array_get(object, va_arg(path, size_t));
if(!object){
return NULL;
}
continue;
}
object = json_object_get(object, va_arg(path, const char*));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for the PR!

It seems to me that the function is quite dangerous: if the json structure differs from the expected one, then the passed array index can be used as a pointer to a string.

In this case, we have no way to return an error to the user, since va args is not typed in any way.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is true, though that seems inherent to a variadic approach. printf and other functions I've found tend to have this issue regardless. I considered an approach of using a pointer array to a specific struct with appropriate typechecking but it defeated the purpose of making a convenient interface.

I largely created this function as a way of simplifying getting parts of a JSON object from a known JSON structure and thought it might be useful for other people as well. the variadic approach handles object traversal well and in my exeprience using this in my own project, I've found that if I ever mess up the location of an index in the path, I just get a null return since it's extremely unlikely that such a space in memory would have a char pointer with a value that's also a key in the object that is being queried.

And even if it did, the worst case result is that it returns an unintended child of the JSON which is still relatively safe.

I've also passed a char* as an index and just had null returns since it's unlikely that the array in question is large enough that the pointer in question is a valid index.

Since any inputs to this function are never edited or copied, simply read from the impact on other parts of the memory should be effectively null.

Granted this is obviously not intended for any application where the path is passed in by a user or some outside source, since that might allow a bad actor to pass in something specifically malicious. (though it would be quite unweildy to work with regardless in this scenario as far as I'm aware)

But in the bounds of standard development where this may be used, I figure that it fails in a sufficiently safe manner even if the path is incorrect.

Beyond that if this still seems too unsafe, I'm not too concerned about it and just wanted to pass along a tool that I made since I thought it might be useful, and I'll just retain it for my own use. Thank you for your time in that case, and if you have any reccomendations on how to make it safer given the goal of this function, please let me know as I'm more than happy to learn.

if(!object){
return NULL;
}
}

va_end(path);

return object;
}

json_t *json_object_getn(const json_t *json, const char *key, size_t key_len) {
json_object_t *object;

Expand Down
4 changes: 3 additions & 1 deletion test/suites/api/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ check_PROGRAMS = \
test_simple \
test_sprintf \
test_unpack \
test_version
test_version \
test_get_path

test_array_SOURCES = test_array.c util.h
test_chaos_SOURCES = test_chaos.c util.h
Expand All @@ -36,6 +37,7 @@ test_simple_SOURCES = test_simple.c util.h
test_sprintf_SOURCES = test_sprintf.c util.h
test_unpack_SOURCES = test_unpack.c util.h
test_version_SOURCES = test_version.c util.h
test_get_path_SOURCES = test_get_path.c util.h

AM_CPPFLAGS = -I$(top_builddir)/src -I$(top_srcdir)/src
LDFLAGS = -static # for speed and Valgrind
Expand Down
153 changes: 153 additions & 0 deletions test/suites/api/test_get_path.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
#include "util.h"
#include <jansson.h>
#include <string.h>

static void run_tests() {
json_error_t *error;

json_t *json = json_pack(
"{s:{s:[{}[]sifbbn],s:s,s:i,s:f,s:b,s:b,s:n,s:{s:{s:{}}}}}",
"base",
"array",
"string 1",
1,
2.2,
1,
0,
"string2",
"string3",
"integer",
3,
"real",
4.4,
"true",
1,
"false",
0,
"null",
"nest 1",
"nest 2",
"nest 3"
);

json_t* value = json_object_get_path(json, 1, "A");
if(value) fail("should not have found object");

value = json_object_get_path(json, 1, "A");
if(value) fail("should not have found array");

value = json_object_get_path(json, 1, "A");
if(value) fail("should not have found string");

value = json_object_get_path(json, 1, "A");
if(value) fail("should not have found integer");

value = json_object_get_path(json, 1, "A");
if(value) fail("should not have found true");

value = json_object_get_path(json, 1, "A");
if(value) fail("should not have found false");

value = json_object_get_path(json, 1, "A");
if(value) fail("should not have found null");

value = json_object_get_path(json, 1, "base");
if(!value) fail("did not find object");

value = json_object_get_path(json, 2, "base", "array");
if(!value) fail("did not find array");

value = json_object_get_path(json, 3, "base", "array", (size_t) 0);
if(!value) fail("did not find array element");
if(value->type != JSON_OBJECT) fail("did not find object in array");

value = json_object_get_path(json, 3, "base", "array", (size_t) 1);
if(!value) fail("did not find array element");
if(value->type != JSON_ARRAY) fail("did not find array in array");

value = json_object_get_path(json, 3, "base", "array", (size_t) 2);
if(!value) fail("did not find array element");
if(value->type != JSON_STRING) fail("did not find string in array");

value = json_object_get_path(json, 3, "base", "array", (size_t) 3);
if(!value) fail("did not find array element");
if(value->type != JSON_INTEGER) fail("did not find integer in array");

value = json_object_get_path(json, 3, "base", "array", (size_t) 4);
if(!value) fail("did not find array element");
if(value->type != JSON_REAL) fail("did not find real in array");

value = json_object_get_path(json, 3, "base", "array", (size_t) 5);
if(!value) fail("did not find array element");
if(value->type != JSON_TRUE) fail("did not find real in array");

value = json_object_get_path(json, 3, "base", "array", (size_t) 6);
if(!value) fail("did not find array element");
if(value->type != JSON_FALSE) fail("did not find real in array");

value = json_object_get_path(json, 3, "base", "array", (size_t) 7);
if(!value) fail("did not find array element");
if(value->type != JSON_NULL) fail("did not find real in array");

value = json_object_get_path(json, 3, "base", "array", (size_t) 8);
if(value) fail("should not have found array element");

value = json_object_get_path(json, 2, "base", "string2");
if(!value) fail("did not find string in object");
if(value->type != JSON_STRING) fail("did not find string in object");
if(strcmp(json_string_value(value), "string3")) fail("did not get appropriate string value");

value = json_object_get_path(json, 2, "base", "integer");
if(!value) fail("did not find integer in object");
if(value->type != JSON_INTEGER) fail("did not find integer in object");
if(json_integer_value(value) != 3) fail("did not get appropriate integer value");

value = json_object_get_path(json, 2, "base", "real");
if(!value) fail("did not find real in object");
if(value->type != JSON_REAL) fail("did not find real in object");
if(json_real_value(value)!= 4.4) fail("did not get appropriate real value");

value = json_object_get_path(json, 2, "base", "true");
if(!value) fail("did not find true in object");
if(value->type != JSON_TRUE) fail("did not find true in object");
if(!json_is_true(value)) fail("did not get appropriate true value");

value = json_object_get_path(json, 2, "base", "false");
if(!value) fail("did not find false in object");
if(value->type != JSON_FALSE) fail("did not find false in object");
if(!json_is_false(value)) fail("did not get appropriate false value");

value = json_object_get_path(json, 2, "base", "null");
if(!value) fail("did not find null in object");
if(value->type != JSON_NULL) fail("did not find null in object");

value = json_object_get_path(json, 2, "base", "null", "nest 1", "nest 2", "nest 3");
if(!value) fail("did not ignore garbage arguments");

value = json_object_get_path(json, 3, "base", "nest 1", "nest 2");
if(!value) fail("did not find nested object");

value = json_object_get_path(json, 4, "base", "nest 1", "nest 2", "nest 3");
if(!value) fail("did not find nested object");

json = json_pack(
"[sss]",
"base",
"array",
"string 1"
);

value = json_object_get_path(json, 1, (size_t) 0);
if(!value) fail("did not find object in list");
if(strcmp(json_string_value(value), "base")) fail("did not get appropriate string value");

value = json_object_get_path(json, 1, (size_t) 1);
if(!value) fail("did not find object in list");
if(strcmp(json_string_value(value), "array")) fail("did not get appropriate string value");

value = json_object_get_path(json, 1, (size_t) 2);
if(!value) fail("did not find object in list");
if(strcmp(json_string_value(value), "string 1")) fail("did not get appropriate string value");

json_decref(json);
}