複数レコードを一括で更新できるようなフォームを以前にもRails5(5.0.6)で作っていて、Rails5.1(5.1.6)で再利用しようとしたらエラーが出た。
Rails5.0における一括更新の実装
以前はこちらを参考にさせていただいた。
Rails4、fields_forを使って一括更新する処理のベストプラクティスは何だろう
記載されているコードを引用させていただく
<%= form_tag items_path, method: :put do %>
<% @items.each do |item| %>
<%= fields_for "items[]", item do |fi| %>
<%= fi.text_field :name %>
<%= fi.text_field :price %>
<br />
<% end %>
<% end %>
<%= submit_tag %>
<% end %>
class ItemsController < ApplicationController
respond_to :html
def edit
@items = Item.all
end
def update
@items = items_params.map do |id, item_param|
item = Item.find(id)
item.update_attributes(item_param)
item
end
respond_with(@items, location: edit_items_path)
end
private
def items_params
params.permit(items: [:name, :price])[:items]
end
end
上記のようなフォームからsubmitすると、
Parameters:
{ "items" =>
{
"1" => {"name" => "hoge", "price" => "1000"},
"2" => {"name" => "fuga", "price" => "2000"}
}
}
のように、イテレートされたitemのidをkey、フォームから送信されたパラメータをvalueに持つハッシュ形式のパラメータが送信される。
それをコントローラ側で受け取り、
- params[:items]の中身をitems_paramsに格納
- items_paramsをmapメソッドでイテレートし、keyをidに、valueをitem_paramに格納
- 該当するitemをfindしてupdate
という処理を行っている。
Rails5.0はこれでうまくいっていたので5.1でもそのまま使おうと思っていたらエラーが出た。
5.1で出たエラー
undefined method `map' for #ActionController::Parameters:0x007f3db55ab370
Did you mean? tap (NoMethodError)
メソッドがない???
5.1で消されたってことかな。。
5.0の更新処理を見直してみる
5.0の方のログに何かヒントがないかと思い、一括更新のフォームで更新を行った際のログを見直してみたところ、以下のような警告が出ていた。
DEPRECATION WARNING: Method map is deprecated and will be removed in Rails 5.1, as
ActionController::Parameters
no longer inherits from hash.
Using this deprecated behavior exposes potential security problems.
If you continue to use this method you may be creating a security vulnerability in your app that can be exploited.
Instead, consider using one of these documented methods which are not deprecated: http://api.rubyonrails.org/v5.0.6/classes/ActionController/Parameters.html
つまり、Hashにセキュリティリスクが存在するため、ActionController::ParametersはHashクラスからの継承をやめており、そのためmapメソッドが使えなくなっていたためにエラーが発生していた。
代替手段
なんか別の方法ないかな?と思いActionController::Parametersの公式ドキュメント(Ruby on Rails 5.1.6)を見てみた。
keysメソッドを使ってパラメータのkeyになっているidだけ取り出してeachで回せば解決しそう。
というわけでコントローラーを修正してみる。
class ItemsController < ApplicationController
...
def update
@items = items_params.keys.each do |id|
item = Item.find(id)
item.update_attributes(item_params[id])
item
end
respond_with(@items, location: edit_items_path)
end
...
end
これで処理が通るようになった。
でも、そもそもこれでセキュリティ上のリスクって解消できてるのかな?
感想とか
- NoMethodErrorが出た時点で公式ドキュメント見れば良かったし、そもそも5.0で実装した段階でエラーに気づいとけよと
- 結局公式ドキュメント見るのが一番早いってよく見かけるけど最近やっとその実感が出てきた
- セキュアなアプリケーションの作り方とか、そもそもどんなリスクがあるのかとか知りたい。設計する段階から考えたいし。