Bug #9836: Bad Implementation of Time.strptime

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: ruby 2.1.2p95 (2014-05-08 revision 45877) [x86_64-linux]
• Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN

[silverhammermba (Max Anselm)]

According to the documentation, Time.strptime "parses +date+ using Date._strptime and converts it to a Time object." However this conversion is extremely naive, using only certain fields return by Date._strptime and ignoring the rest (for example :wnum0).

This creates confusing and inconsistent behavior when compared to Date and DateTime's strptime methods.

For example:

puts Date.strptime('201418', "%Y%U")
=> 2014-05-04
puts DateTime.strptime('201418', "%Y%U")
=> 2014-05-04T00:00:00+00:00
puts Time.strptime('201418', "%Y%U")
=> 2014-01-01 00:00:00 -0500

[matz (Yukihiro Matsumoto)]

• Status changed from Open to Feedback

I am not sure what behavior you are referring as "extremely naive".
Please be concrete. How do you think it should behave?

Matz.

[nobu (Nobuyoshi Nakada)]

I guess the OP expects %U not to be ignored.

diff --git a/lib/time.rb b/lib/time.rb
index 3728fef..ec45767 100644
--- a/lib/time.rb
+++ b/lib/time.rb
@@ -194,13 +194,18 @@ class Time
 
     LeapYearMonthDays = [31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31] # :nodoc:
     CommonYearMonthDays = [31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31] # :nodoc:
-    def month_days(y, m)
+    def month_days_of_year(y)
       if ((y % 4 == 0) && (y % 100 != 0)) || (y % 400 == 0)
-        LeapYearMonthDays[m-1]
+        LeapYearMonthDays
       else
-        CommonYearMonthDays[m-1]
+        CommonYearMonthDays
       end
     end
+    private :month_days_of_year
+
+    def month_days(y, m)
+      month_days_of_year(y)[m-1]
+    end
     private :month_days
 
     def apply_offset(year, mon, day, hour, min, sec, off)
@@ -432,7 +437,34 @@ class Time
       else
         year = d[:year]
         year = yield(year) if year && block_given?
-        t = make_time(date, year, d[:mon], d[:mday], d[:hour], d[:min], d[:sec], d[:sec_fraction], d[:zone], now)
+        mon = d[:mon]
+        mday = d[:mday]
+        unless mon and mday
+          case
+          when (w = d[:wnum0])  # %U
+            range = 0..53
+            mday = +1
+          when (w = d[:wnum1])  # %W
+            range = 0..53
+            mday = +2
+          when (w = d[:cweek])  # %V
+            range = 1..53
+            mday = -6
+          end
+          if mday
+            raise ArgumentError, "invalid date" unless range.cover?(w)
+            t = make_time(date, year, 1, 1, nil, nil, nil, nil, d[:zone], now)
+            mday += w * 7 - t.wday
+            if w = d[:wday] || d[:cwday]
+              mday += w
+            end
+            month_days_of_year(t.year).each_with_index do |n, m|
+              break mon = m+1 if mday <= n
+              mday -= n
+            end
+          end
+        end
+        t = make_time(date, year, mon, mday, d[:hour], d[:min], d[:sec], d[:sec_fraction], d[:zone], now)
       end
       t
     end

[silverhammermba (Max Anselm)]

Sorry for the unclear wording. I would expect that it behaves similar to DateTime's implementation and to not ignore any fields returned by Date._strptime. At the very least the documentation should reflect the fact that it will only use certain values when constructing the Time object.

The fact that

Date.strptime("201418", "%Y%U").to_time
Time.strptime("201418", "%Y%U")

return different times is very surprising.

[jeremyevans0 (Jeremy Evans)]

• Related to Bug #14241: Time.strptime() doesn't support the directive "%W". added

[jeremyevans0 (Jeremy Evans)]

• File time-strptime.patch time-strptime.patch added

I tried updating the patch to apply to the master branch (it is attached). It doesn't appear to handle %W correctly:

str = Time.local(2019, 1, 30).strftime('%w %W %Y')
# => "3 04 2019"

Date.strptime(str, '%w %W %Y')
# => #<Date: 2019-01-30 ((2458514j,0s,0n),+0s,2299161j)>

Time.strptime(str, '%w %W %Y')
# => 2019-01-31 00:00:00 -0800

I think the approach I'm proposing in #14241 is simpler, by leaving the calculation to Date.strptime.

• Status changed from Feedback to Closed

Applied in changeset git|a9d4f2d03c847ec1c89dc03a5076a9fa29ffa61f.

---

Support %U/%u/%W/%w/%V/%g/%G formats in Time.strptime

Most of these formats were documented as supported, but were not
actually supported. Document that %g and %G are supported.

If %U/%W is specified without yday and mon/mday are not specified,
then Date.strptime is used to get the appropriate yday.

If cwyear is specifier without the year, or cwday and cweek are
specified without mday and mon, then use Date.strptime and convert
the resulting value to Time, since Time.make_time cannot handle
those conversions

Fixes [Bug #9836]
Fixes [Bug #14241]