Vincent.Z wrote:
if i dont assign the params hash to one variable,is it ok to protect
the injection?
such as:
- 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:
- 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.
–
Phlip