Vincent.Z wrote:
if i dont assign the params hash to one variable,is it ok to protect
the injection?
such as:
1. Email.find(:all,
:conditions => [ "owner_id = 123 AND subject = ?" ,
params[:subject] ])
Perfectly fine at the code level. The ? cannot see where that value came from; it's just one more argument to find().
However, while certain Rails coders tend to revel in long unclear lines, you should not. You should add variables with names that reveal intent, and should write short lines wherever possible. So you could put params[:subject] into a variable, and maybe also use it somewhere else, or you could write this:
subjects_owned_by_mikey = [ "owners.name = 123 AND
subject = ?", params[:subject] ]
Email.find(:all, :conditions => subjects_owned_by_mikey, :include => :owners)
That way you don't have a hardcoded 123 (if that's Mikey;).
Or you could go the other way, and use one of ActiveRecord's automatic methods:
Email.find_all_by_subject(params[:subject], :conditions => ['owner_id = 123'])
You can also do the :include trick there.
and how about this one:
2. Email.find(:all,
:conditions =>{"owner_id = 123 AND subject = :subject",
[:subject=>params[:subject]]})
Of course. However, one reason for the :inserter facility is you might already have a hash available, for something else to use, and the :conditions can borrow it. Looking around, I see one such hash right there!:
Email.find_all_by_owner_id(123, :conditions => ["subject => :subject", params])
Any other elements in params, your :conditions will ignore them.
Ruby, and Rails, permit an incredible diversity of expression, so you should find the one that reads well for your problem, and that has the fewest moving parts. find_all_by_owner_id has less parts than the primitive alternative.
Also, you should put useful routines like these into your own methods on the Email model, and you should write unit tests for every one of their behaviors. That way you can change the lines around, and find the most expressive statements, after getting their behavior right. Pass all the tests after every few edits, and revert your code if the tests fail.
