More options (#895)

* Add float_format option * Oj.dump should honor max_nesting
ohler55 · Aug 16, 2023 · 0e4e6f5 · 0e4e6f5
1 parent 56b6d84
commit 0e4e6f5
Show file tree

Hide file tree

Showing 14 changed files with 99 additions and 48 deletions.
diff --git a/.github/workflows/rubocop.yml → .github/workflows/rubocop.yml.ignore b/.github/workflows/rubocop.yml → .github/workflows/rubocop.yml.ignore
@@ -1,3 +1,8 @@
+
+# Don't bother with rubocop. It is too opinionated and the opinions
+# change. It's not worth chasing when the code is perfectly acceptable
+# otherwise.
+
 name: Rubocop Check
 
 on:

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,14 @@
 # CHANGELOG
 
+## 3.16.0 - 2023-08-16
+
+- Added the `float_format` option.
+
+- Expanded the `max_nesting` option to allow integer values as well as
+  the previous boolean (true or nil).
+
+- Skip nesting tests with Truffle Ruby in the json gem tests.
+
 ## 3.15.1 - 2023-07-30
 
 - Add protection against some using `require 'oj/json`, an internal file.

diff --git a/ext/oj/dump_compat.c b/ext/oj/dump_compat.c
@@ -851,36 +851,36 @@ static DumpFunc compat_funcs[] = {
 };
 
 static void set_state_depth(VALUE state, int depth) {
-    VALUE json_module = rb_const_get_at(rb_cObject, rb_intern("JSON"));
-    VALUE ext         = rb_const_get(json_module, rb_intern("Ext"));
-    VALUE generator   = rb_const_get(ext, rb_intern("Generator"));
-    VALUE state_class = rb_const_get(generator, rb_intern("State"));
-
-    if (state_class == rb_obj_class(state)) {
-        rb_funcall(state, rb_intern("depth="), 1, INT2NUM(depth));
-    }
+		if (0 == rb_const_defined(rb_cObject, rb_intern("JSON"))) {
+				rb_require("oj/json");
+		}
+		{
+				VALUE json_module = rb_const_get_at(rb_cObject, rb_intern("JSON"));
+				VALUE ext         = rb_const_get(json_module, rb_intern("Ext"));
+				VALUE generator   = rb_const_get(ext, rb_intern("Generator"));
+				VALUE state_class = rb_const_get(generator, rb_intern("State"));
+
+				if (state_class == rb_obj_class(state)) {
+						rb_funcall(state, rb_intern("depth="), 1, INT2NUM(depth));
+				}
+		}
 }
 
 void oj_dump_compat_val(VALUE obj, int depth, Out out, bool as_ok) {
     int type = rb_type(obj);
 
     TRACE(out->opts->trace, "dump", obj, depth, TraceIn);
+		// The max_nesting logic is that an empty Array or Hash is assumed to have
+		// content so the max_nesting should fail but a non-collection value is
+		// okay. That means a check for a collectable value is needed before
+		// raising.
     if (out->opts->dump_opts.max_depth <= depth) {
-        // When JSON.dump is called then an ArgumentError is expected and the
-        // limit is the depth inclusive. If JSON.generate is called then a
-        // NestingError is expected and the limit is inclusive. Worse than
-        // that there are unit tests for both.
-        if (CALLER_DUMP == out->caller) {
-            if (0 < out->argc) {
-                set_state_depth(*out->argv, depth);
-            }
-            rb_raise(rb_eArgError, "Too deeply nested.");
-        } else if (out->opts->dump_opts.max_depth < depth) {
-            if (0 < out->argc) {
-                set_state_depth(*out->argv, depth - 1);
-            }
-            raise_json_err("Too deeply nested", "NestingError");
-        }
+				if (RUBY_T_ARRAY == type || RUBY_T_HASH == type) {
+						if (0 < out->argc) {
+								set_state_depth(*out->argv, depth);
+						}
+						raise_json_err("Too deeply nested", "NestingError");
+				}
     }
     if (0 < type && type <= RUBY_T_FIXNUM) {
         DumpFunc f = compat_funcs[type];

diff --git a/ext/oj/mimic_json.c b/ext/oj/mimic_json.c
@@ -209,7 +209,6 @@ static VALUE mimic_dump(int argc, VALUE *argv, VALUE self) {
 
     oj_out_init(&out);
 
-    out.caller        = CALLER_DUMP;
     copts.escape_mode = JXEsc;
     copts.mode        = CompatMode;
 
@@ -368,7 +367,6 @@ static VALUE mimic_generate_core(int argc, VALUE *argv, Options copts) {
     oj_out_init(&out);
 
     out.omit_nil = copts->dump_opts.omit_nil;
-    out.caller   = CALLER_GENERATE;
     // For obj.to_json or generate nan is not allowed but if called from dump
     // it is.
     copts->dump_opts.nan_dump = RaiseNan;

diff --git a/ext/oj/oj.c b/ext/oj/oj.c
@@ -123,6 +123,7 @@ static VALUE escape_mode_sym;
 static VALUE integer_range_sym;
 static VALUE fast_sym;
 static VALUE float_prec_sym;
+static VALUE float_format_sym;
 static VALUE float_sym;
 static VALUE huge_sym;
 static VALUE ignore_sym;
@@ -232,7 +233,7 @@ struct _options oj_default_options = {
         NULL,    // tail
         {'\0'},  // err
     },
-    NULL,        // ignore
+    NULL,  // ignore
 };
 
 /* Document-method: default_options()
@@ -267,6 +268,8 @@ struct _options oj_default_options = {
  *seconds portion of time
  * - *:float_precision* [_Fixnum_|_nil_] number of digits of precision when dumping floats, 0
  *indicates use Ruby
+ * - *:float_format* [_String_] the C printf format string for printing floats. Default follows
+ * the float_precision and will be changed if float_precision is changed. The string can be no more than 6 bytes.
  * - *:use_to_json* [_Boolean_|_nil_] call to_json() methods on dump, default is false
  * - *:use_as_json* [_Boolean_|_nil_] call as_json() methods on dump, default is false
  * - *:use_raw_json* [_Boolean_|_nil_] call raw_json() methods on dump, default is false
@@ -378,6 +381,7 @@ static VALUE get_def_opts(VALUE self) {
                  oj_safe_sym,
                  (Yes == oj_default_options.safe) ? Qtrue : ((No == oj_default_options.safe) ? Qfalse : Qnil));
     rb_hash_aset(opts, float_prec_sym, INT2FIX(oj_default_options.float_prec));
+    rb_hash_aset(opts, float_format_sym, rb_str_new_cstr(oj_default_options.float_fmt));
     rb_hash_aset(opts, cache_str_sym, INT2FIX(oj_default_options.cache_str));
     rb_hash_aset(
         opts,
@@ -519,6 +523,8 @@ static VALUE get_def_opts(VALUE self) {
  *load.
  *   - *:second_precision* [_Fixnum_|_nil_] number of digits after the decimal when dumping the
  *seconds portion of time.
+ *   - *:float_format* [_String_] the C printf format string for printing floats. Default follows
+ * the float_precision and will be changed if float_precision is changed. The string can be no more than 6 bytes.
  *   - *:float_precision* [_Fixnum_|_nil_] number of digits of precision when dumping floats, 0
  *indicates use Ruby.
  *   - *:use_to_json* [_Boolean_|_nil_] call to_json() methods on dump, default is false.
@@ -617,7 +623,6 @@ static int parse_options_cb(VALUE k, VALUE v, VALUE opts) {
     if (set_yesno_options(k, v, copts)) {
         return ST_CONTINUE;
     }
-
     if (oj_indent_sym == k) {
         switch (rb_type(v)) {
         case T_NIL:
@@ -757,7 +762,6 @@ static int parse_options_cb(VALUE k, VALUE v, VALUE opts) {
         if (Qnil == v) {
             return ST_CONTINUE;
         }
-
         copts->compat_bigdec = (Qtrue == v);
     } else if (oj_decimal_class_sym == k) {
         if (rb_cFloat == v) {
@@ -949,6 +953,25 @@ static int parse_options_cb(VALUE k, VALUE v, VALUE opts) {
             return ST_CONTINUE;
         }
         copts->sym_key = (Qtrue == v) ? Yes : No;
+
+    } else if (oj_max_nesting_sym == k) {
+				if (Qtrue == v) {
+						copts->dump_opts.max_depth = 100;
+				} else if (Qfalse == v || Qnil == v) {
+						copts->dump_opts.max_depth = MAX_DEPTH;
+				} else if (T_FIXNUM == rb_type(v)) {
+						copts->dump_opts.max_depth = NUM2INT(v);
+						if (0 >= copts->dump_opts.max_depth) {
+								copts->dump_opts.max_depth = MAX_DEPTH;
+						}
+				}
+    } else if (float_format_sym == k) {
+        rb_check_type(v, T_STRING);
+        if (6 < (int)RSTRING_LEN(v)) {
+            rb_raise(rb_eArgError, ":float_format must be 6 bytes or less.");
+        }
+        strncpy(copts->float_fmt, RSTRING_PTR(v), (size_t)RSTRING_LEN(v));
+        copts->float_fmt[RSTRING_LEN(v)] = '\0';
     }
     return ST_CONTINUE;
 }
@@ -957,7 +980,6 @@ void oj_parse_options(VALUE ropts, Options copts) {
     if (T_HASH != rb_type(ropts)) {
         return;
     }
-
     rb_hash_foreach(ropts, parse_options_cb, (VALUE)copts);
     oj_parse_opt_match_string(&copts->str_rx, ropts);
 
@@ -1296,7 +1318,6 @@ static VALUE dump(int argc, VALUE *argv, VALUE self) {
 
     arg.out->omit_nil       = copts.dump_opts.omit_nil;
     arg.out->omit_null_byte = copts.dump_opts.omit_null_byte;
-    arg.out->caller         = CALLER_DUMP;
 
     return rb_ensure(dump_body, (VALUE)&arg, dump_ensure, (VALUE)&arg);
 }
@@ -1308,8 +1329,9 @@ static VALUE dump(int argc, VALUE *argv, VALUE self) {
  * will be called. The mode is set to :compat.
  * - *obj* [_Object_] Object to serialize as an JSON document String
  * - *options* [_Hash_]
- *   - *:max_nesting* [_boolean_] It true nesting is limited to 100. The option to detect circular
- * references is available but is not compatible with the json gem., default is false
+ *   - *:max_nesting* [_Fixnum_|_boolean_] It true nesting is limited to 100. If a Fixnum nesting
+ * is set to the provided value. The option to detect circular references is available but is not
+ * compatible with the json gem., default is false or unlimited.
  *   - *:allow_nan* [_boolean_] If true non JSON compliant words such as Nan and Infinity will be
  * used as appropriate, default is true.
  *   - *:quirks_mode* [_boolean_] Allow single JSON values instead of documents, default is true
@@ -1929,6 +1951,8 @@ void Init_oj(void) {
     rb_gc_register_address(&integer_range_sym);
     fast_sym = ID2SYM(rb_intern("fast"));
     rb_gc_register_address(&fast_sym);
+    float_format_sym = ID2SYM(rb_intern("float_format"));
+    rb_gc_register_address(&float_format_sym);
     float_prec_sym = ID2SYM(rb_intern("float_precision"));
     rb_gc_register_address(&float_prec_sym);
     float_sym = ID2SYM(rb_intern("float"));

diff --git a/ext/oj/oj.h b/ext/oj/oj.h
@@ -103,13 +103,6 @@ typedef enum {
     FILE_IO   = 'f',
 } StreamWriterType;
 
-typedef enum {
-    CALLER_DUMP     = 'd',
-    CALLER_TO_JSON  = 't',
-    CALLER_GENERATE = 'g',
-    // Add the fast versions if necessary. Maybe unparse as well if needed.
-} DumpCaller;
-
 typedef struct _dumpOpts {
     bool    use;
     char    indent_str[16];
@@ -203,7 +196,6 @@ typedef struct _out {
     bool       omit_null_byte;
     int        argc;
     VALUE     *argv;
-    DumpCaller caller;  // used for the mimic json only
     ROptTable  ropts;
 } *Out;
 

diff --git a/ext/oj/rails.c b/ext/oj/rails.c
@@ -890,7 +890,6 @@ static VALUE encode(VALUE obj, ROptTable ropts, Options opts, int argc, VALUE *a
     oj_out_init(&out);
 
     out.omit_nil = copts.dump_opts.omit_nil;
-    out.caller   = 0;
     out.cur      = out.buf;
     out.circ_cnt = 0;
     out.opts     = &copts;

diff --git a/ext/oj/string_writer.c b/ext/oj/string_writer.c
@@ -70,7 +70,6 @@ void oj_str_writer_init(StrWriter sw, int buf_size) {
     sw->out.depth      = 0;
     sw->out.argc       = 0;
     sw->out.argv       = NULL;
-    sw->out.caller     = 0;
     sw->out.ropts      = NULL;
     sw->out.omit_nil   = oj_default_options.dump_opts.omit_nil;
 }

diff --git a/lib/oj/version.rb b/lib/oj/version.rb
@@ -1,4 +1,4 @@
 module Oj
   # Current version of the module.
-  VERSION = '3.15.1'
+  VERSION = '3.16.0'
 end
diff --git a/notes b/notes
@@ -7,7 +7,6 @@
  - and move to dump parameters
 
 
-
 - stream writes
  - dump.c:685
  - stream_writer.c

diff --git a/test/json_gem/json_common_interface_test.rb b/test/json_gem/json_common_interface_test.rb
@@ -128,8 +128,10 @@ def test_dump
     too_deep = '[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]'
     assert_equal too_deep, JSON.dump(eval(too_deep))
     assert_kind_of String, Marshal.dump(eval(too_deep))
-    assert_raise(ArgumentError) { JSON.dump(eval(too_deep), 100) }
-    assert_raise(ArgumentError) { Marshal.dump(eval(too_deep), 100) }
+    if RUBY_ENGINE != 'truffleruby'
+      assert_raise(ArgumentError) { JSON.dump(eval(too_deep), 100) }
+      assert_raise(ArgumentError) { Marshal.dump(eval(too_deep), 100) }
+    end
     assert_equal too_deep, JSON.dump(eval(too_deep), 101)
     assert_kind_of String, Marshal.dump(eval(too_deep), 101)
     output = StringIO.new

diff --git a/test/json_gem/json_generator_test.rb b/test/json_gem/json_generator_test.rb
@@ -352,7 +352,7 @@ def test_nesting
     too_deep_ary = eval too_deep
     assert_raise(JSON::NestingError) { JSON.generate too_deep_ary }
     assert_raise(JSON::NestingError) { JSON.generate too_deep_ary, :max_nesting => 100 }
-    ok = JSON.generate too_deep_ary, :max_nesting => 101
+    ok = JSON.generate too_deep_ary, :max_nesting => 102
     assert_equal too_deep, ok
     ok = JSON.generate too_deep_ary, :max_nesting => nil
     assert_equal too_deep, ok

diff --git a/test/test_compat.rb b/test/test_compat.rb
@@ -467,10 +467,28 @@ def test_range
   end
 
   def test_arg_passing
-    json = Oj.to_json(Argy.new(), :max_nesting=> 40)
+    json = Oj.to_json(Argy.new(), :max_nesting => 40)
     assert_equal(%|{"args":"[{:max_nesting=>40}]"}|, json)
   end
 
+  def test_max_nesting
+    assert_raises() { Oj.to_json([[[[[]]]]], :max_nesting => 3) }
+    assert_raises() { Oj.dump([[[[[]]]]], :max_nesting => 3, :mode=>:compat) }
+
+    assert_raises() { Oj.to_json([[]], :max_nesting => 1) }
+    assert_equal('[[]]', Oj.to_json([[]], :max_nesting => 2))
+
+    assert_raises() { Oj.dump([[]], :max_nesting => 1, :mode=>:compat) }
+    assert_equal('[[]]', Oj.dump([[]], :max_nesting => 2, :mode=>:compat))
+
+    assert_raises() { Oj.to_json([[3]], :max_nesting => 1) }
+    assert_equal('[[3]]', Oj.to_json([[3]], :max_nesting => 2))
+
+    assert_raises() { Oj.dump([[3]], :max_nesting => 1, :mode=>:compat) }
+    assert_equal('[[3]]', Oj.dump([[3]], :max_nesting => 2, :mode=>:compat))
+
+  end
+
   def test_bad_unicode
     assert_raises() { Oj.to_json("\xE4xy") }
   end

diff --git a/test/test_various.rb b/test/test_various.rb
@@ -105,6 +105,7 @@ def test_set_options
       allow_gc: false,
       quirks_mode: false,
       allow_invalid_unicode: true,
+      float_format: '%0.13g',
       float_precision: 13,
       mode: :strict,
       escape_mode: :ascii,
@@ -416,6 +417,11 @@ def test_time_years
     }
   end
 
+  def test_dump_float
+    json = Oj.dump(1.23e-2, :mode => :null, :float_format => '%0.4f')
+    assert_equal('0.0123', json)
+  end
+
   # Class
   def test_class_null
     json = Oj.dump(Juice, :mode => :null)