Rails Ruby on Rails → 3.0.2

Comments and changes to this ticket

• You flagged this item as spam.
  

  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.

• You flagged this item as spam.
  

  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 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.

• You flagged this item as spam.
  

  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 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 May 16th, 2009 @ 05:27 PM

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

• You flagged this item as spam.
  

  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.

• You flagged this item as spam.
  

  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.

• You flagged this item as spam.
  

  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?

  • 2019.patch 2.5 KB

• You flagged this item as spam.
  

  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 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.

• You flagged this item as spam.
  

  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.

  • two-digit-years.diff 1.6 KB

• You flagged this item as spam.
  

  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 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

• You flagged this item as spam.
  

  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.

  • two-digit-years.diff 1.6 KB
• 

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

  Is there a reason not to pull this into master?

• 

  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...

• You flagged this item as spam.
  

  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 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 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 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 October 9th, 2010 @ 09:56 PM

  • Tag cleared.

  Automatic cleanup of spam.

• 

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

  • Milestone set to “3.0.2”

  [bulk edit]

• 

  Jeff Kreeftmeijer November 8th, 2010 @ 08:51 AM

  Automatic cleanup of spam.