Rails - Dec 2009 - Proper/Safe way to search across columns?

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

--

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-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@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.

On Dec 18, 2009, at 3:58 PM, Rilindo Foster wrote:
> 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

--

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-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en.