Proper/Safe way to search across columns?

rubyonrails-talk
1

Thanks to people on this list, I was able to implement a search across columns (thank you guys!) However, I am not sure if I did it the right way.

What I did is that on the view page, I have implemented the search as follows:

<% form_tag books_path, :method => 'get' do %>   <p>     <%= text_field_tag :search, params[:search] %>     <%= select_tag(:fieldname,options_for_select(Book.column_names.reject{|x| x == 'id' || x.match(/(_id\Z|_at\Z)/)})) %>     <%= submit_tag "Search" %>   </p> <% end %>

The controller I setup as follows:

class BooksController < ApplicationController

  def index

    @books =Book.search(params[:search], params[:page], params[:fieldname])

    respond_to do |format|       format.html # index.html.erb       format.xml { render :xml => @books }     end   end

end

And the model:

  def self.search(search, page, fieldname)     if fieldname.nil?       paginate :per_page => 20, :page => page,                :conditions => ['name like ?', "%#{search}%"],                :order => 'name'     else       paginate :per_page => 20, :page => page,                :conditions => ["#{fieldname} like ?", "%#{search}%"],                :order => 'name'     end   end

It seems to work for me, but I can't get rid of the nagging feeling that I am not doing something right and I think its has to do with the model. Is it safe to pass column names like that?

Also, there is some duplication across all the controllers for the index search - should I just try consolidate all of that into the application controller (I am not sure how I can redefine the model name if consolidation is necessary, but that I can search for and figure out for a while).

Finally, if this isn't right/or proper way, is there another way that is better? I know a bit about the searches based on model and/or named scopes (by way of railscasts) - are those the recommended way in general in the Rails community (aside from an index daemon like Sphinx* which I may end up using anyway in the future).

*If you are wondering why I don't want to use Sphinx, its mostly because I want to be prepared in case I am in the situation where I can't put in Sphinx or any other search daemon and I have to rely on searching by this method instead.

- Rilindo

2

Thanks to people on this list, I was able to implement a search across columns (thank you guys!) However, I am not sure if I did it the right way.

What I did is that on the view page, I have implemented the search as follows:

<% form_tag books_path, :method => 'get' do %> <p>    <%= text_field_tag :search, params[:search] %>    <%= select_tag(:fieldname,options_for_select(Book.column_names.reject{|x> x == 'id' || x.match(/(_id\Z|_at\Z)/)})) %>    <%= submit_tag "Search" %> </p> <% end %>

The controller I setup as follows:

class BooksController < ApplicationController

def index

   @books =Book.search(params[:search], params[:page], params[:fieldname])

   respond_to do |format|      format.html # index.html.erb      format.xml { render :xml => @books }    end end

end

And the model:

def self.search(search, page, fieldname)    if fieldname.nil?      paginate :per_page => 20, :page => page,               :conditions => ['name like ?', "%#{search}%"],               :order => 'name'    else      paginate :per_page => 20, :page => page,               :conditions => ["#{fieldname} like ?", "%#{search}%"],               :order => 'name'    end end

It seems to work for me, but I can't get rid of the nagging feeling that I am not doing something right and I think its has to do with the model. Is it safe to pass column names like that?

Only if you don't mind the fact that I can search any of your book columns by specifying my own url... what stops me from doing this:

http://your.site/books/?search=foo&fieldname=some_id

Your view code prevents that, but your controller code doesn't.

If it were me I'd explicitly list the columns that are *okay* to search as a constant array in the book model. Use that to both render the select options in the view and *check* it in the controller (or model, whichever).

You've got a bigger problem though.

               :conditions => ["#{fieldname} like ?", "%#{search}%"],

That's dangerous. I don't see where you sanity check params[:fieldname] which is what fieldname is. So, what stops me from doing this:

http://your.site/books/?search=foo&fieldname=true;delete from your_table; select foo

Or something similar to that to trick that query into executing multiple statements? (Google "sql injection")

If you explicitly list the columns that you want to be searchable and then match against that, then the above code is okay because you've sanity checked it.

Also, there is some duplication across all the controllers for the index search - should I just try consolidate all of that into the application controller (I am not sure how I can redefine the model name if consolidation is necessary, but that I can search for and figure out for a while).

Finally, if this isn't right/or proper way, is there another way that is better? I know a bit about the searches based on model and/or named scopes (by way of railscasts) - are those the recommended way in general in the Rails community (aside from an index daemon like Sphinx* which I may end up using anyway in the future).

You might look into SearchLogic. I've never used it, but people love it. But if you're gonna want to search across several columns quickly, just install Sphinx (thinking sphinx gem is great).

*If you are wondering why I don't want to use Sphinx, its mostly because I want to be prepared in case I am in the situation where I can't put in Sphinx or any other search daemon and I have to rely on searching by this method instead.

Then find a host that will you run sphinx. I know what you're saying, but that's akin to saying you'll only use Rails 1.1.6 because that's what your ISP supports...

Sphinx makes all of this easy, fast, scalable, etc. etc. etc...

Good luck!

-philip