We have been fuzzing our Rails applications to see what breaks, and finding some interesting things. This week, the continuous integration build started to break at random, but only on one application. Pull out the Yak clippers!
By looking at the test logs, we quickly isolated the problem. Rails date helpers let users input bad dates, e.g. February 30, 2008. (In almost all scenarios, we avoid the built-in date helpers anyway. But almost only works for horseshoes and hand grenades, and I don't want special cases that break.)
Obviously our fuzz generator will generate bad dates occasionally. Before you read further, test yourself: How does ActiveRecord handle such dates? How should it? The answer is in ActiveRecord::Base's execute_callstack_for_multiparameter_attributes, and it surprised me:
- If you are using the
Timeclass, the day overflows into the next month, so 2/29 magically becomes 3/1. Yuck. - If you are using
Date, Ruby raises an error. Rails then wraps this error, and by the time it gets back to you the offending values are not easily accessible.
Neither of these approaches appeal to me. A bad date should be validation error. Matthew pointed me to a plugin that lets you, on a per model basis, monkey patch ActiveRecord to report bad Dates as validation errors.
I decided that I wanted to have a patch that overrode Rails' behavior for all models, so I have written the first_class_dates plugin. This gets our tests passing again, and it keeps bad data from raising application exceptions, but it is still a hack. A better solution would:
- work consistently for
TimeandDate. - show the user the bad values they chose. (This is tricky since
Daterefuses even to enter a bad value state.)
The better solution would require a more substantial patch, and should be done in Rails itself. If others agree with the approach, we will submit the patch.
You could validate times with Time.days_in_month—something like (just winging it here):
raise SomeError if values1.to_i > Time.days_in_month(values1.to_i, values0.to_i)
Geoff: this won’t help. I don’t want invalid dates or times to throw exceptions. I want them to function as validations. Otherwise many controllers have to have two different sad paths: one for Rails validations, and another for bad dates and times.
Stuart: agreed, one path, not two. The code I pasted above shows an easy way to validate whether or not a year-month-day combo is valid or not—you’re going to need to do something like that, somewhere, or else Time will silently turn Feb 31 into Mar 2 and you won’t know.
If you want to make the offending values easily accessible, why not create an exception class that’s tailored to that need?
raise InvalidDateError.new(values) if invalid_date?(values)
You’d then rescue that error, and turn it into a meaningful validation error.
It’s great to see that someone is attacking this problem. I have been leaning towards replacing all date_selects with some fancy-schmancy javascript to pop up calendars and the works. Or possibly just accept a date string, preferably in iso format. But when you talk about patching, it strikes me that there may be more ways. How about (a) adding new singleton methods to the faulty dates so that they can return exactly what the user entered, not matter how wrong, or (b) monkey-patch Date so that it accepts invalid dates during creation, but doesn’t give a string representation that the db layer can use, or© create a ModelDate class that inherits from Date but does The Right Thing when it gets the user data.
Well, it might be stupid, but it struck me as possibilities right now, and I thought this was the best place to make a note of it. Tomorrow I will try to have a look at your plugin and possibly at the rails code. Hopefully that helps.