Fix xmlschema time parsing (#899)

* Don't assume xmlschema nanoseconds and don't fudge fractional seconds * Remove unused defs from #810 * Fixed clang-format violations
ohler55 · Dec 4, 2023 · e2825f7 · e2825f7
1 parent 6c8a708
commit e2825f7
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 17 deletions.
diff --git a/ext/oj/object.c b/ext/oj/object.c
@@ -83,8 +83,9 @@ static int parse_num(const char *str, const char *end, int cnt) {
 
 VALUE
 oj_parse_xml_time(const char *str, int len) {
-    VALUE       args[8];
-    const char *end = str + len;
+    VALUE       args[7];
+    const char *end  = str + len;
+    const char *orig = str;
     int         n;
 
     // year
@@ -144,17 +145,24 @@ oj_parse_xml_time(const char *str, int len) {
         char c = *str++;
 
         if ('.' == c) {
-            long long nsec = 0;
+            unsigned long long       num            = 0;
+            unsigned long long       den            = 1;
+            const unsigned long long last_den_limit = ULLONG_MAX / 10;
 
             for (; str < end; str++) {
                 c = *str;
                 if (c < '0' || '9' < c) {
                     str++;
                     break;
                 }
-                nsec = nsec * 10 + (c - '0');
+                if (den > last_den_limit) {
+                    // bail to Time.parse if there are more fractional digits than a ULLONG rational can hold
+                    return rb_funcall(rb_cTime, oj_parse_id, 1, rb_str_new(orig, len));
+                }
+                num = num * 10 + (c - '0');
+                den *= 10;
             }
-            args[5] = rb_float_new((double)n + ((double)nsec + 0.5) / 1000000000.0);
+            args[5] = rb_funcall(INT2NUM(n), oj_plus_id, 1, rb_rational_new(ULL2NUM(num), ULL2NUM(den)));
         } else {
             args[5] = rb_ll2inum(n);
         }

diff --git a/ext/oj/oj.c b/ext/oj/oj.c
@@ -33,7 +33,6 @@ 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;
@@ -51,6 +50,7 @@ ID oj_json_create_id;
 ID oj_length_id;
 ID oj_new_id;
 ID oj_parse_id;
+ID oj_plus_id;
 ID oj_pos_id;
 ID oj_raw_json_id;
 ID oj_read_id;
@@ -91,9 +91,7 @@ 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;
@@ -1848,7 +1846,6 @@ 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");
@@ -1866,6 +1863,7 @@ void Init_oj(void) {
     oj_length_id       = rb_intern("length");
     oj_new_id          = rb_intern("new");
     oj_parse_id        = rb_intern("parse");
+    oj_plus_id         = rb_intern("+");
     oj_pos_id          = rb_intern("pos");
     oj_raw_json_id     = rb_intern("raw_json");
     oj_read_id         = rb_intern("read");
@@ -1998,14 +1996,10 @@ 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"));

diff --git a/ext/oj/oj.h b/ext/oj/oj.h
@@ -311,9 +311,7 @@ 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;
@@ -331,7 +329,6 @@ 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;
@@ -349,6 +346,7 @@ extern ID oj_json_create_id;
 extern ID oj_length_id;
 extern ID oj_new_id;
 extern ID oj_parse_id;
+extern ID oj_plus_id;
 extern ID oj_pos_id;
 extern ID oj_read_id;
 extern ID oj_readpartial_id;

diff --git a/test/test_custom.rb b/test/test_custom.rb
@@ -503,8 +503,9 @@ def test_time
     # 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.
+    # that the loaded Ruby object 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 => :xmlschema, :create_id => '^o', :create_additions => true, second_precision: 3)
     dump_load_dump(obj, false, :time_format => :ruby, :create_id => '^o', :create_additions => true)
   end
 

diff --git a/test/test_object.rb b/test/test_object.rb
@@ -951,6 +951,20 @@ def test_odd_datetime
     dump_and_load(DateTime.new(2012, 6, 19, 13, 5, Rational(7_123_456_789, 1_000_000_000)), false)
   end
 
+  def test_odd_xml_time
+    str = "2023-01-01T00:00:00Z"
+    assert_equal(Time.parse(str), Oj.load('{"^t":"' + str + '"}', mode: :object))
+
+    str = "2023-01-01T00:00:00.3Z"
+    assert_equal(Time.parse(str), Oj.load('{"^t":"' + str + '"}', mode: :object))
+
+    str = "2023-01-01T00:00:00.123456789123456789Z"
+    assert_equal(Time.parse(str), Oj.load('{"^t":"' + str + '"}', mode: :object))
+
+    str = "2023-01-01T00:00:00.123456789123456789123456789Z"
+    assert_equal(Time.parse(str), Oj.load('{"^t":"' + str + '"}', mode: :object))
+  end
+
   def test_bag
     json = %{{
   "^o":"ObjectJuice::Jem",