This project is archived and is in readonly mode.

#5963 ✓stale
Steven

ActiveRecord attempts to mass-assign protected attributes of nested records

Reported by Steven | November 12th, 2010 @ 07:53 PM

In 2.3.9, a commit was introduced to fix #5053 (12bbc34aca509aba5032e8cc8859ef0c0c845cca, https://github.com/rails/rails/commit/12bbc34aca509aba5032e8cc8859e... ).

This is actually a back port of a 3.0 commit:

https://github.com/rails/rails/commit/0057d2df71ec7c2a788acd3b4bade...

So the problem probably exists in 3.0 too.

There is a problem with the line:

t.attributes = f.attributes.except(*keys)

since this can result in an attempt to mass-assign attributes in t that have been protected against mass-assignment with attr_accessible / attr_protected. At one level, this isn't a problem since rails ignores those changes (so things in t won't be changed), but it does log a WARNING

WARNING: Can't mass-assign these protected attributes: created_at, updated_at, parent_id, state

(if you are using libraries like http://henrik.nyh.se/uploads/whiny_protected_attributes.rb then it throws an error too.)

You can work around this by adding additional elements keys so that all protected attributes are excluded.

#current code
keys = ["id"] + t.changes.keys + (f.attribute_names - t.attribute_names)                      

# now make sure any blacklisted attributes aren't assigned                
keys += t.class.protected_attributes.to_a    
                      
#if a whitelist exists, make sure anything present in f that isn't in the whitelist is excluded          
f.attributes.each { |attrib_name, attrib_value| keys << attrib_name unless t.class.accessible_attributes.include?(attrib_name) }

There is one thing I'm worried about that may be a problem now (and wouldn't be fixed by my proposed change) is that if there are any protected attributes in t that should be copied over from f?

If so, we'd need to also do something after the mass-assignment line to copy those values across one by one.

Comments and changes to this ticket

  • Steven

    Steven November 12th, 2010 @ 07:56 PM

    Here is a tar.gz of a tiny rails project with two models (parent & child) that exhibits the problem

  • Subba

    Subba November 12th, 2010 @ 08:45 PM

    Hi steven
    the ticket 5053 has several commits in 3.0. i backported all of them into one commit in 2.3.9.
    the actual change was below commit.

    https://github.com/rails/rails/commit/0057d2df71ec7c2a788acd3b4bade...

  • Steven

    Steven November 12th, 2010 @ 10:15 PM

    I updated the ticket to include that 3.0 commit in the description.

  • rails

    rails February 12th, 2011 @ 11:25 PM

    • State changed from “new” to “open”

    This issue has been automatically marked as stale because it has not been commented on for at least three months.

    The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

    Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

  • rails

    rails February 12th, 2011 @ 11:25 PM

    • no changes were found...
  • rails

    rails February 12th, 2011 @ 11:34 PM

    • State changed from “open” to “stale”
    • Importance changed from “” to “Low”

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Attachments

Pages