yajl: skip garbadge fields

Fix bug with skipping of garbage fields when parsing json. All
fields, except for "id", "method" and "params", are considered
garbage fields.

Closes #132

Author: romanhabibov

src/tp_transcode.c

@@ -467,23 +467,17 @@ yajl_start_map(void *ctx)
 {
     yajl_ctx_t *s_ctx = (yajl_ctx_t *)ctx;
 
-    if (unlikely(s_ctx->stage != PARAMS))
-        return 1;
-
     stack_grow_array(s_ctx);
 
-    bool r = false;
-    if (unlikely(s_ctx->size == 0))
-        /**/
-        r = stack_push(s_ctx, s_ctx->tp.p, TYPE_MAP | PARAMS);
-    else
-        r = stack_push(s_ctx, s_ctx->tp.p, TYPE_MAP);
-
+    bool r = stack_push(s_ctx, s_ctx->tp.p, TYPE_MAP);
     if (unlikely(!r)) {
         say_error(s_ctx, -32603, "[BUG?] 'stack' overflow");
         return 0;
     }
 
+    if (unlikely(s_ctx->stage != PARAMS))
+        return 1;
+
     if (unlikely(s_ctx->tp.e < s_ctx->tp.p + 1 + sizeof(uint32_t)))
         say_overflow_r_2(s_ctx);
 
@@ -498,8 +492,8 @@ yajl_end_map(void *ctx)
 {
     yajl_ctx_t *s_ctx = (yajl_ctx_t *)ctx;
 
-    if (s_ctx->size == 0) {
-
+    stack_item_t *item = stack_pop(s_ctx);
+    if (item != NULL && s_ctx->size == 0) {
         if (!(s_ctx->been_stages & PARAMS)) {
             dd("ADDING EMPTY PARAMS\n");
 
@@ -523,19 +517,21 @@ yajl_end_map(void *ctx)
     if (unlikely(s_ctx->stage != PARAMS))
         return 1;
 
-    stack_item_t *item = stack_pop(s_ctx);
     if (likely(item != NULL)) {
-
         dd("map close, count %d '}'\n", (int)item->count);
 
-        if (unlikely(item->type & PARAMS)) {
-            say_wrong_params(s_ctx);
-            return 0;
-        }
+	/*
+	 * A map instead of an array as "params" value.
+	 *
+	 * {<...>, "params": {<...>}}
+	 */
+	if (unlikely(s_ctx->size == 1)) {
+	        say_wrong_params(s_ctx);
+		return 0;
+	}
 
         *(item->ptr++) = 0xdf;
         *(uint32_t *) item->ptr = mp_bswap_u32(item->count);
-
     } else {
         say_wrong_params(s_ctx);
         return 0;
@@ -550,30 +546,36 @@ yajl_start_array(void *ctx)
 {
     yajl_ctx_t *s_ctx = (yajl_ctx_t *)ctx;
 
+    /*
+     * Don't store a stack item for 'batching' array. This way we
+     * unify processing of both cases: when this array is present
+     * and when it does not.
+     *
+     * 1. {"method": <...>, "params": <...>, "id": <...>, <...>}
+     * 2. [
+     *        {"method": <...>, "params": <...>, "id": <...>, <...>},
+     *        {<...>}
+     *    ]
+     *
+     * All other arrays and maps are tracked in the stack.
+     */
     if (s_ctx->stage == INIT) {
         s_ctx->batch_mode_on = true;
         return 1;
     }
 
-    if (unlikely(s_ctx->stage != PARAMS))
-        return 1;
-
     dd("array open '['\n");
-
     stack_grow_array(s_ctx);
 
-    bool push_ok = false, bind_first_argument = false;
-    if (unlikely(s_ctx->size == 0)) {
-        push_ok = stack_push(s_ctx, s_ctx->tp.p, TYPE_ARRAY | PARAMS);
-        bind_first_argument = true;
-    } else
-        push_ok = stack_push(s_ctx, s_ctx->tp.p, TYPE_ARRAY);
-
+    bool push_ok = stack_push(s_ctx, s_ctx->tp.p, TYPE_ARRAY);
     if (unlikely(!push_ok)) {
         say_error(s_ctx, -32603, "[BUG?] 'stack' overflow");
         return 0;
     }
 
+    if (unlikely(s_ctx->stage != PARAMS))
+        return 1;
+
     if (unlikely(s_ctx->tp.e < (s_ctx->tp.p + 1 + sizeof(uint32_t))))
         say_overflow_r_2(s_ctx);
 
@@ -582,7 +584,7 @@ yajl_start_array(void *ctx)
     // Here is bind data
     // e.g. http request
     // [
-    if (bind_first_argument) {
+    if (s_ctx->size == 2) {
         if (unlikely(!bind_data(s_ctx)))
             say_overflow_r_2(s_ctx);
     }
@@ -596,16 +598,22 @@ yajl_end_array(void *ctx)
 {
     yajl_ctx_t *s_ctx = (yajl_ctx_t *)ctx;
 
+    stack_item_t *item = stack_pop(s_ctx);
+
     if (unlikely(s_ctx->stage != PARAMS))
         return 1;
 
-    stack_item_t *item = stack_pop(s_ctx);
     if (likely(item != NULL)) {
-
         dd("array close, count %d ']'\n", item->count);
 
         size_t item_count = item->count;
-        if (unlikely(item->type & PARAMS)) {
+
+	/*
+	 * An end of "params" array.
+	 *
+	 * {<...>, "params": [<...>]}.
+	 */
+        if (unlikely(s_ctx->size == 1)) {
             dd("PARAMS END\n");
             s_ctx->stage = WAIT_NEXT;
             s_ctx->been_stages |= PARAMS;

test/test.lua

@@ -5,6 +5,10 @@ yaml = require('yaml')
 os   = require('os')
 fiber = require('fiber')
 
+function echo(...)
+  return ...
+end
+
 function echo_1(a)
   return {a}
 end

test/v27_features.py

@@ -3,6 +3,7 @@
 
 import sys
 import time
+from collections import OrderedDict
 sys.path.append('./t')
 from http_utils import *
 
@@ -20,3 +21,88 @@
 assert(result['uri'] == '/issue_120/{}'), "expected result"
 print ('[+] OK')
 
+print('[+] Garbage fields skipping.')
+preset_method_location = BASE_URL + '/echo'
+exp_res = {u'id': 1, u'result': [u'param']}
+
+skipping_map_at_the_beginning = OrderedDict([
+  ("skipping_map_at_the_beginning", {"ttl": 123}),
+  ("id", 1),
+  ("method", "echo"),
+  ("params", ["param"])
+])
+result = post_success_pure(preset_method_location, skipping_map_at_the_beginning, {})
+assert (result == exp_res), "skipping_map_at_the_beginning"
+
+skipping_map_in_the_middle = OrderedDict([
+  ("id", 1),
+  ("skipping_map_in_the_middle", {"ttl": 123}),
+  ("method", "echo"),
+  ("params", ["param"])
+])
+result = post_success_pure(preset_method_location, skipping_map_in_the_middle, {})
+assert (result == exp_res), "skipping_map_in_the_middle"
+
+skipping_map_in_the_end = OrderedDict([
+  ("id", 1),
+  ("method", "echo"),
+  ("params", ["param"]),
+  ("skipping_map_in_the_end", {"ttl": 123})
+])
+result = post_success_pure(preset_method_location, skipping_map_in_the_end, {})
+assert (result == exp_res), "skipping_map_in_the_end"
+
+skipping_array_at_the_beginning = OrderedDict([
+  ("skipping_array_at_the_beginning", ["ttl", 123]),
+  ("id", 1),
+  ("method", "echo"),
+  ("params", ["param"]),
+  ("meta", ["ttl", 123])
+])
+result = post_success_pure(preset_method_location, skipping_array_at_the_beginning, {})
+assert (result == exp_res), "skipping_array_at_the_beginning"
+
+skipping_array_in_the_middle = OrderedDict([
+  ("id", 1),
+  ("method", "echo"),
+  ("skipping_array_in_the_middle", ["ttl", 123]),
+  ("params", ["param"])
+])
+result = post_success_pure(preset_method_location, skipping_array_in_the_middle, {})
+assert (result == exp_res), "skipping_array_in_the_middle"
+
+skipping_array_in_the_end = OrderedDict([
+  ("id", 1),
+  ("method", "echo"),
+  ("params", ["param"]),
+  ("skipping_array_in_the_end", ["ttl", 123])
+])
+result = post_success_pure(preset_method_location, skipping_array_in_the_end, {})
+assert (result == exp_res), "skipping_array_in_the_end"
+
+skipping_map_with_nesting_1 = OrderedDict([
+  ("skipping_map_with_nesting_1", {"ttl": {"ttl": 123, "array":[]}}),
+  ("id", 1),
+  ("method", "echo"),
+  ("params", ["param"])
+])
+result = post_success_pure(preset_method_location, skipping_map_with_nesting_1, {})
+assert (result == exp_res), "skipping_map_with_nesting_1"
+
+skipping_array_with_nesting_2 = OrderedDict([
+  ("skipping_array_with_nesting_2", ["ttl", ["ttl", {"map": 123}]]),
+  ("id", 1),
+  ("method", "echo"),
+  ("params", ["param"])
+])
+result = post_success_pure(preset_method_location, skipping_array_with_nesting_2, {})
+assert (result == exp_res), "skipping_array_with_nesting_2"
+
+batch = [skipping_map_at_the_beginning, skipping_map_in_the_middle,
+         skipping_map_in_the_end, skipping_array_at_the_beginning,
+         skipping_array_in_the_middle, skipping_array_in_the_end,
+         skipping_map_with_nesting_1, skipping_array_with_nesting_2]
+result = post_success_pure(preset_method_location, batch, {})
+for item in result:
+  assert (item == exp_res), "batch"
+print('[+] OK')