新手问题 我觉得我是用 Java 的方式在写 ROR,求修改

milk · 2013年03月17日 · 最后由 cloud 回复于 2013年03月29日 · 4226 次阅读
#第几页
page = params[:p] || ''
page = /^\d+$/ =~ page ? page.to_i-1 : 0

#根据名称或序列号查询
name = (params[:name] || '')[0,20]  #限制长度
serial = (params[:serial] || '')[0,20]

p = []
condition = ""

unless name.blank? then
  condition  = "name LIKE ?"
  p << "%#{name}%"
end

unless serial.blank? then
  condition += unless name.blank? then " and serial LIKE ?" else "serial LIKE ?" end
    p << "%#{serial}%"
end

如上代码,功能是实现了,不过感觉和 JAVA 的差不多,换大家会怎么写?

用 ransack 和 will_paginate,然后 index 方法类似这样:

class Shift6SChecklistsController < ApplicationController
  def index
    @q = Shift6SChecklist.search(params[:q])
    @shift6_s_checklists = @q.result.order("created_at DESC").paginate(:page => params[:page], :per_page => 10)

view 类似这样:

<%= search_form_for @q, :class => "well form-search" do |f| %>
  <%= link_to 'Check 6S', check_shift6_s_checklists_path, :class => 'btn btn-primary' %>
  <% if user_signed_in? %>
  <%= link_to 'My Items', shift6_s_checklists_path(:q => {:resp_emp_badge_or_resp_supervisor_badge_eq => current_user.badge}), :class => 'btn' %>
  <% end %>
  <label class="checkbox">Badge or ID:<%= f.text_field :resp_emp_badge_or_resp_supervisor_badge_or_id_eq, :class => 'input-small search-query', :placeholder => 'Badge or ID', :title => 'Type here to search Employee ID or item ID.'  %></label>
  <label class="checkbox">Finding or Action contain:<%= f.text_field :finding_fact_or_resp_action_cont, :class => 'search-query',:placeholder => 'Finding or Action', :title => 'Type here to search finding or action' %></label>
  <label class="checkbox"><%= f.check_box :close_date_null -%>Open only</label>
  <br />
  <label class="checkbox">Type:<%= f.select :check_type_eq, :check_type => Shift6SChecklist.type_description.keys.unshift('')  %></label>
  <label class="checkbox">Area:<%= f.select :station_eq, :area => Shift6SChecklist.station_collection.keys.unshift('') %></label>
  <label class="checkbox">Shift:<%= f.select :create_shift_code_eq, :create_shift_code => User.shift_collection.unshift('') %></label>
  <%= f.submit :class => "btn" %>
  <%= link_to 'Export Excel CSV', shift6_s_checklists_path.concat('.csv') %>
<% end %>

<%= will_paginate @shift6_s_checklists %>
#第几页
page = params[:p] || ''
page = /^\d+$/ =~ page ? page.to_i-1 : 0
#根据名称或序列号查询
name = (params[:name] || '')[0,20]  #限制长度
serial = (params[:serial] || '')[0,20]
p = []
condition = ""
unless name.blank? then
  condition  = "name LIKE ?"
  p << "%#{name}%"
end
unless serial.blank? then
  condition += unless name.blank? then " and serial LIKE ?" else "serial LIKE ?" end
    p << "%#{serial}%"
end

可以修改成

page = params[:p] || 1
conditions = []
conditions << "name LIKE %#{params[:name][0,20]}%" if params[:name].present?
conditions << "serial LIKE %#{serial}%" if params[:serial].present?

# 最后在使用conditions
conditions.join(" AND ")

@chucai 用 join 写多个条件果然好用,谢谢。不过,有个疑问如下

page = params[:p] || 1
conditions = []
#这样子直接写在String里面,不会有注入危险么?还是ROR有特殊处理的?
conditions << "name LIKE %#{params[:name][0,20]}%" if params[:name].present?
conditions << "serial LIKE %#{serial}%" if params[:serial].present?
# 最后在使用conditions
conditions.join(" AND ")

对,是有 sql 注入危险

#2 楼 @chucai #3 楼 @milk 这个代码最糟的不是象不象 java,而是有安全漏洞,可以做 SQL 注入了 受到楼主影响,2 楼的代码也是有这个问题的

#5 楼 @fsword 对 但可以修改一下就行

#3 楼 @milk 如果是我写的话 @models = Model @models = @models.scoped(:conditions => ['name LIKE ?', '%#{params[:name][0,20]}%']) unless params[:name].blank? @models = @models.scoped(:conditions => ['serial LIKE ?', '%#{params[:serial][0,20]}%']) unless params[:serial].blank? @models.paginate(params[:p])

楼主的代码,对数据库的操作应该放到 model。多使用 query interface 去查,应该就会 rubyist 化了。 请参考这里: http://guides.rubyonrails.org/active_record_querying.html

#5 楼 @fsword 你能说的具体点吗。因为我之前也差不多这样写过。

#9 楼 @metal 例如这句话

conditions << "serial LIKE %#{serial}%" if params[:serial].present?

攻击者可以在 serial 这个传入的变量上玩很多花样,详情 google 一下就知道了,如果要防范,就是 #8 楼 @xds2000 的建议

另外,稍微靠谱一点的应用都不用 like 来查询,规模大的用搜索引擎,小的可以用华顺写的 https://github.com/huacnlee/redis-search

@fsword 我以为 ROR 中使用?来就相当于 JAVA 中的参数化查询,应该可以避免 SQL 注入了,是我理解错了么?我试了几个常用的注入的字符,没什么问题 我这是个小系统,用的人不过 10 来个人,这个表数据不会超过 1000 条,所以甚至连数据库都用的是 SQLITE

个人觉得 rails 中不应该写那些 sql 语句来进行查询 查询例子 serial_condition=params[:serial].nil? ? '1 = 1' : "serial =‘#{params[:serial]}’" name_condition=params[:name].nil? ? '1 = 1' : "name ='#{params[:serial]}'" User.where(serial_condition).where(name_condition)

分页的话直接用 will_paginate gem 实现,前辈们用心写出来的 gem,bug 又少,时间又少,为啥不用

需要 登录 后方可回复, 如果你还没有账号请 注册新账号