On Thursday 14 April 2011, Corin Langosch wrote: > account = Account.find_by_confirmation_token(params[:secret_token]) > > Confirmed accounts have their confirmation_token set to nil. > Unconfirmed accounts have some digest there. Now when the > controller action is called without secret_token set in the params > hash a random confirmed account is returned. This is because there's > no check if the underlying query returns more than one row. Of > course this is not desired.
The problem here is not so much with #find_by_whatever, but with how you're using it. It is completely legimitate that a finder returns only the first of possibly many matching objects. If you want a specific object, you have to impose a narrower scope or an ordering. The moral: Don't pass unchecked params to methods. > To help finding those subtile bugs/ design flaws, I'd suggest to > remove the "LIMIT 1" from the generated sql by the named finders and > instead raise an exception if the query returns more than one row. > Suggestion: "named finders expect the sql to return at most one > result, got xx". This would break code like Klass.order_by_foo.find_by_bar(...) I'd suggest that dynamic finders raise an exception when they are passed nil, but I'm pretty sure that this would break existing code, too. Michael -- Michael Schuerig mailto:[email protected] http://www.schuerig.de/michael/ -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
