Cover JSON.load edge cases

ohler55 · Oct 2, 2017 · 5c40e67 · 5c40e67
1 parent 862fa8b
commit 5c40e67
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 17 deletions.
diff --git a/ext/oj/mimic_json.c b/ext/oj/mimic_json.c
@@ -795,6 +795,7 @@ oj_mimic_json_methods(VALUE json) {
     } else {
     	oj_json_parser_error_class = rb_define_class_under(json, "ParserError", json_error);
     }
+    printf("*** setting parse error to %lx %s\n", oj_json_parser_error_class, rb_class2name(oj_json_parser_error_class));
     if (rb_const_defined_at(json, rb_intern("GeneratorError"))) {
         oj_json_generator_error_class = rb_const_get(json, rb_intern("GeneratorError"));
     } else {

diff --git a/ext/oj/parse.c b/ext/oj/parse.c
@@ -921,8 +921,10 @@ oj_pi_parse(int argc, VALUE *argv, ParseInfo pi, char *json, size_t len, int yie
     // value stack (while it is in scope).
     wrapped_stack = oj_stack_init(&pi->stack);
     rb_protect(protect_parse, (VALUE)pi, &line);
-    if (Qundef == pi->stack.head->val && !empty_ok(&pi->options) && No == pi->options.nilnil) {
-	oj_set_error_at(pi, oj_parse_error_class, __FILE__, __LINE__, "Empty input");
+    if (Qundef == pi->stack.head->val && !empty_ok(&pi->options)) {
+	if (No == pi->options.nilnil || (CompatMode == pi->options.mode && 0 < pi->cur - pi->json)) {
+	    oj_set_error_at(pi, oj_json_parser_error_class, __FILE__, __LINE__, "Empty input");
+	}
     }
     result = stack_head_val(&pi->stack);
     DATA_PTR(wrapped_stack) = 0;

diff --git a/ext/oj/parse.h b/ext/oj/parse.h
@@ -93,15 +93,15 @@ parse_info_init(ParseInfo pi) {
 static inline bool
 empty_ok(Options options) {
     switch (options->mode) {
-    case StrictMode:
-    case NullMode:
     case ObjectMode:
-    case CustomMode:
     case WabMode:
 	return true;
     case CompatMode:
     case RailsMode:
 	return false;
+    case StrictMode:
+    case NullMode:
+    case CustomMode:
     default:
 	break;
     }

diff --git a/pages/Options.md b/pages/Options.md
@@ -91,18 +91,21 @@ specifying the class for an encoded object. The default is `json_create`.
 
 ### :empty_string [Boolean]
 
-If true an empty input will not raise an Exception. The default differs
-according to the mode and in some cases the function used to load or dump. The
-defaults are:
-
- - :null - true
- - :strict - true
- - :compat or :json - true
-    - JSON.parse() - false
-    - JSON.load() - true (or what ever is set in the defaults)
- - :rails - TBD
- - :object - true
- - :custom - true
+If true an empty or all whitespace input will not raise an Exception. The
+default_options will be honored for :null, :strict, and :custom modes. Ignored
+for :custom and :wab. The :compat has a more complex set of rules. The JSON
+gem compatibility is best described by examples.
+
+```
+JSON.parse('') => raise
+JSON.parse(' ') => raise
+JSON.load('') => nil
+JSON.load('', nil, allow_blank: false) => raise
+JSON.load('', nil, allow_blank: true) => nil
+JSON.load(' ') => raise
+JSON.load(' ', nil, allow_blank: false) => raise
+JSON.load(' ', nil, allow_blank: true) => raise
+```
 
 ### :escape_mode [Symbol]
 

diff --git a/test/json_gem/json_common_interface_test.rb b/test/json_gem/json_common_interface_test.rb
@@ -112,6 +112,11 @@ def test_load_null
     assert_equal nil, JSON.load(nil, nil, :allow_blank => true)
     assert_raise(TypeError) { JSON.load(nil, nil, :allow_blank => false) }
     assert_raise(JSON::ParserError) { JSON.load('', nil, :allow_blank => false) }
+    # The next tests are added by Oj to catch additional cases.
+    assert_equal nil, JSON.load('', nil, :allow_blank => true)
+    assert_raise(JSON::ParserError) { JSON.load('', nil, :allow_blank => false) }
+    assert_raise(JSON::ParserError) { JSON.load(' ', nil, :allow_blank => true) }
+    assert_raise(JSON::ParserError) { JSON.load(' ', nil, :allow_blank => false) }
   end
 
   def test_dump