This project is archived and is in readonly mode.

#2019 ✓wontfix
Josh Symonds

Format two-digit years correctly

Reported by Josh Symonds | February 19th, 2009 @ 07:15 PM | in 3.0.2

Again and again in projects I've run into the problem of users typing the following in date or datetime fields:

01/01/09

And having Rails save 0009-01-01 to the database. I can't imagine this is the correct functionality so I think it qualifies as a bug, and since so many users enter time like this I thought it might do to fix it. This patch allows users to enter time in as 01/01/09 and Rails will save 2009-01-01 to the database.

Comments and changes to this ticket

  • Robby Russell

    Robby Russell February 24th, 2009 @ 04:59 AM

    -1

    In my opinion this should be handled in your application as defaulting to 2050 vs 1950 when you input '01/01/50' could be problematic.

  • Josh Symonds

    Josh Symonds February 24th, 2009 @ 03:19 PM

    This patch isn't intended to decide on 2050 vs 1950 in applications. Its intention is to prevent Rails from deciding on 0050 and entering it into the database, which is the present behavior.

    Even if you think that there should be some debate around which century to intelligently choose, almost no user actually intends 0050 when they write a date as 01/01/50. In this situation I think it's clearly better to let Rails try to interpret the 50 (which, in the current Date.parse, becomes 1950).

    This is obviously a Rails bug, not some kind of new application default.

  • Michael Koziarski

    Michael Koziarski March 2nd, 2009 @ 05:19 AM

    Could you rebase the patch down into a single changeset, multiple changesets for a single change are hard to follow.

  • Joel Hoffman

    Joel Hoffman March 4th, 2009 @ 06:26 PM

    This is the default behavior of Ruby's Date class. If the second option to Date._parse is true, 50-1-1 is parsed as Jan 1 2050, but it defaults to false, and active_support/core_ext/string/conversions.rb unhelpfully specifies false. I've used the following monkeypatch to solve the problem:

    class Date class << self

    def _parse_with_default_comp(str='-4712-01-01', comp=nil)
      _parse_without_default_comp(str,true)                  
    end                                                      
    
    alias_method_chain :_parse, :default_comp
    
    

    end
    end

  • Geoff Buesing

    Geoff Buesing March 14th, 2009 @ 05:10 PM

    This isn't a Rails bug: you're assigning a String object to an attribute, and Rails is passing that value to your database.

    Your particular database is interpreting the string "01/01/09" to mean "0009-01-01".

    I don't think the logic of parsing/interpreting "01/01/09" as 2009, 1909 or 0009 AD should be built into write_attribute or connection quoting. The proper place for this would be a custom attribute writer, something like:

    
    def start_date=(val)
      # could use Chronic.parse here instead
      val = Date.parse(val, true) if val.is_a?(String)
      write_attribute(:start_date, val)
    end
    
  • Pratik

    Pratik May 16th, 2009 @ 05:27 PM

    • Assigned user set to “Geoff Buesing”
    • State changed from “new” to “invalid”
  • James

    James May 25th, 2009 @ 07:18 AM

    I am a little confused by the comment that Rails is doing nothing at all except passing the value through to the database - most probably due to my failing to understand the inner workings of ActiveRecord.

    For reference I'm running rails 2.2.2

    Lets say I've a model object with
    - a Date attribute called 'd', and - an Integer attribute called 'x'.

    I set that attribute to a string (it's returned from some form) in the normal fashion:
    m = Model.new( {:d => '1/1/2000', :x => '1'} )

    If I now do:
    m.attributes_before_type_cast then I can see that the values for both 'd' and 'x' are set as a strings.

    If I do:
    m.attributes then I see that 'x' is set to the integer 1, whilst 'd' is nil.

    This record has not made it through to the db, and yet something has converted the string to an int for 'x' and not the string to a date for 'd'.

    Interestingly? I've a validation in Model also:
    validates_presence_of :d This validation passes in spite of the nil for 'd' - presumably it is simply checking for existence in attributes_before_type_cast.

    Saving the object crashes due to the db not being able to convert the string to a date - which perhaps supports the comments by Geoff?

    I'm not suggesting Geoff's approach is not the way to go. It is certain that the underlying behaviour has certainly changed at some point along the way as this same code worked in an older version of rails which we upgraded from, without overrides of d=(), and with the date in the same format as it is now.

  • James

    James May 25th, 2009 @ 07:32 AM

    Further to my above comments, if the model object is constructed using:

    1: Presuming m is created with :d => '1/1/2000'

    It is possible to do m.d and get an appropriately created Time object back, in spite of it not appearing in m.attributes. The object still fails to save due to the db rejecting the string.

    2: Presuming m is created with :d => 'Sat, 01 Jan 2000 00:00:00 +0000'

    Then m.attributes shows that d has been set to a Time object, and now the object will save properly.

  • Joel Hoffman

    Joel Hoffman May 25th, 2009 @ 06:45 PM

    The above comment that the string is simply passed unmodified to the
    database is incorrect. Dates assigned to ActiveRecord objects from
    params are ultimately converted to strings by the code in the file
    active_support/core_ext/string/conversions.rb -- you can see that the
    second option to Date#parse (provided by the Ruby core Date library)
    is false. This instructs Date#
    parse to treat the two digit date 09
    as though it meant the actual year 9 instead of 2009, 1909, 1609, or
    2509, etc. The value true would cause it to prefer the year closest
    to current. See the attached patch.

    However, false is the default value. Rails should at least stop
    passing the default, broken value -- then when Ruby is fixed, Rails
    will be -- but should really coerce 2-digit dates to the current year. If necessary, historians can still enter 0009 for Roman records or whatever.

    Is there an i18n issue I'm overlooking here, or is there just a
    reluctance to break a small amount of expected behavior in favor of
    fixing a large amount of unexpected behavior?

  • Joel Hoffman

    Joel Hoffman May 25th, 2009 @ 06:59 PM

    I mean, this isn't hard to demonstrate. If i is an activerecord object with a date column named test_date,

    >> i.test_date = '09-05-25' => "09-05-25" >> i.test_date => Sat, 25 May 0009

    No database interaction.

  • Geoff Buesing

    Geoff Buesing May 26th, 2009 @ 05:30 AM

    • Milestone cleared.
    • State changed from “invalid” to “open”

    @Joel Hoffman -- you're right, my previous diagnosis of what was causing the problem was incorrect: a string assigned to a Date column is coerced to a Date object inside of read_attribute (which is called when sending values to the db.)

    Note that the parsing code is located in ActiveRecord::ConnectionAdapters::Column.fallback_string_to_date, not String#to_date, so your patch wouldn't address any issues with sending values to the db.

    I was initially against this change because the first patch added this logic to write_attribute, which seemed like the wrong place to do this work. But it looks to me that we can just change the boolean parameter for Date._parse in ActiveRecord::ConnectionAdapters::Column.fallback_string_to_date, so that two-digit years would then be interpreted as 19xx or 20xx (the threshold for 19xx is <= 69).

    Which seems reasonable enough. Apps that want different behavior can always do their own date parsing with a custom attribute writer; this wouldn't break that.

    However, I'm not sure if any i18n issues would arise from this change. I'd guess no, but we should probably be sure. If someone is willing to pull together a patch for this, we can solicit feedback on the core list just to be sure it wouldn't break anything.

  • Matt Ganderup

    Matt Ganderup June 1st, 2009 @ 07:18 PM

    @Geoff

    Here's a patch following your suggestion. I've played around with it some and it seems to work great.

  • Stephan Wehner

    Stephan Wehner June 2nd, 2009 @ 09:38 AM

    Is this not a input validation problem, as opposed to a parsing problem? Or a problem with how the date is collected (separate fields for month/day/year)

    Stephan

  • Geoff Buesing

    Geoff Buesing June 8th, 2009 @ 03:00 AM

    @Matt Ganderlup: I get a failing test with your patch, because topic.bonus_time isn't a date column

  • Matt Ganderup

    Matt Ganderup June 8th, 2009 @ 07:19 PM

    @Geoff Sorry, this is my first rails patch.
    The updated test passes in mysql and sqlite3. I was unable to test in postgres, but because of the nature of the change (parsing the input before it gets to the model) it should run the same in postgres.

  • Yehuda Katz (wycats)

    Yehuda Katz (wycats) June 22nd, 2009 @ 04:29 PM

    Is there a reason not to pull this into master?

  • Repository

    Repository August 4th, 2009 @ 04:42 AM

    • State changed from “open” to “resolved”

    (from [55d1d12c32a1b99f3f07d2346b49a63650ba2e9d]) fallback_string_to_date sets Date._parse comp arg to true, so that strings with two-digit years, e.g. '1/1/09', are interpreted as modern years [#2019 state:resolved] http://github.com/rails/rails/commit/55d1d12c32a1b99f3f07d2346b49a6...

  • Akira Matsuda

    Akira Matsuda August 4th, 2009 @ 10:44 PM

    On Ruby 1.9, that won't work as you Americans have expected.

    % ruby -rdate -e "p Date._parse('08/05/09', true)"
    {:mon=>8, :year=>2009, :mday=>5}
    
    % ruby19 -rdate -e "p Date._parse('08/05/09', true)"
    {:year=>2008, :mon=>5, :mday=>9}
    
  • Michael Koziarski

    Michael Koziarski August 5th, 2009 @ 02:43 AM

    • State changed from “resolved” to “open”

    Reopening as per Akira's comment.

    If it's going to fail in 1.9, we should just continue to do without this functionality.

  • Geoff Buesing

    Geoff Buesing August 5th, 2009 @ 02:57 PM

    • State changed from “open” to “wontfix”

    I agree, no point in making this change if it doesn't give us the desired behavior in Ruby 1.9.

    Reverted: http://github.com/rails/rails/commit/bfafe8c4055bcb8dcf7440015d95a3...

    @Akira -- thanks for the heads up.

    Note, however, that there are still differences in Ruby 1.8 and 1.9 Date._parse with comp=false:

    Ruby 1.8

    >> Date._parse('08/05/09', false)
    => {:mday=>5, :mon=>8, :year=>9}
    

    Ruby 1.9

    >> Date._parse('08/05/09', false)
    => {:year=>8, :mon=>5, :mday=>9}
    

    ... the upshot is, if you want your app to handle ambiguous date strings like '08/05/09', you'll need to add something custom to your app; Rails can't do this for you in a way that's consistent across 1.8 and 1.9.

  • Repository

    Repository August 5th, 2009 @ 03:05 PM

    (from [bfafe8c4055bcb8dcf7440015d95a32c9773f40b]) Revert "fallback_string_to_date sets Date._parse comp arg to true, so that strings with two-digit years, e.g. '1/1/09', are interpreted as modern years" [#2019 state:wontfix]

    This reverts commit 55d1d12c32a1b99f3f07d2346b49a63650ba2e9d.
    http://github.com/rails/rails/commit/bfafe8c4055bcb8dcf7440015d95a3...

  • Ryan Bigg

    Ryan Bigg October 9th, 2010 @ 09:56 PM

    • Tag cleared.

    Automatic cleanup of spam.

  • Jeremy Kemper

    Jeremy Kemper October 15th, 2010 @ 11:01 PM

    • Milestone set to 3.0.2
  • Jeff Kreeftmeijer

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Referenced by

Pages