the "agile web development with rails" book advocate the bind variable
facility,
that is :
subject = params[:subject]
Email.find(:all,
:conditions => [ "owner_id = 123 AND subject = ?" , subject ])
i have some questions about this,
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] ])
and how about this one:
2. Email.find(:all,
:conditions =>{"owner_id = 123 AND subject = :subject",
[:subject=>params[:subject]]})
thanks!
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---
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. -- Phlip --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
sorry,maybe im not very clear to my problem. i wanna know if these style query can prevent the sql inject attack? thanks again. On Mar 16, 10:15 pm, Phlip <phlip2...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> 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. > > -- > Phlip--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---