Project

General

Profile

Actions

Bug #18978

closed

Unexpected behaviour in Time.utc and Time.local when 8 arguments are passed in

Added by peterzhu2118 (Peter Zhu) 3 months ago. Updated 2 months ago.

Status:
Feedback
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:109692]

Description

GitHub PR: https://github.com/ruby/ruby/pull/6281

Time.utc and Time.local produce inconsistent and unexpected behaviour when 8 arguments is passed in.

For example, consider the following code:

Time.utc(2000, 1, 1, 2, 3, 4, 100)

Here's the output on various Ruby implementations:

MRI: 2000-01-01 02:03:04.0001 UTC
TruffleRuby: 2000-01-01 02:03:04.0001 UTC
Opal: 2000-01-01 02:03:04 UTC

If we add an additional argument:

Time.utc(2000, 1, 1, 2, 3, 4, 100, 1)

The behaviour changes unexpectedly on MRI:

MRI: 2000-01-01 02:03:04 UTC
TruffleRuby: 2000-01-01 02:03:04.0001 UTC
Opal: 2000-01-01 02:03:04 UTC

Notice that the subseconds are lost.

The PR changes it so that 8 arguments is not accepted into the methods (i.e. an ArgumentError is raised when 8 arguments is passed in). Alternatively, we could have similar behaviour as TruffleRuby, where the 8th argument is ignored. However, since the 8th argument is unused, I think it would be less confusing to the user and prevent mistakes if we raised an ArgumentError forbidding it.

Updated by mame (Yusuke Endoh) 2 months ago

This behavior is to work with a ex-standard library called parsedate which was shipped with Ruby 1.8 series.

require 'parsedate'

ParseDate.parsedate "Tuesday, July 5th, 2007, 18:35:20 UTC"
# => [2007, 7, 5, 18, 35, 20, "UTC", 2]

Time.utc accepted this array as is by Time.utc(*ParseDate.parsedate(str)).
Note that the seventh argument is a timezone and the eighth is wday.
Therefore, Time.utc ignored these two arguments when the eighth argument is passed.

Now parsedate is an external gem https://rubygems.org/gems/rubysl-parsedate
I don't know how many people use parsedate now, but we can still see usages in some gems.
Rejecting the eighth argument may break these gems.

/srv/gems/awis4ruby-0.9.0/lib/awis4ruby.rb:      url_info.online_since = Time.local(*(ParseDate.parsedate(ol_since.text)))
/srv/gems/barx-0.2.0/lib/httpclient/cookie.rb:      @expires = Time.gm(*parsedate(value)[0,6])
/srv/gems/caring-r2flickr-0.1.1.7/lib/flickr/base.rb:    p.photos_firstdatetaken = Time.gm(*ParseDate.parsedate(tstr)) if
/srv/gems/caring-r2flickr-0.1.1.7/lib/flickr/base.rb:      dates[:taken] = Time.gm(*ParseDate.parsedate(att['taken']))
/srv/gems/ceritium-relaxdb-0.2.8/lib/relaxdb/view_object.rb:          v = Time.local(*ParseDate.parsedate(v)) rescue v
/srv/gems/cohitre-relaxdb-0.2.2/lib/relaxdb/document.rb:            val = Time.local(*ParseDate.parsedate(val)) rescue val
/srv/gems/cohitre-relaxdb-0.2.2/lib/relaxdb/view_object.rb:          v = Time.local(*ParseDate.parsedate(v)) rescue v
/srv/gems/coreymartella-universal_ruby_whois-1.2.1/lib/universal_ruby_whois/domain.rb:        @creation_date = (Time.local(*ParseDate.parsedate($2)) rescue nil)
/srv/gems/coreymartella-universal_ruby_whois-1.2.1/lib/universal_ruby_whois/domain.rb:        @expiration_date = (Time.local(*ParseDate.parsedate($2)) rescue nil)
/srv/gems/coreymartella-universal_ruby_whois-1.2.1/test/whois.rb:    assert_equal Time.local(*ParseDate.parsedate("1997-09-15")), domain.creation_date
/srv/gems/coreymartella-universal_ruby_whois-1.2.1/test/whois.rb:    assert_equal Time.local(*ParseDate.parsedate("2011-09-13")), domain.expiration_date
/srv/gems/csexton-twitter_archive-0.0.7/lib/twitter_archive/backends/blogger_archive.rb:        time = Time.gm(*ParseDate.parsedate(date_str)[0..4])
/srv/gems/ddate-1.0.0/bin/ddate:    time = Time.gm(*ParseDate.parsedate(ARGV.join(" "),true))
/srv/gems/dm-keeper-adapter-0.0.4/lib/dm-keeper-adapter/read.rb:          record[key] = Time.utc(ParseDate.parsedate(value))
/srv/gems/dustin-r2flickr-0.1.1.7/lib/flickr/base.rb:    p.photos_firstdatetaken = Time.gm(*ParseDate.parsedate(tstr)) if
/srv/gems/dustin-r2flickr-0.1.1.7/lib/flickr/base.rb:      dates[:taken] = Time.gm(*ParseDate.parsedate(att['taken']))
/srv/gems/eeml-0.0.42/lib/eeml/json_environment_parser_v005.rb:      env.updated = Time.gm(*ParseDate.parsedate(env_hash['updated'])) unless env_hash['updated'].nil?
/srv/gems/eeml-0.0.42/lib/eeml/json_environment_parser_v006.rb:      env.updated = Time.gm(*ParseDate.parsedate(env_hash['updated'])) unless env_hash['updated'].nil?
/srv/gems/eeml-0.0.42/lib/eeml/json_environment_parser_v100.rb:      env.updated = Time.gm(*ParseDate.parsedate(env_hash['updated'])) unless env_hash['updated'].nil?
/srv/gems/eeml-0.0.42/lib/eeml/libxml_eeml_parser_v005.rb:      env.updated = Time.gm(*ParseDate.parsedate(env_node['updated'])).utc if !env_node['updated'].nil?
/srv/gems/instiki-0.10.2/app/controllers/wiki_controller.rb:    start_date = Time.local(*ParseDate::parsedate(@params['start'])) rescue nil
/srv/gems/instiki-0.10.2/app/controllers/wiki_controller.rb:    end_date = Time.local(*ParseDate::parsedate(@params['end'])) rescue nil
/srv/gems/jstorimer-deep-test-2.0.0/sample_rails_project/vendor/rails/activesupport/lib/active_support/core_ext/string/conversions.rb:          ::Time.send(form, *ParseDate.parsedate(self))
/srv/gems/markoa-r2flickr-0.1.1.6/lib/flickr/base.rb:           p.photos_firstdatetaken = Time.gm(*ParseDate.parsedate(tstr)) if
/srv/gems/markoa-r2flickr-0.1.1.6/lib/flickr/base.rb:                   dates[:taken] = Time.gm(*ParseDate.parsedate(att['taken']))
/srv/gems/mlightner-universal_ruby_whois-1.2.7/lib/universal_ruby_whois/domain.rb:        @creation_date = (Time.local(*ParseDate.parsedate($2)) rescue nil)
/srv/gems/mlightner-universal_ruby_whois-1.2.7/lib/universal_ruby_whois/domain.rb:        @expiration_date = (Time.local(*ParseDate.parsedate($2)) rescue nil)
/srv/gems/mlightner-universal_ruby_whois-1.2.7/test/whois.rb:    assert_equal Time.local(*ParseDate.parsedate("1997-09-15")), domain.creation_date
/srv/gems/mlightner-universal_ruby_whois-1.2.7/test/whois.rb:    assert_equal Time.local(*ParseDate.parsedate("2011-09-14")), domain.expiration_date
/srv/gems/monetra-ruby-0.0.6/lib/monetra/active_support/core_ext/string/conversions.rb:          ::Time.send(form, *ParseDate.parsedate(self))
/srv/gems/octocat_herder-0.1.3/lib/octocat_herder/base.rb:        Time.utc(*ParseDate.parsedate(date_time))
/srv/gems/paulcarey-relaxdb-0.3.5/lib/relaxdb/view_object.rb:          v = Time.local(*ParseDate.parsedate(v)) rescue v
/srv/gems/php4r-0.0.4/lib/php4r.rb:        return Time.local(*ParseDate.parsedate(date_string)).to_i
/srv/gems/r2flickr-0.2/lib/flickr/base.rb:    p.photos_firstdatetaken = Time.gm(*ParseDate.parsedate(tstr)) if
/srv/gems/r2flickr-0.2/lib/flickr/base.rb:      dates[:taken] = Time.gm(*ParseDate.parsedate(att['taken']))
/srv/gems/rails-units-1.7.1/lib/rails_units/date.rb:      Time.local(*ParseDate.parsedate(self.to_s))
/srv/gems/rails-units-1.7.1/lib/rails_units/string.rb:        Time.local(*ParseDate.parsedate(self))
/srv/gems/rails-units-1.7.1/lib/ruby_units/string.rb:        Time.local(*ParseDate.parsedate(self))
/srv/gems/rflickr-2006.02.01/lib/flickr/base.rb:                p.photos_firstdatetaken = Time.gm(*ParseDate.parsedate(tstr)) if
/srv/gems/rflickr-2006.02.01/lib/flickr/base.rb:                        dates[:taken] = Time.gm(*ParseDate.parsedate(att['taken']))
/srv/gems/rss-client-2.0.10/lib/rss-client/http-access2/cookie.rb:          @expires = Time.gm(*parsedate(value)[0,6])
/srv/gems/rubot-base-0.0.1/lib/rubot/base.rb:                         :time    => Time.local(*(ParseDate.parsedate element.inner_html[/20\d\d-\d\d-\d\dT\d\d:\d\d:\d\d/])) }
/srv/gems/ruby-units-2.4.1/lib/ruby_units/date.rb:      Time.local(*ParseDate.parsedate(to_s))
/srv/gems/ruby-units-brewpoo-1.3.0/lib/ruby_units/date.rb:      Time.local(*ParseDate.parsedate(self.to_s))
/srv/gems/ruby-units-brewpoo-1.3.0/lib/ruby_units/string.rb:        Time.local(*ParseDate.parsedate(self))
/srv/gems/shattered-0.7.0/lib/shattered_support/core_ext/string/conversions.rb:          ::Time.send(form, *ParseDate.parsedate(self))
/srv/gems/shattered_support-0.5.1/lib/shattered_support/core_ext/string/conversions.rb:          ::Time.send(form, *ParseDate.parsedate(self))
/srv/gems/swivel-0.0.175/vendor/activesupport-2.0.2-/lib/active_support/core_ext/string/conversions.rb:          ::Time.send("#{form}_time", *ParseDate.parsedate(self)[0..5].map {|arg| arg || 0})
/srv/gems/tk-0.4.0/sample/tkextlib/tcllib/datefield.rb:    t = Time.local(*(ParseDate.parsedate(my_date1.value)))
/srv/gems/tsm-0.9/lib/tsm.rb:                                   svr[:established] = Time.local(*(ParseDate.parsedate("#{m[1]} #{m[2]}")))
/srv/gems/tsm-0.9/lib/tsm.rb:                                   svr[:access] = Time.local(*(ParseDate.parsedate("#{m[4]} #{m[5]}")))
/srv/gems/tsm-command-1.2/lib/tsm/dsmadmc.rb:                                        svr[:established] = Time.local(*(ParseDate.parsedate("#{m[1]} #{m[2]}")))
/srv/gems/tsm-command-1.2/lib/tsm/dsmadmc.rb:                                        svr[:access] = Time.local(*(ParseDate.parsedate("#{m[4]} #{m[5]}")))
/srv/gems/ubsafe-0.5/lib/ubsafe/ubsafe_commands/ubsafe_command_backup.rb:          file_mtime = Time.utc(*ParseDate.parsedate(cmd_output[0]))
/srv/gems/universal_ruby_whois-1.2.10/lib/universal_ruby_whois/domain.rb:        @creation_date = (Time.local(*ParseDate.parsedate($2)) rescue nil)
/srv/gems/universal_ruby_whois-1.2.10/lib/universal_ruby_whois/domain.rb:        @expiration_date = (Time.local(*ParseDate.parsedate($2)) rescue nil)
/srv/gems/universal_ruby_whois-1.2.10/test/whois.rb:    assert_equal Time.local(*ParseDate.parsedate("1997-09-15")), domain.creation_date
/srv/gems/universal_ruby_whois-1.2.10/test/whois.rb:    assert_equal Time.local(*ParseDate.parsedate("2011-09-14")), domain.expiration_date
/srv/gems/universal_ruby_whois-1.2.10/test/whois.rb:    assert_equal Time.local(*ParseDate.parsedate("1999-11-15")), domain.creation_date
/srv/gems/yaanno-relaxdb-0.2.2/lib/relaxdb/document.rb:            val = Time.local(*ParseDate.parsedate(val)) rescue val
/srv/gems/yaanno-relaxdb-0.2.2/lib/relaxdb/view_object.rb:          v = Time.local(*ParseDate.parsedate(v)) rescue v

I am not against the removal of the eighth argument, but I don't see the need to remove them at the cost of incompatibility.

Updated by mame (Yusuke Endoh) 2 months ago

BTW, if you remove the eighth argument, you may also want to remove the following code to handle the argument.

https://github.com/ruby/ruby/blob/d89f8a046753e2f166ee252510aadf1579dbcd63/time.c#L2947-L2955

Updated by peterzhu2118 (Peter Zhu) 2 months ago

Thanks for checking usages in gems @mame (Yusuke Endoh).

It looks like the last release of rubysl-parsedate is almost 10 years ago and the repo no longer exists. I still think it's better to remove the 8th argument and break compatibility since (1) it's undocumented, and (2) it has unexpected behavior that also drops the 7th argument. Additionally, this also opens up the opportunity to use the 8th argument for something else if needed in the future.

Updated by matz (Yukihiro Matsumoto) 2 months ago

  • Status changed from Open to Feedback

Even if parsedate is a pretty old library, I am not sure if it's OK to stop supporting it.
Do we rally have enough benefit to break the compatibility?

Matz.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0