Some plugins/gems break when set attributes internally #1

Open
maxim opened this Issue Jul 5, 2009 · 3 comments

Comments

Projects
None yet
3 participants
@maxim

maxim commented Jul 5, 2009

Take delayed_job for example. It sets priority and other metadata to its internal Delayed::Job model. Of course this causes an error, unless I add .trust to its hashes or set attr_accessible :all on delayed job's model. I've been thinking about a way to work around that, and found that the only reasonable place to guard against external params is in controllers. Maybe it makes sense to explicitly check where method #trusted? is being called, and always return true unless its context is_a?(ActionController::Base). Everywhere else you're pretty much on your own - since developer should be allowed to freely exchange params internally. If someone sets user-supplied attributes anywhere but in their controller - well then ... they suck.

@maxim

This comment has been minimized.

Show comment Hide comment
@maxim

maxim Jul 6, 2009

Since it's almost impossible to find out in which context you're calling trust (except maybe looking at Kernel#caller and relying on rails directory structure conventions) - maybe it's best to add a config option to disable attr_accessible requirement for all models.

maxim commented Jul 6, 2009

Since it's almost impossible to find out in which context you're calling trust (except maybe looking at Kernel#caller and relying on rails directory structure conventions) - maybe it's best to add a config option to disable attr_accessible requirement for all models.

@ghost

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Jan 6, 2010

I ran into this problem in a project where I was using acts-as-taggable-on. It uses constructs like this: Tagging.create(:tag_id => new_tag.id, :context => tag_type, :taggable => self, :tagger => owner) to create various records.

I've patched acts-as-taggable-on (http://github.com/mbleigh/acts-as-taggable-on/commit/f3bf4e743722f191b7c08090bfc49f8b3513e1b0), so it sets attr_accessible, but this could break apps where the Tag/Tagging are extended with custom attributes (I have, for instance, a application where I add a position column to Tagging's).

At first I thought forcing attr_accessible was a good thing, but now I'm not so sure. I think a config option is probably best.

ghost commented Jan 6, 2010

I ran into this problem in a project where I was using acts-as-taggable-on. It uses constructs like this: Tagging.create(:tag_id => new_tag.id, :context => tag_type, :taggable => self, :tagger => owner) to create various records.

I've patched acts-as-taggable-on (http://github.com/mbleigh/acts-as-taggable-on/commit/f3bf4e743722f191b7c08090bfc49f8b3513e1b0), so it sets attr_accessible, but this could break apps where the Tag/Tagging are extended with custom attributes (I have, for instance, a application where I add a position column to Tagging's).

At first I thought forcing attr_accessible was a good thing, but now I'm not so sure. I think a config option is probably best.

@smudge

This comment has been minimized.

Show comment Hide comment
@smudge

smudge Feb 23, 2011

I ran into this issue with the vestal_versions gem. The solution for me was to add attr_accessible to the versions model in an initializer. (/config/initializers/vestal_versions_override.rb):

    VestalVersions::Version.class_eval do
        attr_accessible :all
    end

smudge commented Feb 23, 2011

I ran into this issue with the vestal_versions gem. The solution for me was to add attr_accessible to the versions model in an initializer. (/config/initializers/vestal_versions_override.rb):

    VestalVersions::Version.class_eval do
        attr_accessible :all
    end
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment