[Asterisk-code-review] jansson-bundled: Add patches to improve json pack error repo... (asterisk[master])
Joshua Colp
asteriskteam at digium.com
Mon Oct 1 06:24:47 CDT 2018
Joshua Colp has submitted this change and it was merged. ( 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, 317 insertions(+), 0 deletions(-)
Approvals:
Richard Mudgett: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, approved; Approved for Submit
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..fe86db8
--- /dev/null
+++ b/third-party/jansson/patches/0029-json_pack-Improve-handling-of-formats-with-and.patch
@@ -0,0 +1,217 @@
+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
+ '*'.
+
+Test updates have been removed for easier merging for bundled build.
+
+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 *);
+--
+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..debb2f5
--- /dev/null
+++ b/third-party/jansson/patches/0030-More-work-on-json_pack-error-reporting.patch
@@ -0,0 +1,100 @@
+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.
+
+Test updates have been removed for easier merging for bundled build.
+
+* 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);
+--
+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: merged
Gerrit-Change-Id: I045e420d5e73e60639079246e810da6ae21ae22b
Gerrit-Change-Number: 10319
Gerrit-PatchSet: 2
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2 (1000185)
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20181001/084d90be/attachment-0001.html>
More information about the asterisk-code-review
mailing list