[Asterisk-code-review] jansson-bundled: Add patches to improve json pack error repo... (asterisk[master])
Corey Farrell
asteriskteam at digium.com
Fri Sep 28 14:46:56 CDT 2018
Corey Farrell has uploaded this change for review. ( https://gerrit.asterisk.org/10319
Change subject: jansson-bundled: Add patches to improve json_pack error reporting.
......................................................................
jansson-bundled: Add patches to improve json_pack error reporting.
Change-Id: I045e420d5e73e60639079246e810da6ae21ae22b
---
A third-party/jansson/patches/0029-json_pack-Improve-handling-of-formats-with-and.patch
A third-party/jansson/patches/0030-More-work-on-json_pack-error-reporting.patch
2 files changed, 503 insertions(+), 0 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/19/10319/1
diff --git a/third-party/jansson/patches/0029-json_pack-Improve-handling-of-formats-with-and.patch b/third-party/jansson/patches/0029-json_pack-Improve-handling-of-formats-with-and.patch
new file mode 100644
index 0000000..c5900fe
--- /dev/null
+++ b/third-party/jansson/patches/0029-json_pack-Improve-handling-of-formats-with-and.patch
@@ -0,0 +1,323 @@
+From 5df5fc5b13cac5212482d36e7f3a78951782cfb5 Mon Sep 17 00:00:00 2001
+From: Corey Farrell <git at cfware.com>
+Date: Tue, 25 Sep 2018 14:31:56 -0400
+Subject: [PATCH 29/30] 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, "<args>", json_error_null_value, "NULL string argument");
++ if (!optional) {
++ set_error(s, "<args>", 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, "<args>", 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, "<format>", 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, "<args>", json_error_null_value, "NULL string argument");
++ set_error(s, "<args>", 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, "<format>", 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, "<args>", 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, "<format>", 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", "<format>", 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", "<format>", 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 '*'", "<format>", 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", "<format>", 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", "<format>", 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 '*'", "<format>", 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", "<args>", 1, 1, 1);
++ check_error(json_error_null_value, "NULL string", "<args>", 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", "<args>", 1, 2, 2);
++ check_error(json_error_null_value, "NULL object key", "<args>", 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", "<args>", 1, 4, 4);
++ check_error(json_error_null_value, "NULL string", "<args>", 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", "<args>", 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", "<args>", 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", "<args>", 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");
+--
+2.17.1
+
diff --git a/third-party/jansson/patches/0030-More-work-on-json_pack-error-reporting.patch b/third-party/jansson/patches/0030-More-work-on-json_pack-error-reporting.patch
new file mode 100644
index 0000000..0624f0b
--- /dev/null
+++ b/third-party/jansson/patches/0030-More-work-on-json_pack-error-reporting.patch
@@ -0,0 +1,180 @@
+From 8d659113d53d7ef60eae6a6e2c5b0ecfc89fc74b Mon Sep 17 00:00:00 2001
+From: Corey Farrell <git at cfware.com>
+Date: Tue, 25 Sep 2018 17:34:25 -0400
+Subject: [PATCH 30/30] More work on json_pack error reporting.
+
+* Remove errant line-feed from pack_object error message.
+* Correct error message in pack_object_inter.
+* Create pack_integer / pack_real to get the correct error messages on
+ failure when packing numeric values.
+* Add tests for packing NAN and infinity directly, in an array and as
+ an object value.
+---
+ src/pack_unpack.c | 46 +++++++++++++++++++++++++++----
+ test/suites/api/test_pack.c | 54 +++++++++++++++++++++++++++++++++++--
+ 2 files changed, 93 insertions(+), 7 deletions(-)
+
+diff --git a/src/pack_unpack.c b/src/pack_unpack.c
+index fc98df4..ec04bc3 100644
+--- a/src/pack_unpack.c
++++ b/src/pack_unpack.c
+@@ -261,7 +261,7 @@ static json_t *pack_object(scanner_t *s, va_list *ap)
+ jsonp_free(key);
+
+ if(valueOptional != '*') {
+- set_error(s, "<args>", json_error_null_value, "NULL object value\n");
++ set_error(s, "<args>", json_error_null_value, "NULL object value");
+ s->has_error = 1;
+ }
+
+@@ -396,11 +396,47 @@ static json_t *pack_object_inter(scanner_t *s, va_list *ap, int need_incref)
+ break;
+ }
+
+- set_error(s, "<args>", json_error_null_value, "NULL object key");
++ set_error(s, "<args>", json_error_null_value, "NULL object");
+ s->has_error = 1;
+ return NULL;
+ }
+
++static json_t *pack_integer(scanner_t *s, json_int_t value)
++{
++ json_t *json = json_integer(value);
++
++ if (!json) {
++ set_error(s, "<internal>", json_error_out_of_memory, "Out of memory");
++ s->has_error = 1;
++ }
++
++ return json;
++}
++
++static json_t *pack_real(scanner_t *s, double value)
++{
++ /* Allocate without setting value so we can identify OOM error. */
++ json_t *json = json_real(0.0);
++
++ if (!json) {
++ set_error(s, "<internal>", json_error_out_of_memory, "Out of memory");
++ s->has_error = 1;
++
++ return NULL;
++ }
++
++ if (json_real_set(json, value)) {
++ json_decref(json);
++
++ set_error(s, "<args>", json_error_numeric_overflow, "Invalid floating point value");
++ s->has_error = 1;
++
++ return NULL;
++ }
++
++ return json;
++}
++
+ static json_t *pack(scanner_t *s, va_list *ap)
+ {
+ switch(token(s)) {
+@@ -420,13 +456,13 @@ static json_t *pack(scanner_t *s, va_list *ap)
+ return va_arg(*ap, int) ? json_true() : json_false();
+
+ case 'i': /* integer from int */
+- return json_integer(va_arg(*ap, int));
++ return pack_integer(s, va_arg(*ap, int));
+
+ case 'I': /* integer from json_int_t */
+- return json_integer(va_arg(*ap, json_int_t));
++ return pack_integer(s, va_arg(*ap, json_int_t));
+
+ case 'f': /* real */
+- return json_real(va_arg(*ap, double));
++ return pack_real(s, va_arg(*ap, double));
+
+ case 'O': /* a json_t object; increments refcount */
+ return pack_object_inter(s, ap, 1);
+diff --git a/test/suites/api/test_pack.c b/test/suites/api/test_pack.c
+index 7e1c0b5..ab3aa91 100644
+--- a/test/suites/api/test_pack.c
++++ b/test/suites/api/test_pack.c
+@@ -15,8 +15,39 @@
+ #include <string.h>
+ #include <jansson.h>
+ #include <stdio.h>
++#include <math.h>
+ #include "util.h"
+
++#ifdef INFINITY
++// This test triggers "warning C4756: overflow in constant arithmetic"
++// in Visual Studio. This warning is triggered here by design, so disable it.
++// (This can only be done on function level so we keep these tests separate)
++#ifdef _MSC_VER
++#pragma warning(push)
++#pragma warning (disable: 4756)
++#endif
++static void test_inifity()
++{
++ json_error_t error;
++
++ if (json_pack_ex(&error, 0, "f", INFINITY))
++ fail("json_pack infinity incorrectly succeeded");
++ check_error(json_error_numeric_overflow, "Invalid floating point value", "<args>", 1, 1, 1);
++
++ if (json_pack_ex(&error, 0, "[f]", INFINITY))
++ fail("json_pack infinity array element incorrectly succeeded");
++ check_error(json_error_numeric_overflow, "Invalid floating point value", "<args>", 1, 2, 2);
++
++ if (json_pack_ex(&error, 0, "{s:f}", "key", INFINITY))
++ fail("json_pack infinity object value incorrectly succeeded");
++ check_error(json_error_numeric_overflow, "Invalid floating point value", "<args>", 1, 4, 4);
++
++#ifdef _MSC_VER
++#pragma warning(pop)
++#endif
++}
++#endif // INFINITY
++
+ static void run_tests()
+ {
+ json_t *value;
+@@ -313,6 +344,25 @@ static void run_tests()
+ fail("json_pack array optional failed");
+ json_decref(value);
+
++#ifdef NAN
++ /* Invalid float values */
++ if (json_pack_ex(&error, 0, "f", NAN))
++ fail("json_pack NAN incorrectly succeeded");
++ check_error(json_error_numeric_overflow, "Invalid floating point value", "<args>", 1, 1, 1);
++
++ if (json_pack_ex(&error, 0, "[f]", NAN))
++ fail("json_pack NAN array element incorrectly succeeded");
++ check_error(json_error_numeric_overflow, "Invalid floating point value", "<args>", 1, 2, 2);
++
++ if (json_pack_ex(&error, 0, "{s:f}", "key", NAN))
++ fail("json_pack NAN object value incorrectly succeeded");
++ check_error(json_error_numeric_overflow, "Invalid floating point value", "<args>", 1, 4, 4);
++#endif
++
++#ifdef INFINITY
++ test_inifity();
++#endif
++
+ /* Whitespace; regular string */
+ value = json_pack(" s\t ", "test");
+ if(!json_is_string(value) || strcmp("test", json_string_value(value)))
+@@ -439,9 +489,9 @@ static void run_tests()
+
+ if(json_pack_ex(&error, 0, "{s:o}", "foo", NULL))
+ fail("json_pack failed to catch nullable object");
+- check_error(json_error_null_value, "NULL object key", "<args>", 1, 4, 4);
++ check_error(json_error_null_value, "NULL object", "<args>", 1, 4, 4);
+
+ if(json_pack_ex(&error, 0, "{s:O}", "foo", NULL))
+ fail("json_pack failed to catch nullable incref object");
+- check_error(json_error_null_value, "NULL object key", "<args>", 1, 4, 4);
++ check_error(json_error_null_value, "NULL object", "<args>", 1, 4, 4);
+ }
+--
+2.17.1
+
--
To view, visit https://gerrit.asterisk.org/10319
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I045e420d5e73e60639079246e810da6ae21ae22b
Gerrit-Change-Number: 10319
Gerrit-PatchSet: 1
Gerrit-Owner: Corey Farrell <git at cfware.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180928/98677406/attachment-0001.html>
More information about the asterisk-code-review
mailing list