Skip to content

Commit

Permalink
Time fix (#810)
Browse files Browse the repository at this point in the history 
* Avoid float precision issues when parsing Unix time with offset.

* Assert that loaded objects are equal to dumped objects.

* Reformat `custom.c` with `clang-format`.
  • Loading branch information
@aardvark179
aardvark179 committed Aug 23, 2022
1 parent a2a1328 commit 9e414b1
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 55 deletions.
61 changes: 21 additions & 40 deletions ext/oj/custom.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,14 @@ static void dump_obj_str(VALUE obj, int depth, Out out) {

static void dump_obj_as_str(VALUE obj, int depth, Out out) {
volatile VALUE rstr = rb_funcall(obj, oj_to_s_id, 0);
const char * str = RSTRING_PTR(rstr);
const char *str = RSTRING_PTR(rstr);

oj_dump_cstr(str, RSTRING_LEN(rstr), 0, 0, out);
}

static void bigdecimal_dump(VALUE obj, int depth, Out out) {
volatile VALUE rstr = rb_funcall(obj, oj_to_s_id, 0);
const char * str = RSTRING_PTR(rstr);
const char *str = RSTRING_PTR(rstr);
int len = (int)RSTRING_LEN(rstr);

if (0 == strcasecmp("Infinity", str)) {
Expand Down Expand Up @@ -82,8 +82,7 @@ static VALUE complex_load(VALUE clas, VALUE args) {
real_id = rb_intern("real");
imag_id = rb_intern("imag");
}
return rb_complex_new(rb_hash_aref(args, rb_id2str(real_id)),
rb_hash_aref(args, rb_id2str(imag_id)));
return rb_complex_new(rb_hash_aref(args, rb_id2str(real_id)), rb_hash_aref(args, rb_id2str(imag_id)));
}

static void time_dump(VALUE obj, int depth, Out out) {
Expand Down Expand Up @@ -246,8 +245,7 @@ static VALUE rational_load(VALUE clas, VALUE args) {
numerator_id = rb_intern("numerator");
denominator_id = rb_intern("denominator");
}
return rb_rational_new(rb_hash_aref(args, rb_id2str(numerator_id)),
rb_hash_aref(args, rb_id2str(denominator_id)));
return rb_rational_new(rb_hash_aref(args, rb_id2str(numerator_id)), rb_hash_aref(args, rb_id2str(denominator_id)));
}

static VALUE regexp_load(VALUE clas, VALUE args) {
Expand Down Expand Up @@ -292,8 +290,7 @@ static int hash_cb(VALUE key, VALUE value, VALUE ov) {
assure_size(out, depth * out->indent + 1);
fill_indent(out, depth);
} else {
assure_size(out,
depth * out->opts->dump_opts.indent_size + out->opts->dump_opts.hash_size + 1);
assure_size(out, depth * out->opts->dump_opts.indent_size + out->opts->dump_opts.hash_size + 1);
if (0 < out->opts->dump_opts.hash_size) {
APPEND_CHARS(out->cur, out->opts->dump_opts.hash_nl, out->opts->dump_opts.hash_size);
}
Expand Down Expand Up @@ -352,9 +349,7 @@ static void dump_hash(VALUE obj, int depth, Out out, bool as_ok) {
assure_size(out, depth * out->indent + 2);
fill_indent(out, depth);
} else {
assure_size(
out,
depth * out->opts->dump_opts.indent_size + out->opts->dump_opts.hash_size + 1);
assure_size(out, depth * out->opts->dump_opts.indent_size + out->opts->dump_opts.hash_size + 1);
if (0 < out->opts->dump_opts.hash_size) {
APPEND_CHARS(out->cur, out->opts->dump_opts.hash_nl, out->opts->dump_opts.hash_size);
}
Expand All @@ -372,10 +367,10 @@ static void dump_hash(VALUE obj, int depth, Out out, bool as_ok) {
}

static void dump_odd(VALUE obj, Odd odd, VALUE clas, int depth, Out out) {
ID * idp;
AttrGetFunc * fp;
ID *idp;
AttrGetFunc *fp;
volatile VALUE v;
const char * name;
const char *name;
size_t size;
int d2 = depth + 1;

Expand All @@ -384,7 +379,7 @@ static void dump_odd(VALUE obj, Odd odd, VALUE clas, int depth, Out out) {
if (NULL != out->opts->create_id && Yes == out->opts->create_ok) {
const char *classname = rb_class2name(clas);
int clen = (int)strlen(classname);
size_t sep_len = out->opts->dump_opts.before_size + out->opts->dump_opts.after_size + 2;
size_t sep_len = out->opts->dump_opts.before_size + out->opts->dump_opts.after_size + 2;

size = d2 * out->indent + 10 + clen + out->opts->create_id_len + sep_len;
assure_size(out, size);
Expand Down Expand Up @@ -481,7 +476,7 @@ static VALUE dump_common(VALUE obj, int depth, Out out) {
oj_dump_raw_json(obj, depth, out);
} else if (Yes == out->opts->to_json && rb_respond_to(obj, oj_to_json_id)) {
volatile VALUE rs;
const char * s;
const char *s;
int len;

if (RB_UNLIKELY(Yes == out->opts->trace)) {
Expand Down Expand Up @@ -521,11 +516,7 @@ static VALUE dump_common(VALUE obj, int depth, Out out) {
if (aj == obj) {
volatile VALUE rstr = rb_funcall(obj, oj_to_s_id, 0);

oj_dump_cstr(RSTRING_PTR(rstr),
(int)RSTRING_LEN(rstr),
false,
false,
out);
oj_dump_cstr(RSTRING_PTR(rstr), (int)RSTRING_LEN(rstr), false, false, out);
} else {
oj_dump_custom_val(aj, depth, out, true);
}
Expand Down Expand Up @@ -609,7 +600,7 @@ static void dump_obj_attrs(VALUE obj, VALUE clas, slot_t id, int depth, Out out)
assure_size(out, 2);
*out->cur++ = '{';
if (Qundef != clas && NULL != out->opts->create_id && Yes == out->opts->create_ok) {
size_t sep_len = out->opts->dump_opts.before_size + out->opts->dump_opts.after_size + 2;
size_t sep_len = out->opts->dump_opts.before_size + out->opts->dump_opts.after_size + 2;
const char *classname = rb_obj_classname(obj);
size_t len = strlen(classname);

Expand Down Expand Up @@ -911,7 +902,7 @@ void oj_dump_custom_val(VALUE obj, int depth, Out out, bool as_ok) {
///// load functions /////

static void hash_set_cstr(ParseInfo pi, Val kval, const char *str, size_t len, const char *orig) {
const char * key = kval->key;
const char *key = kval->key;
int klen = kval->klen;
Val parent = stack_peek(&pi->stack);
volatile VALUE rkey = kval->key_val;
Expand All @@ -928,14 +919,14 @@ static void hash_set_cstr(ParseInfo pi, Val kval, const char *str, size_t len, c
}
}
} else {
volatile VALUE rstr = oj_cstr_to_value(str, len, (size_t)pi->options.cache_str);
//volatile VALUE rstr = rb_utf8_str_new(str, len);
volatile VALUE rstr = oj_cstr_to_value(str, len, (size_t)pi->options.cache_str);
// volatile VALUE rstr = rb_utf8_str_new(str, len);

if (Qundef == rkey) {
if (Yes == pi->options.sym_key) {
rkey = ID2SYM(rb_intern3(key, klen, oj_utf8_encoding));
} else {
rkey = rb_utf8_str_new(key, klen);
rkey = rb_utf8_str_new(key, klen);
}
}
if (Yes == pi->options.create_ok && NULL != pi->options.str_rx.head) {
Expand Down Expand Up @@ -1008,20 +999,10 @@ static void hash_set_num(struct _parseInfo *pi, Val kval, NumInfo ni) {
// match the expected value.
parent->val = rb_funcall2(parent->val, oj_utc_id, 0, 0);
} else if (ni->has_exp) {
int64_t t = (int64_t)(ni->i + ni->exp);
struct _timeInfo ti;
VALUE args[8];

sec_as_time(t, &ti);

args[0] = LONG2NUM(ti.year);
args[1] = LONG2NUM(ti.mon);
args[2] = LONG2NUM(ti.day);
args[3] = LONG2NUM(ti.hour);
args[4] = LONG2NUM(ti.min);
args[5] = rb_float_new((double)ti.sec + ((double)nsec + 0.5) / 1000000000.0);
args[6] = LONG2NUM(ni->exp);
parent->val = rb_funcall2(rb_cTime, oj_new_id, 7, args);
struct timespec ts;
ts.tv_sec = ni->i;
ts.tv_nsec = nsec;
parent->val = rb_time_timespec_new(&ts, ni->exp);
} else {
parent->val = rb_time_nano_new(ni->i, (long)nsec);
}
Expand Down
17 changes: 4 additions & 13 deletions ext/oj/object.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,19 +269,10 @@ static int hat_num(ParseInfo pi, Val parent, Val kval, NumInfo ni) {
// match the expected value.
parent->val = rb_funcall2(parent->val, oj_utc_id, 0, 0);
} else if (ni->has_exp) {
int64_t t = (int64_t)(ni->i + ni->exp);
struct _timeInfo ti;
VALUE args[8];

sec_as_time(t, &ti);
args[0] = LONG2NUM((long)(ti.year));
args[1] = LONG2NUM(ti.mon);
args[2] = LONG2NUM(ti.day);
args[3] = LONG2NUM(ti.hour);
args[4] = LONG2NUM(ti.min);
args[5] = rb_float_new((double)ti.sec + ((double)nsec + 0.5) / 1000000000.0);
args[6] = LONG2NUM(ni->exp);
parent->val = rb_funcall2(rb_cTime, oj_new_id, 7, args);
struct timespec ts;
ts.tv_sec = ni->i;
ts.tv_nsec = nsec;
parent->val = rb_time_timespec_new(&ts, ni->exp);
} else {
parent->val = rb_time_nano_new(ni->i, (long)nsec);
}
Expand Down
8 changes: 8 additions & 0 deletions ext/oj/oj.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ ID oj_array_append_id;
ID oj_array_end_id;
ID oj_array_start_id;
ID oj_as_json_id;
ID oj_at_id;
ID oj_begin_id;
ID oj_bigdecimal_id;
ID oj_end_id;
Expand Down Expand Up @@ -89,7 +90,9 @@ VALUE oj_array_class_sym;
VALUE oj_create_additions_sym;
VALUE oj_decimal_class_sym;
VALUE oj_hash_class_sym;
VALUE oj_in_sym;
VALUE oj_indent_sym;
VALUE oj_nanosecond_sym;
VALUE oj_object_class_sym;
VALUE oj_quirks_mode_sym;
VALUE oj_safe_sym;
Expand Down Expand Up @@ -1814,6 +1817,7 @@ void Init_oj(void) {
oj_array_end_id = rb_intern("array_end");
oj_array_start_id = rb_intern("array_start");
oj_as_json_id = rb_intern("as_json");
oj_at_id = rb_intern("at");
oj_begin_id = rb_intern("begin");
oj_bigdecimal_id = rb_intern("BigDecimal");
oj_end_id = rb_intern("end");
Expand Down Expand Up @@ -1961,10 +1965,14 @@ void Init_oj(void) {
rb_gc_register_address(&oj_decimal_class_sym);
oj_hash_class_sym = ID2SYM(rb_intern("hash_class"));
rb_gc_register_address(&oj_hash_class_sym);
oj_in_sym = ID2SYM(rb_intern("in"));
rb_gc_register_address(&oj_in_sym);
oj_indent_sym = ID2SYM(rb_intern("indent"));
rb_gc_register_address(&oj_indent_sym);
oj_max_nesting_sym = ID2SYM(rb_intern("max_nesting"));
rb_gc_register_address(&oj_max_nesting_sym);
oj_nanosecond_sym = ID2SYM(rb_intern("nanosecond"));
rb_gc_register_address(&oj_nanosecond_sym);
oj_object_class_sym = ID2SYM(rb_intern("object_class"));
rb_gc_register_address(&oj_object_class_sym);
oj_object_nl_sym = ID2SYM(rb_intern("object_nl"));
Expand Down
3 changes: 3 additions & 0 deletions ext/oj/oj.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,9 @@ extern VALUE oj_ascii_only_sym;
extern VALUE oj_create_additions_sym;
extern VALUE oj_decimal_class_sym;
extern VALUE oj_hash_class_sym;
extern VALUE oj_in_sym;
extern VALUE oj_indent_sym;
extern VALUE oj_nanosecond_sym;
extern VALUE oj_max_nesting_sym;
extern VALUE oj_object_class_sym;
extern VALUE oj_object_nl_sym;
Expand All @@ -334,6 +336,7 @@ extern ID oj_array_append_id;
extern ID oj_array_end_id;
extern ID oj_array_start_id;
extern ID oj_as_json_id;
extern ID oj_at_id;
extern ID oj_begin_id;
extern ID oj_bigdecimal_id;
extern ID oj_end_id;
Expand Down
11 changes: 9 additions & 2 deletions test/test_custom.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -493,8 +493,15 @@ def test_time
skip 'TruffleRuby fails this spec' if RUBY_ENGINE == 'truffleruby'

obj = Time.now()
dump_load_dump(obj, false, :time_format => :unix, :create_id => "^o", :create_additions => true)
dump_load_dump(obj, false, :time_format => :unix_zone, :create_id => "^o", :create_additions => true)
# These two forms should be able to recreate the time precisely,
# so we check they can load a dumped version and recreate the
# original object correctly.
dump_and_load(obj, false, :time_format => :unix, :create_id => "^o", :create_additions => true)
dump_and_load(obj, false, :time_format => :unix_zone, :create_id => "^o", :create_additions => true)
# These two forms will lose precision while dumping as they don't
# preserve full precision. We check that a dumped version is equal
# to that version loaded and dumped a second time, but don't check
# that the loaded Ruby objects is still the same as the original.
dump_load_dump(obj, false, :time_format => :xmlschema, :create_id => "^o", :create_additions => true)
dump_load_dump(obj, false, :time_format => :ruby, :create_id => "^o", :create_additions => true)
end
Expand Down

0 comments on commit 9e414b1

Please sign in to comment.