From 5df5fc5b13cac5212482d36e7f3a78951782cfb5 Mon Sep 17 00:00:00 2001 From: Corey Farrell Date: Tue, 25 Sep 2018 14:31:56 -0400 Subject: [PATCH] json_pack: Improve handling of formats with '?' and '*'. When NULL is received for an optional argument we should not set an error message as this would block later error messages. If NULL is received for a non-optional string we should set has_error. Set has_error for UTF-8 errors to ensure optional strings with UTF-8 errors are not replaced with json_null(). Use 'purpose' argument in NULL error messages of read_string. Add error handling and tests for invalid formats where '+', '#', or '%' is used on an optional string 's?' or 's*'. Fix NULL string error messages to use 'purpose'. Refactor skipping of '*' token, this is now handled by read_string and pack_object_inter. This allows invalid format strings such as 's*#' and 's*+' to produce error messages. Fixes #437 --- src/pack_unpack.c | 74 +++++++++++++++++++++++-------------- test/suites/api/test_pack.c | 49 ++++++++++++++++++++++-- 2 files changed, 93 insertions(+), 30 deletions(-) diff --git a/src/pack_unpack.c b/src/pack_unpack.c index b842772..fc98df4 100644 --- a/src/pack_unpack.c +++ b/src/pack_unpack.c @@ -130,7 +130,7 @@ static json_t *pack(scanner_t *s, va_list *ap); /* ours will be set to 1 if jsonp_free() must be called for the result afterwards */ static char *read_string(scanner_t *s, va_list *ap, - const char *purpose, size_t *out_len, int *ours) + const char *purpose, size_t *out_len, int *ours, int optional) { char t; strbuffer_t strbuff; @@ -147,7 +147,10 @@ static char *read_string(scanner_t *s, va_list *ap, str = va_arg(*ap, const char *); if(!str) { - set_error(s, "", json_error_null_value, "NULL string argument"); + if (!optional) { + set_error(s, "", json_error_null_value, "NULL %s", purpose); + s->has_error = 1; + } return NULL; } @@ -155,11 +158,17 @@ static char *read_string(scanner_t *s, va_list *ap, if(!utf8_check_string(str, length)) { set_error(s, "", json_error_invalid_utf8, "Invalid UTF-8 %s", purpose); + s->has_error = 1; return NULL; } *out_len = length; return (char *)str; + } else if (optional) { + set_error(s, "", json_error_invalid_format, "Cannot use '%c' on optional strings", t); + s->has_error = 1; + + return NULL; } if(strbuffer_init(&strbuff)) { @@ -170,7 +179,7 @@ static char *read_string(scanner_t *s, va_list *ap, while(1) { str = va_arg(*ap, const char *); if(!str) { - set_error(s, "", json_error_null_value, "NULL string argument"); + set_error(s, "", json_error_null_value, "NULL %s", purpose); s->has_error = 1; } @@ -226,6 +235,7 @@ static json_t *pack_object(scanner_t *s, va_list *ap) size_t len; int ours; json_t *value; + char valueOptional; if(!token(s)) { set_error(s, "", json_error_invalid_format, "Unexpected end of format string"); @@ -237,20 +247,21 @@ static json_t *pack_object(scanner_t *s, va_list *ap) goto error; } - key = read_string(s, ap, "object key", &len, &ours); - if (!key) - s->has_error = 1; + key = read_string(s, ap, "object key", &len, &ours, 0); next_token(s); + next_token(s); + valueOptional = token(s); + prev_token(s); + value = pack(s, ap); if(!value) { if(ours) jsonp_free(key); - if(strchr("soO", token(s)) && s->next_token.token == '*') { - next_token(s); - } else { + if(valueOptional != '*') { + set_error(s, "", json_error_null_value, "NULL object value\n"); s->has_error = 1; } @@ -269,8 +280,6 @@ static json_t *pack_object(scanner_t *s, va_list *ap) if(ours) jsonp_free(key); - if(strchr("soO", token(s)) && s->next_token.token == '*') - next_token(s); next_token(s); } @@ -289,6 +298,7 @@ static json_t *pack_array(scanner_t *s, va_list *ap) while(token(s) != ']') { json_t *value; + char valueOptional; if(!token(s)) { set_error(s, "", json_error_invalid_format, "Unexpected end of format string"); @@ -296,11 +306,13 @@ static json_t *pack_array(scanner_t *s, va_list *ap) goto error; } + next_token(s); + valueOptional = token(s); + prev_token(s); + value = pack(s, ap); if(!value) { - if(strchr("soO", token(s)) && s->next_token.token == '*') { - next_token(s); - } else { + if(valueOptional != '*') { s->has_error = 1; } @@ -316,8 +328,6 @@ static json_t *pack_array(scanner_t *s, va_list *ap) s->has_error = 1; } - if(strchr("soO", token(s)) && s->next_token.token == '*') - next_token(s); next_token(s); } @@ -332,23 +342,33 @@ error: static json_t *pack_string(scanner_t *s, va_list *ap) { char *str; + char t; size_t len; int ours; - int nullable; + int optional; next_token(s); - nullable = token(s) == '?'; - if (!nullable) + t = token(s); + optional = t == '?' || t == '*'; + if (!optional) prev_token(s); - str = read_string(s, ap, "string", &len, &ours); - if (!str) { - return nullable ? json_null() : NULL; - } else if (ours) { - return jsonp_stringn_nocheck_own(str, len); - } else { - return json_stringn_nocheck(str, len); + str = read_string(s, ap, "string", &len, &ours, optional); + + if (!str) + return t == '?' && !s->has_error ? json_null() : NULL; + + if (s->has_error) { + if (!ours) + jsonp_free(str); + + return NULL; } + + if (ours) + return jsonp_stringn_nocheck_own(str, len); + + return json_stringn_nocheck(str, len); } static json_t *pack_object_inter(scanner_t *s, va_list *ap, int need_incref) @@ -359,7 +379,7 @@ static json_t *pack_object_inter(scanner_t *s, va_list *ap, int need_incref) next_token(s); ntoken = token(s); - if (ntoken != '?') + if (ntoken != '?' && ntoken != '*') prev_token(s); json = va_arg(*ap, json_t *); diff --git a/test/suites/api/test_pack.c b/test/suites/api/test_pack.c index a1e8e01..7e1c0b5 100644 --- a/test/suites/api/test_pack.c +++ b/test/suites/api/test_pack.c @@ -99,6 +99,16 @@ static void run_tests() fail("json_pack nullable string (NULL case) refcount failed"); json_decref(value); + /* nullable string concatenation */ + if(json_pack_ex(&error, 0, "s?+", "test", "ing")) + fail("json_pack failed to catch invalid format 's?+'"); + check_error(json_error_invalid_format, "Cannot use '+' on optional strings", "", 1, 2, 2); + + /* nullable string with integer length */ + if(json_pack_ex(&error, 0, "s?#", "test", 4)) + fail("json_pack failed to catch invalid format 's?#'"); + check_error(json_error_invalid_format, "Cannot use '#' on optional strings", "", 1, 2, 2); + /* string and length (int) */ value = json_pack("s#", "test asdf", 4); if(!json_is_string(value) || strcmp("test", json_string_value(value))) @@ -247,14 +257,32 @@ static void run_tests() value = json_pack("{s:s,s:o,s:O}", "a", NULL, "b", NULL, "c", NULL); if(value) fail("json_pack object optional incorrectly succeeded"); + value = json_pack("{s:**}", "a", NULL); if(value) fail("json_pack object optional invalid incorrectly succeeded"); + + if (json_pack_ex(&error, 0, "{s:i*}", "a", 1)) + fail("json_pack object optional invalid incorrectly succeeded"); + check_error(json_error_invalid_format, "Expected format 's', got '*'", "", 1, 5, 5); + value = json_pack("{s:s*,s:o*,s:O*}", "a", NULL, "b", NULL, "c", NULL); if(!json_is_object(value) || json_object_size(value) != 0) fail("json_pack object optional failed"); json_decref(value); + value = json_pack("{s:s*}", "key", "\xff\xff"); + if(value) + fail("json_pack object optional with invalid UTF-8 incorrectly succeeded"); + + if(json_pack_ex(&error, 0, "{s: s*#}", "key", "test", 1)) + fail("json_pack failed to catch invalid format 's*#'"); + check_error(json_error_invalid_format, "Cannot use '#' on optional strings", "", 1, 6, 6); + + if(json_pack_ex(&error, 0, "{s: s*+}", "key", "test", "ing")) + fail("json_pack failed to catch invalid format 's*+'"); + check_error(json_error_invalid_format, "Cannot use '+' on optional strings", "", 1, 6, 6); + /* simple array */ value = json_pack("[i,i,i]", 0, 1, 2); if(!json_is_array(value) || json_array_size(value) != 3) @@ -272,6 +300,11 @@ static void run_tests() value = json_pack("[s,o,O]", NULL, NULL, NULL); if(value) fail("json_pack array optional incorrectly succeeded"); + + if (json_pack_ex(&error, 0, "[i*]", 1)) + fail("json_pack array optional invalid incorrectly succeeded"); + check_error(json_error_invalid_format, "Unexpected format character '*'", "", 1, 3, 3); + value = json_pack("[**]", NULL); if(value) fail("json_pack array optional invalid incorrectly succeeded"); @@ -338,7 +371,7 @@ static void run_tests() /* NULL string */ if(json_pack_ex(&error, 0, "s", NULL)) fail("json_pack failed to catch null argument string"); - check_error(json_error_null_value, "NULL string argument", "", 1, 1, 1); + check_error(json_error_null_value, "NULL string", "", 1, 1, 1); /* + on its own */ if(json_pack_ex(&error, 0, "+", NULL)) @@ -353,13 +386,13 @@ static void run_tests() /* NULL key */ if(json_pack_ex(&error, 0, "{s:i}", NULL, 1)) fail("json_pack failed to catch NULL key"); - check_error(json_error_null_value, "NULL string argument", "", 1, 2, 2); + check_error(json_error_null_value, "NULL object key", "", 1, 2, 2); /* NULL value followed by object still steals the object's ref */ value = json_incref(json_object()); if(json_pack_ex(&error, 0, "{s:s,s:o}", "badnull", NULL, "dontleak", value)) fail("json_pack failed to catch NULL value"); - check_error(json_error_null_value, "NULL string argument", "", 1, 4, 4); + check_error(json_error_null_value, "NULL string", "", 1, 4, 4); if(value->refcount != (size_t)1) fail("json_pack failed to steal reference after error."); json_decref(value); @@ -389,6 +422,16 @@ static void run_tests() fail("json_pack failed to catch invalid UTF-8 in a string"); check_error(json_error_invalid_utf8, "Invalid UTF-8 string", "", 1, 4, 4); + /* Invalid UTF-8 in an optional '?' string */ + if(json_pack_ex(&error, 0, "{s:s?}", "foo", "\xff\xff")) + fail("json_pack failed to catch invalid UTF-8 in an optional '?' string"); + check_error(json_error_invalid_utf8, "Invalid UTF-8 string", "", 1, 5, 5); + + /* Invalid UTF-8 in an optional '*' string */ + if(json_pack_ex(&error, 0, "{s:s*}", "foo", "\xff\xff")) + fail("json_pack failed to catch invalid UTF-8 in an optional '*' string"); + check_error(json_error_invalid_utf8, "Invalid UTF-8 string", "", 1, 5, 5); + /* Invalid UTF-8 in a concatenated key */ if(json_pack_ex(&error, 0, "{s+:i}", "\xff\xff", "concat", 42)) fail("json_pack failed to catch invalid UTF-8 in an object key");