GET params in ruby-on-rails project - best practices?

Question

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' & 'controller' 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...

1. Is it safe? Does ActiveRecord protect me from SQL injection attacks (notice I'm trusting the params that are coming in)?
2. Is this the best way to do it, or is there some automagical rails feature I'm not familiar with?

Answer 1

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.

Answer 2

1. Yes, the code you listed should be safe from SQL Injection.
2. 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.