This project is archived and is in readonly mode.

#638 ✓resolved
Clemens Kofler

DateHelper#date_or_time_select and DateHelper#default_time_from_options modify options hash

Reported by Clemens Kofler | July 17th, 2008 @ 12:30 AM

While working on a plugin that monkey patched some of Rails' DateHelper methods, I've stumbled across a small but potentially very confusing bug in date_or_time_select.

Here's the relevant code of the method:

order = (options[:order] ||= [:year, :month, :day])
# ...
[:day, :month, :year].each { |o| order.unshift(o) unless order.include?(o) }
# ...
[:hour, :minute, :second].each { |o| order.delete(o); order.push(o) }

The problem is that options[:order] is modified directly instead of using a cloned or duped version of it.

Let's assume the following snippet of code:

# in some configuration
date_helper_config = { :order => [ :year, :month, :day ] }
# in some view
<%= object.datetime_select :some_date, date_helper_config %>
<%= date_helper_config %>

In this case, date_helper_config all of a sudden has turned into [ :year, :month, :day, :hour, :minute, :second ].

I've tried duping or cloning the options before passing them in but since they only make shallow copies and keep the references in place, that doesn't change the problem. Only a workaround with deep cloning (as can be seen e.g. at http://zeljkofilipin.com/2006/02... would work in this case.

Since I think it's fairly usual to dup parameters when you intend to modify them I've created a patch that does exactly that - it just dups the options[:order] before using it. It also modifies the default_time_from_options method to prevent it from changing options[:default].

I've attached the failing test plus the patched date helper. All existing tests pass.

Comments and changes to this ticket

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>

People watching this ticket

Attachments

Pages