Skip to content
Snippets Groups Projects
0029-json_pack-Improve-handling-of-formats-with-and.patch 6.67 KiB
From 5df5fc5b13cac5212482d36e7f3a78951782cfb5 Mon Sep 17 00:00:00 2001
From: Corey Farrell <git@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