mirror of https://github.com/rails/rails
Don't short-circuit reject_if proc
When updating an associated record via nested attribute hashes the reject_if proc could be bypassed if the _destroy flag was set in the attribute hash and allow_destroy was set to false. The fix is to only short-circuit if the _destroy flag is set and the option allow_destroy is set to true. It also fixes an issue where a new record wasn't created if _destroy was set and the option allow_destroy was set to false. CVE-2015-7577
This commit is contained in:
parent
221937c8ba
commit
5875bc3ade
|
@ -523,7 +523,7 @@ module ActiveRecord
|
|||
# has_destroy_flag? or if a <tt>:reject_if</tt> proc exists for this
|
||||
# association and evaluates to +true+.
|
||||
def reject_new_record?(association_name, attributes)
|
||||
has_destroy_flag?(attributes) || call_reject_if(association_name, attributes)
|
||||
will_be_destroyed?(association_name, attributes) || call_reject_if(association_name, attributes)
|
||||
end
|
||||
|
||||
# Determines if a record with the particular +attributes+ should be
|
||||
|
@ -532,7 +532,8 @@ module ActiveRecord
|
|||
#
|
||||
# Returns false if there is a +destroy_flag+ on the attributes.
|
||||
def call_reject_if(association_name, attributes)
|
||||
return false if has_destroy_flag?(attributes)
|
||||
return false if will_be_destroyed?(association_name, attributes)
|
||||
|
||||
case callback = self.nested_attributes_options[association_name][:reject_if]
|
||||
when Symbol
|
||||
method(callback).arity == 0 ? send(callback) : send(callback, attributes)
|
||||
|
@ -541,6 +542,15 @@ module ActiveRecord
|
|||
end
|
||||
end
|
||||
|
||||
# Only take into account the destroy flag if <tt>:allow_destroy</tt> is true
|
||||
def will_be_destroyed?(association_name, attributes)
|
||||
allow_destroy?(association_name) && has_destroy_flag?(attributes)
|
||||
end
|
||||
|
||||
def allow_destroy?(association_name)
|
||||
self.nested_attributes_options[association_name][:allow_destroy]
|
||||
end
|
||||
|
||||
def raise_nested_attributes_record_not_found!(association_name, record_id)
|
||||
raise RecordNotFound, "Couldn't find #{self.class._reflect_on_association(association_name).klass.name} with ID=#{record_id} for #{self.class.name} with ID=#{id}"
|
||||
end
|
||||
|
|
|
@ -161,6 +161,19 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase
|
|||
assert man.reload.interests.empty?
|
||||
end
|
||||
|
||||
def test_reject_if_is_not_short_circuited_if_allow_destroy_is_false
|
||||
Pirate.accepts_nested_attributes_for :ship, reject_if: ->(a) { a[:name] == "The Golden Hind" }, allow_destroy: false
|
||||
|
||||
pirate = Pirate.create!(catchphrase: "Stop wastin' me time", ship_attributes: { name: "White Pearl", _destroy: "1" })
|
||||
assert_equal "White Pearl", pirate.reload.ship.name
|
||||
|
||||
pirate.update!(ship_attributes: { id: pirate.ship.id, name: "The Golden Hind", _destroy: "1" })
|
||||
assert_equal "White Pearl", pirate.reload.ship.name
|
||||
|
||||
pirate.update!(ship_attributes: { id: pirate.ship.id, name: "Black Pearl", _destroy: "1" })
|
||||
assert_equal "Black Pearl", pirate.reload.ship.name
|
||||
end
|
||||
|
||||
def test_has_many_association_updating_a_single_record
|
||||
Man.accepts_nested_attributes_for(:interests)
|
||||
man = Man.create(name: 'John')
|
||||
|
|
Loading…
Reference in New Issue