GET params in ruby-on-rails project - best practices?
I've inherited a little rails app and I need to extend it slightly. It's actually quite simple, but I want to make sure I'm doing it the right way...
If I visit myapp:3000/api/persons it gives me a full list of people in XML format. I want to pass param in the URL so that I can return users that match the login or the email e.g. yapp:3000/api/persons?login=jsmith would give me the person with the corresponding login. Here's the code:
def index
if params.size > 2 # We have 'action' & 'control开发者_如何学Goler' by default
if params['login']
@person = [Person.find(:first, :conditions => { :login => params['login'] })]
elsif params['email']
@persons = [Person.find(:first, :conditions => { :email => params['email'] })]
end
else
@persons = Person.find(:all)
end
end
Two questions...
- Is it safe? Does ActiveRecord protect me from SQL injection attacks (notice I'm trusting the params that are coming in)?
- Is this the best way to do it, or is there some automagical rails feature I'm not familiar with?
- Yes, the code you listed should be safe from SQL Injection.
- Yes, this is generally acceptable rails code...but
There are some oddities.
Your index action talks about @person and @persons. By convention, @persons is expected, and @person is unusual. I suspect you can eliminate @person, and clear things up in one swoop. Something like this (untested):
def index
@persons = if params[:email]
Person.find_all_by_email(params[:email])
elsif params[:login]
Person.find_all_by_login(params[:login])
else
Person.all
end
end
Don't forget to update your view -- I suspect it's still looking for @person. If your view is doing anything "interesting" with @person, you probably want to move it to your show action.
Here's the ruby on rails security guide section on SQL Injection. It looks like what you have, using hash conditions, is pretty safe. Sounds like using Person.find_by_login
or Person.find_by_email
might be a little better.
精彩评论