Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

AR:Base mangles dates when user submits only part of the fields

#1715

When using date_select, it's possible for the user to submit only month and day, leaving year empty. How to handle that issue is up to the developer of the site, but his job is made a lot harder by ActiveRecord::Base which throws empty values out of the hash before assigning the composite values to a field.

The offending code:

  def extract_callstack_for_multiparameter_attributes(pairs)
    attributes = { }

    for pair in pairs
      multiparameter_name, value = pair
      attribute_name = multiparameter_name.split("(").first
      attributes[attribute_name] = [] unless attributes.include?(attribute_name)

      unless value.empty?
        attributes[attribute_name] <<
          [ find_parameter_position(multiparameter_name), type_cast_attribute_value(multiparameter_name, value) ]
      end
    end

    attributes.each { |name, values| attributes[name] = values.sort_by{ |v| v.first }.collect { |v| v.last } }
  end

This means that if the user submits something like:

{"date(1i)"=>"", "date(2i)"=>"6", "date(3i)"=>"5"}

The date becomes "0005-06-01" instead of "0000-06-05".

Both are meaningless dates, but in the latter case, at least it's obvious what the problem is. The developer can easily validate this and return a meaningful error. In the former case, this is not so easy.

Reported by Martijn Vos · January 9th, 2009 @ 07:05 AM

State: committed
Milestone: none
Assigned to: nobody
Importance: none

Activity

  1. Martijn Vos
    Martijn Vos

    This patch fixes the problem. For me at least -- it's likely there are use cases I'm not aware of.

    The patch doesn't add or update any tests because I couldn't find any relevant ones, and I'm not very familiar with these tests yet. This is my first Rails patch. Please check carefully.

    The patch is also in http://github.com/mcv/rails/comm...

    January 9th, 2009 @ 07:38 AM

  2. Pratik
    Pratik
    • State changed from new to incomplete
    • Assigned user set to Michael Koziarski

    Patch is missing tests.

    Thanks.

    March 12th, 2009 @ 03:47 PM

  3. CancelProfileIsBroken
    CancelProfileIsBroken
    • Tag changed from activecord, bug, date to activecord, bug, bugmash, date

    August 5th, 2009 @ 03:30 PM

  4. Hugo Peixoto
    Hugo Peixoto

    -1 for the patch, as it breaks two of the current tests.

    Throws an ActiveRecord::MultiparameterAssignmentErrors exception when the day is set "", because it tries to execute Date.new(2004, 12, 0), which is invalid. It also breaks when every parameter is set to "", because it executes Date.new(0, 0, 0), which is also invalid.

    August 9th, 2009 @ 07:35 AM

  5. Hugo Peixoto
    Hugo Peixoto

    Actually, according to date_select,

    NOTE: Discarded selects will default to 1. So if no month select is available, January will be assumed.

    So I've attached a patch that respects this behaviour, with additional tests, checking many combinations of empty values.

    August 9th, 2009 @ 08:22 AM

  6. Rizwan Reza
    Rizwan Reza

    verified

    This patch applies cleanly to 2-3-stable. All tests pass.

    August 9th, 2009 @ 04:06 PM

  7. Elad Meidar
    Elad Meidar

    +1 Verified, +1 patch. applies on 2-3-stable and master with all tests pass

    August 9th, 2009 @ 05:40 PM

  8. Repository
    Repository
    • State changed from incomplete to committed

    (from [2c4f4a8734b4137adc331186bb2255fb6a38c31e]) With multiparameter date attributes, the behaviour when empty fields are present is now coherent with the one described in the date_select documentation.

    [#1715 AR:Base mangles dates when user submits only part of the fields state:committed]

    Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
    http://github.com/rails/rails/commit/2c4f4a8734b4137adc331186bb2255...

    August 9th, 2009 @ 09:43 PM

  9. Repository
    Repository

    (from [870750ed4b1e0b0e574aaec86db3e2cdf94b1190]) With multiparameter date attributes, the behaviour when empty fields are present is now coherent with the one described in the date_select documentation.

    [#1715 AR:Base mangles dates when user submits only part of the fields state:committed]

    Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
    http://github.com/rails/rails/commit/870750ed4b1e0b0e574aaec86db3e2...

    August 9th, 2009 @ 09:43 PM

  10. CancelProfileIsBroken
    CancelProfileIsBroken
    • Assigned user cleared.
    • Tag changed from activecord, bug, bugmash, date to activecord, bug, date
    • Milestone cleared.

    August 9th, 2009 @ 10:05 PM

  11. Raimonds Simanovskis
    Raimonds Simanovskis

    These new tests fail on Oracle database (using oracle_enhanced adapter) because Oracle adapter by default returns date values as Time values and for Time values execute_callstack_for_multiparameter_attributes method does not replace missing values with 1.

    August 10th, 2009 @ 02:43 PM

  12. Pratik
    Pratik

    Hey Raimonds,

    If you have a patch ready, I'll be happy to push.

    Thanks.

    August 10th, 2009 @ 03:34 PM

  13. Raimonds Simanovskis
    Raimonds Simanovskis

    I modified the test so that using Oracle enhanced adapter attribute type is forced to Date and now tests are passing: http://github.com/rsim/rails/commit/416d6cc51a6c67a89a564c86de8fede...

    But the question remains is it necessary to substitute missing values with 1 when using datetime_select to assign values to Time attribute. Currently it seems that in such case result is unpredictable but in this case nothing is told in API documentation :)

    August 10th, 2009 @ 08:26 PM