This project is archived and is in readonly mode.
ActiveRecord Association Proxy/Collection create method incorrectly merges attributes
-
C. Bedard
- Tag changed from activerecord rails3, associations, build, create, scope, with_scope to activerecord rails3, associations, build, create, patch, scope, with_scope
More on this issue:
I initially thought the problem was in
ActiveRecord::Base#with_scopebut now after more digging, I found out the problem originates inActiveRecord::Base#initialize, where 2 lines should be reversed in order for scope attributes to be assigned after the attributes Hash that is received as a parameter. Currently the method goes like this:def initialize(attributes = nil) .... .... ensure_proper_type # THE FOLLOWING 2 LINES SHOULD BE REVERSED! populate_with_current_scope_attributes self.attributes = attributes unless attributes.nil? result = yield self if block_given? _run_initialize_callbacks result endThe reason is simple: as it is right now, the scope attributes are first assigned, but then attributes are overwritten with the attributes Hash passed as a parameter. Reversing those to lines solves the problem.
Attached is the patch acting on
ActiveRecord::Base#initialize, which simply assigns scope attributes after the attributes parameter, therby enforcing the scope attributes over the parameter attributes.This is, by the way, the way it was done in Rails 2. Maybe there is a reason for this in Rails 3, but it is not readily apparent.
-
Mike Ragalie
I confirm that this bug exists in edge. And it is potentially a problem, since I can see someone doing something like this:
post = Post.where(:title => "My Favorite Things") post_dup = Post.where(:title => "My Favorite Thingz") comment = post_dup.comments.first post.comments.create(comment.attributes)However, the above patch breaks other AR functionality (some of the tests fail) so I created tests for the problem and I've attached a new fix that passes all tests. In short, the build methods on has_one/has_many call #set_belongs_to_association_for but the create methods do not. I refactored the has_one create methods to operate similarly to the has_many create methods and added in a call to #set_belongs_to_association_for for both association types.
-
Jesse Storimer
+1 for Mike's patch. I definitely agree that this is unexpected behaviour and might raise a security eyebrow.
