[Asterisk-code-review] jansson-bundled: Add patches to improve json pack error repo... (asterisk[15])

Joshua Colp asteriskteam at digium.com
Mon Oct 1 06:24:55 CDT 2018


Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/10321 )

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/10321
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: merged
Gerrit-Change-Id: I045e420d5e73e60639079246e810da6ae21ae22b
Gerrit-Change-Number: 10321
Gerrit-PatchSet: 1
Gerrit-Owner: 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/0989458b/attachment-0001.html>


More information about the asterisk-code-review mailing list