From 4ee26ac90981bbe5d7b5114ddd5036a5cea24251 Mon Sep 17 00:00:00 2001 From: Florian Frank Date: Mon, 29 Aug 2011 23:12:58 +0200 Subject: Add testcase for a possible crash with bignum --- tests/test_json_generate.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/test_json_generate.rb b/tests/test_json_generate.rb index da96603..2110eba 100755 --- a/tests/test_json_generate.rb +++ b/tests/test_json_generate.rb @@ -210,4 +210,24 @@ EOT ensure GC.stress = stress end if GC.respond_to?(:stress=) + + def test_broken_bignum # [ruby-core:38867] + pid = fork do + Bignum.class_eval do + def to_s + end + end + begin + JSON::Ext::Generator::State.new.generate(1<<64) + exit 1 + rescue TypeError + exit 0 + end + end + _, status = Process.waitpid2(pid) + assert status.success? + rescue NotImplementedError + # forking to avoid modifying core class of a parent process and + # introducing race conditions of tests are run in parallel + end if defined?(JSON::Ext) end -- cgit v1.2.1 From a4e599122fc0a6b849350c7cf4aa0721b0d45c3c Mon Sep 17 00:00:00 2001 From: Florian Frank Date: Wed, 9 Nov 2011 01:53:01 +0100 Subject: start to make buffer_initial_length configurable --- benchmarks/generator2_benchmark.rb | 2 +- benchmarks/generator_benchmark.rb | 2 +- benchmarks/parser2_benchmark.rb | 2 +- benchmarks/parser_benchmark.rb | 2 +- ext/json/ext/generator/generator.c | 76 ++++++++++++++++++++++++++++---------- ext/json/ext/generator/generator.h | 6 +-- lib/json/pure/generator.rb | 29 ++++++++++----- tests/test_json_generate.rb | 72 +++++++++++++++++++++--------------- 8 files changed, 126 insertions(+), 65 deletions(-) diff --git a/benchmarks/generator2_benchmark.rb b/benchmarks/generator2_benchmark.rb index 9885143..90fc711 100755 --- a/benchmarks/generator2_benchmark.rb +++ b/benchmarks/generator2_benchmark.rb @@ -199,7 +199,7 @@ if $0 == __FILE__ system "#{RAKE_PATH} clean" system "#{RUBY_PATH} #$0 rails" system "#{RUBY_PATH} #$0 pure" - system "#{RAKE_PATH} compile_ext" + system "#{RAKE_PATH} compile" system "#{RUBY_PATH} #$0 ext" system "#{RUBY_PATH} #$0 yajl" Bullshit.compare do diff --git a/benchmarks/generator_benchmark.rb b/benchmarks/generator_benchmark.rb index 83fa577..0554bca 100755 --- a/benchmarks/generator_benchmark.rb +++ b/benchmarks/generator_benchmark.rb @@ -201,7 +201,7 @@ if $0 == __FILE__ system "#{RAKE_PATH} clean" system "#{RUBY_PATH} #$0 rails" system "#{RUBY_PATH} #$0 pure" - system "#{RAKE_PATH} compile_ext" + system "#{RAKE_PATH} compile" system "#{RUBY_PATH} #$0 ext" system "#{RUBY_PATH} #$0 yajl" Bullshit.compare do diff --git a/benchmarks/parser2_benchmark.rb b/benchmarks/parser2_benchmark.rb index 95a510d..8798660 100755 --- a/benchmarks/parser2_benchmark.rb +++ b/benchmarks/parser2_benchmark.rb @@ -233,7 +233,7 @@ if $0 == __FILE__ system "#{RUBY_PATH} #$0 yaml" system "#{RUBY_PATH} #$0 rails" system "#{RUBY_PATH} #$0 pure" - system "#{RAKE_PATH} compile_ext" + system "#{RAKE_PATH} compile" system "#{RUBY_PATH} #$0 ext" system "#{RUBY_PATH} #$0 yajl" Bullshit.compare do diff --git a/benchmarks/parser_benchmark.rb b/benchmarks/parser_benchmark.rb index 9ce7e25..ed2331a 100755 --- a/benchmarks/parser_benchmark.rb +++ b/benchmarks/parser_benchmark.rb @@ -241,7 +241,7 @@ if $0 == __FILE__ system "#{RUBY_PATH} #$0 yaml" system "#{RUBY_PATH} #$0 rails" system "#{RUBY_PATH} #$0 pure" - system "#{RAKE_PATH} compile_ext" + system "#{RAKE_PATH} compile" system "#{RUBY_PATH} #$0 ext" system "#{RUBY_PATH} #$0 yajl" Bullshit.compare do diff --git a/ext/json/ext/generator/generator.c b/ext/json/ext/generator/generator.c index 781e9e6..d82973e 100644 --- a/ext/json/ext/generator/generator.c +++ b/ext/json/ext/generator/generator.c @@ -14,7 +14,8 @@ static VALUE mJSON, mExt, mGenerator, cState, mGeneratorMethods, mObject, static ID i_to_s, i_to_json, i_new, i_indent, i_space, i_space_before, i_object_nl, i_array_nl, i_max_nesting, i_allow_nan, i_ascii_only, i_quirks_mode, i_pack, i_unpack, i_create_id, i_extend, i_key_p, - i_aref, i_send, i_respond_to_p, i_match, i_keys, i_depth, i_dup; + i_aref, i_send, i_respond_to_p, i_match, i_keys, i_depth, + i_buffer_initial_length, i_dup; /* * Copyright 2001-2004 Unicode, Inc. @@ -294,18 +295,10 @@ static char *fstrndup(const char *ptr, unsigned long len) { /* fbuffer implementation */ -static FBuffer *fbuffer_alloc() -{ - FBuffer *fb = ALLOC(FBuffer); - memset((void *) fb, 0, sizeof(FBuffer)); - fb->initial_length = FBUFFER_INITIAL_LENGTH; - return fb; -} - -static FBuffer *fbuffer_alloc_with_length(unsigned long initial_length) +static FBuffer *fbuffer_alloc(unsigned long initial_length) { FBuffer *fb; - assert(initial_length > 0); + if (initial_length <= 0) initial_length = FBUFFER_INITIAL_LENGTH_DEFAULT; fb = ALLOC(FBuffer); memset((void *) fb, 0, sizeof(FBuffer)); fb->initial_length = initial_length; @@ -400,11 +393,10 @@ static FBuffer *fbuffer_dup(FBuffer *fb) unsigned long len = fb->len; FBuffer *result; + assert(len > 0); if (len > 0) { - result = fbuffer_alloc_with_length(len); + result = fbuffer_alloc(len); fbuffer_append(result, FBUFFER_PAIR(fb)); - } else { - result = fbuffer_alloc(); } return result; } @@ -694,6 +686,16 @@ static VALUE cState_configure(VALUE self, VALUE opts) state->depth = 0; } } + tmp = ID2SYM(i_buffer_initial_length); + if (option_given_p(opts, tmp)) { + VALUE buffer_initial_length = rb_hash_aref(opts, tmp); + if (RTEST(buffer_initial_length)) { + long initial_length; + Check_Type(buffer_initial_length, T_FIXNUM); + initial_length = FIX2LONG(buffer_initial_length); + if (initial_length > 0) state->buffer_initial_length = initial_length; + } + } tmp = rb_hash_aref(opts, ID2SYM(i_allow_nan)); state->allow_nan = RTEST(tmp); tmp = rb_hash_aref(opts, ID2SYM(i_ascii_only)); @@ -723,6 +725,7 @@ static VALUE cState_to_h(VALUE self) rb_hash_aset(result, ID2SYM(i_quirks_mode), state->quirks_mode ? Qtrue : Qfalse); rb_hash_aset(result, ID2SYM(i_max_nesting), LONG2FIX(state->max_nesting)); rb_hash_aset(result, ID2SYM(i_depth), LONG2FIX(state->depth)); + rb_hash_aset(result, ID2SYM(i_buffer_initial_length), LONG2FIX(state->buffer_initial_length)); return result; } @@ -920,19 +923,20 @@ static void generate_json(FBuffer *buffer, VALUE Vstate, JSON_Generator_State *s static FBuffer *cState_prepare_buffer(VALUE self) { - FBuffer *buffer = fbuffer_alloc(); + FBuffer *buffer; GET_STATE(self); + buffer = fbuffer_alloc(state->buffer_initial_length); if (state->object_delim) { fbuffer_clear(state->object_delim); } else { - state->object_delim = fbuffer_alloc_with_length(16); + state->object_delim = fbuffer_alloc(16); } fbuffer_append_char(state->object_delim, ','); if (state->object_delim2) { fbuffer_clear(state->object_delim2); } else { - state->object_delim2 = fbuffer_alloc_with_length(16); + state->object_delim2 = fbuffer_alloc(16); } fbuffer_append_char(state->object_delim2, ':'); if (state->space) fbuffer_append(state->object_delim2, state->space, state->space_len); @@ -940,7 +944,7 @@ static FBuffer *cState_prepare_buffer(VALUE self) if (state->array_delim) { fbuffer_clear(state->array_delim); } else { - state->array_delim = fbuffer_alloc_with_length(16); + state->array_delim = fbuffer_alloc(16); } fbuffer_append_char(state->array_delim, ','); if (state->array_nl) fbuffer_append(state->array_delim, state->array_nl, state->array_nl_len); @@ -1009,6 +1013,7 @@ static VALUE cState_initialize(int argc, VALUE *argv, VALUE self) VALUE opts; GET_STATE(self); state->max_nesting = 19; + state->buffer_initial_length = FBUFFER_INITIAL_LENGTH_DEFAULT; rb_scan_args(argc, argv, "01", &opts); if (!NIL_P(opts)) cState_configure(self, opts); return self; @@ -1349,7 +1354,37 @@ static VALUE cState_depth_set(VALUE self, VALUE depth) { GET_STATE(self); Check_Type(depth, T_FIXNUM); - return state->depth = FIX2LONG(depth); + state->depth = FIX2LONG(depth); + return Qnil; +} + +/* + * call-seq: buffer_initial_length + * + * This integer returns the current inital length of the buffer. + */ +static VALUE cState_buffer_initial_length(VALUE self) +{ + GET_STATE(self); + return LONG2FIX(state->buffer_initial_length); +} + +/* + * call-seq: buffer_initial_length=(length) + * + * This sets the initial length of the buffer to +length+, if +length+ > 0, + * otherwise it's set to the default value. + */ +static VALUE cState_buffer_initial_length_set(VALUE self, VALUE buffer_initial_length) +{ + long initial_length; + GET_STATE(self); + Check_Type(buffer_initial_length, T_FIXNUM); + initial_length = FIX2LONG(buffer_initial_length); + if (initial_length > 0) { + state->buffer_initial_length = initial_length; + } + return Qnil; } /* @@ -1391,6 +1426,8 @@ void Init_generator() rb_define_method(cState, "quirks_mode=", cState_quirks_mode_set, 1); rb_define_method(cState, "depth", cState_depth, 0); rb_define_method(cState, "depth=", cState_depth_set, 1); + rb_define_method(cState, "buffer_initial_length", cState_buffer_initial_length, 0); + rb_define_method(cState, "buffer_initial_length=", cState_buffer_initial_length_set, 1); rb_define_method(cState, "configure", cState_configure, 1); rb_define_alias(cState, "merge", "configure"); rb_define_method(cState, "to_h", cState_to_h, 0); @@ -1438,6 +1475,7 @@ void Init_generator() i_ascii_only = rb_intern("ascii_only"); i_quirks_mode = rb_intern("quirks_mode"); i_depth = rb_intern("depth"); + i_buffer_initial_length = rb_intern("buffer_initial_length"); i_pack = rb_intern("pack"); i_unpack = rb_intern("unpack"); i_create_id = rb_intern("create_id"); diff --git a/ext/json/ext/generator/generator.h b/ext/json/ext/generator/generator.h index 1fdd351..ec6cb2a 100644 --- a/ext/json/ext/generator/generator.h +++ b/ext/json/ext/generator/generator.h @@ -57,7 +57,7 @@ typedef struct FBufferStruct { unsigned long capa; } FBuffer; -#define FBUFFER_INITIAL_LENGTH 4096 +#define FBUFFER_INITIAL_LENGTH_DEFAULT 4096 #define FBUFFER_PTR(fb) (fb->ptr) #define FBUFFER_LEN(fb) (fb->len) @@ -65,8 +65,7 @@ typedef struct FBufferStruct { #define FBUFFER_PAIR(fb) FBUFFER_PTR(fb), FBUFFER_LEN(fb) static char *fstrndup(const char *ptr, unsigned long len); -static FBuffer *fbuffer_alloc(); -static FBuffer *fbuffer_alloc_with_length(unsigned long initial_length); +static FBuffer *fbuffer_alloc(unsigned long initial_length); static void fbuffer_free(FBuffer *fb); static void fbuffer_clear(FBuffer *fb); static void fbuffer_append(FBuffer *fb, const char *newstr, unsigned long len); @@ -126,6 +125,7 @@ typedef struct JSON_Generator_StateStruct { char ascii_only; char quirks_mode; long depth; + long buffer_initial_length; } JSON_Generator_State; #define GET_STATE(self) \ diff --git a/lib/json/pure/generator.rb b/lib/json/pure/generator.rb index 7c9b2ad..9c776a1 100644 --- a/lib/json/pure/generator.rb +++ b/lib/json/pure/generator.rb @@ -136,14 +136,15 @@ module JSON # * *quirks_mode*: Enables quirks_mode for parser, that is for example # generating single JSON values instead of documents is possible. def initialize(opts = {}) - @indent = '' - @space = '' - @space_before = '' - @object_nl = '' - @array_nl = '' - @allow_nan = false - @ascii_only = false - @quirks_mode = false + @indent = '' + @space = '' + @space_before = '' + @object_nl = '' + @array_nl = '' + @allow_nan = false + @ascii_only = false + @quirks_mode = false + @buffer_initial_length = 4096 configure opts end @@ -172,6 +173,16 @@ module JSON # it's disabled. attr_accessor :quirks_mode + # XXX + attr_reader :buffer_initial_length + + # XXX + def buffer_initial_length=(length) + if length > 0 + @buffer_initial_length = length + end + end + # This integer returns the current depth data structure nesting in the # generated JSON. attr_accessor :depth @@ -233,7 +244,7 @@ module JSON # passed to the configure method. def to_h result = {} - for iv in %w[indent space space_before object_nl array_nl allow_nan max_nesting ascii_only quirks_mode depth] + for iv in %w[indent space space_before object_nl array_nl allow_nan max_nesting ascii_only quirks_mode buffer_initial_length depth] result[iv.intern] = instance_variable_get("@#{iv}") end result diff --git a/tests/test_json_generate.rb b/tests/test_json_generate.rb index 2110eba..a528209 100755 --- a/tests/test_json_generate.rb +++ b/tests/test_json_generate.rb @@ -121,48 +121,51 @@ EOT def test_pretty_state state = PRETTY_STATE_PROTOTYPE.dup assert_equal({ - :allow_nan => false, - :array_nl => "\n", - :ascii_only => false, - :quirks_mode => false, - :depth => 0, - :indent => " ", - :max_nesting => 19, - :object_nl => "\n", - :space => " ", - :space_before => "", + :allow_nan => false, + :array_nl => "\n", + :ascii_only => false, + :buffer_initial_length => 4096, + :quirks_mode => false, + :depth => 0, + :indent => " ", + :max_nesting => 19, + :object_nl => "\n", + :space => " ", + :space_before => "", }.sort_by { |n,| n.to_s }, state.to_h.sort_by { |n,| n.to_s }) end def test_safe_state state = SAFE_STATE_PROTOTYPE.dup assert_equal({ - :allow_nan => false, - :array_nl => "", - :ascii_only => false, - :quirks_mode => false, - :depth => 0, - :indent => "", - :max_nesting => 19, - :object_nl => "", - :space => "", - :space_before => "", + :allow_nan => false, + :array_nl => "", + :ascii_only => false, + :buffer_initial_length => 4096, + :quirks_mode => false, + :depth => 0, + :indent => "", + :max_nesting => 19, + :object_nl => "", + :space => "", + :space_before => "", }.sort_by { |n,| n.to_s }, state.to_h.sort_by { |n,| n.to_s }) end def test_fast_state state = FAST_STATE_PROTOTYPE.dup assert_equal({ - :allow_nan => false, - :array_nl => "", - :ascii_only => false, - :quirks_mode => false, - :depth => 0, - :indent => "", - :max_nesting => 0, - :object_nl => "", - :space => "", - :space_before => "", + :allow_nan => false, + :array_nl => "", + :ascii_only => false, + :buffer_initial_length => 4096, + :quirks_mode => false, + :depth => 0, + :indent => "", + :max_nesting => 0, + :object_nl => "", + :space => "", + :space_before => "", }.sort_by { |n,| n.to_s }, state.to_h.sort_by { |n,| n.to_s }) end @@ -198,6 +201,15 @@ EOT assert_equal 19, s.depth end + def test_buffer_initial_length + s = JSON.state.new + assert_equal 4096, s.buffer_initial_length + s.buffer_initial_length = 0 + assert_equal 4096, s.buffer_initial_length + s.buffer_initial_length = -1 + assert_equal 4096, s.buffer_initial_length + end + def test_gc bignum_too_long_to_embed_as_string = 1234567890123456789012345 expect = bignum_too_long_to_embed_as_string.to_s -- cgit v1.2.1 From f1f0090a8b4bf3afd1f8e7d790236691386fd398 Mon Sep 17 00:00:00 2001 From: Florian Frank Date: Wed, 9 Nov 2011 01:53:01 +0100 Subject: start to make buffer_initial_length configurable --- tests/test_json_generate.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_json_generate.rb b/tests/test_json_generate.rb index a528209..29ea7ce 100755 --- a/tests/test_json_generate.rb +++ b/tests/test_json_generate.rb @@ -208,6 +208,8 @@ EOT assert_equal 4096, s.buffer_initial_length s.buffer_initial_length = -1 assert_equal 4096, s.buffer_initial_length + s.buffer_initial_length = 128 + assert_equal 128, s.buffer_initial_length end def test_gc -- cgit v1.2.1