Hi Luca, Excellent! I agree with the majority of what you propose. The only thing is I don't think assign_attributes is an adequate solution. Many times what one needs to assign is in the params hash, and this needs to happen conditionally. I suppose one can use slice/except like this...
def create @comment = @commentable.comments.build(params[:reply].except (:spam, :important, :troll)) @comment.assign_attributes(params[:reply].slice (:spam, :important, :troll)) if admin? end This has some duplication and seems a bit more convoluted than it needs to be. I prefer my original proposal of defining if the params hash is trusted. def create params[:reply].trusted! if admin? @comment = @commentable.comments.build(params[:reply]) end Much cleaner. But other than that I agree with everything else. Thanks! Ryan On Jun 1, 4:23 am, Luca Guidi <[email protected]> wrote: > Assumptions: > - attr_accessible/attr_protected aren't used because coders often > forget about those macros, and also because they are annoying to work > with. The current API doesn't encourage them. > - attr_protected is dangerous (rails-spikes.com) > - Protection should be at class level > - Controllers should decide which attributes should be passed > - The API should be pessimistic by default > - A warning isn't enough, since security issues are involved, the > communication's semantic should be stronger (ie an exception) > - Ease the assignment of non-accessible attributes > > Design decisions: > - attr_protected remotion > - All the attributes are unaccessible by default, this encourage the > usage of attr_accessible > - Add the attr_accessible :all facility > - Add a global flag (ActiveRecord::Base.protect_from_mass_assigment, > true by default) for backward compatibility and as context facility. > As instance, when you hack with IRb or the application console, > probably you don't want to be annoyed by security concerns. > - Raise an exception when try to mass assign non-accessible > attributes. This should make the code more robust, and provide a > stronger pointer to the problem. > - Introduction of ActiveRecord::Base.assign_attributes as non- > accessible attributes assignment facility. > Example: > before: > def create > @comment = @commentable.comments.build(params[:reply]) > @comment.user = current_user > @comment.request = request > > # ... > end > > after: > def create > @comment = @commentable.comments.build(params[:reply]) > @comment.assign_attributes :user => current_user, :request => > request > > # ... > end > > It shouldn't be hard to migrate legacy code: > - Existing code with attr_protected, should be changed, with the > opposite semantic. > - Existing code with attr_accessible, is already OK. > - Existing code without any protection, should introduce > attr_accessible in each class. > - Existing code with a large codebase or more complex examples can > turn off the protection in first instance, then re-enable when the > migration is done. > - Optional: coders can use #assign_attributes, as replacement of > manual assignment (@comment.user = current_user) > > Hope this make sense. > > Cheers, > Luca. > > -- > blog:www.lucaguidi.com --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
