Trevor Turk
2008-Jul-31 16:12 UTC
Consider re-implementing ProtectedAttributeAssignmentError
A gotcha that has bitten me quite a few times - when you try to mass- assign a protected attribute, it fails "silently" (but appears in the debug log). There is some useful discussion about this subject here: http://dev.rubyonrails.org/ticket/9966 But I thought this worth bringing up after seeing this commit: http://github.com/rails/rails/commit/108db00aa90fe266564483ab301cf0669cae600f Perhaps this protected attribute assignment error is worth revisiting with the addition of the extremely handy rescue_from additions that have made their way into core? http://github.com/rails/rails/commit/90c930f45c5c6766306929241462ffff8f67b86e Of course, I''m getting better about remembering to add attributes via attr_accessible after being bitten by this one a few times, but perhaps others have been confounded by this gotcha as well? Thanks, - Trevor --~--~---------~--~----~------------~-------~--~----~ 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 rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
Chris Cruft
2008-Aug-01 13:02 UTC
Re: Consider re-implementing ProtectedAttributeAssignmentError
I like the idea of assignment to a protected attribute being an exception. I kinda grok the link to ARes, but it seems that the change in AR is overly "practical" and doesn''t pass the sniff test: assigning to a protected attribute looks like an exception, it can be severe (security-wise) and it used to be an exception. On Jul 31, 12:12 pm, Trevor Turk <trevort...@gmail.com> wrote:> A gotcha that has bitten me quite a few times - when you try to mass- > assign a protected attribute, it fails "silently" (but appears in the > debug log). There is some useful discussion about this subject here: > > http://dev.rubyonrails.org/ticket/9966 > > But I thought this worth bringing up after seeing this commit: > > http://github.com/rails/rails/commit/108db00aa90fe266564483ab301cf066... > > Perhaps this protected attribute assignment error is worth revisiting > with the addition of the extremely handy rescue_from additions that > have made their way into core? > > http://github.com/rails/rails/commit/90c930f45c5c6766306929241462ffff... > > Of course, I''m getting better about remembering to add attributes via > attr_accessible after being bitten by this one a few times, but > perhaps others have been confounded by this gotcha as well? > > Thanks, > - Trevor--~--~---------~--~----~------------~-------~--~----~ 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 rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
Michael Koziarski
2008-Aug-04 13:08 UTC
Re: Consider re-implementing ProtectedAttributeAssignmentError
On Thu, Jul 31, 2008 at 6:12 PM, Trevor Turk <trevorturk@gmail.com> wrote:> > A gotcha that has bitten me quite a few times - when you try to mass- > assign a protected attribute, it fails "silently" (but appears in the > debug log). There is some useful discussion about this subject here: > > http://dev.rubyonrails.org/ticket/9966 > > But I thought this worth bringing up after seeing this commit: > > http://github.com/rails/rails/commit/108db00aa90fe266564483ab301cf0669cae600f > > Perhaps this protected attribute assignment error is worth revisiting > with the addition of the extremely handy rescue_from additions that > have made their way into core? > > http://github.com/rails/rails/commit/90c930f45c5c6766306929241462ffff8f67b86e > > Of course, I''m getting better about remembering to add attributes via > attr_accessible after being bitten by this one a few times, but > perhaps others have been confounded by this gotcha as well?The silent dropping of values bugs me, but in this case I think the cure is worse than the disease. When we had it enabled previously all of my exception trackers were spammed with dozens of random junk coming from adventurous users or broken spam bots. The current behaviour doesn''t have any security related downsides, and it''s just being slightly postel-friendly in the way it behaves. We could add a hook to make it easier for plugins to handle this situation, but at present I think it''s just a little too annoying for enabling it by default. -- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ 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 rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
Steven A Bristol
2008-Aug-04 14:28 UTC
Re: Consider re-implementing ProtectedAttributeAssignmentError
> The silent dropping of values bugs me, but in this case I think the > cure is worse than the disease. When we had it enabled previously all > of my exception trackers were spammed with dozens of random junk > coming from adventurous users or broken spam bots. >I would love it if this would only raise an exception in non-production environments. steven bristol --~--~---------~--~----~------------~-------~--~----~ 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 rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
Trevor Turk
2008-Aug-04 23:49 UTC
Re: Consider re-implementing ProtectedAttributeAssignmentError
On Aug 4, 8:08 am, "Michael Koziarski" <mich...@koziarski.com> wrote:> The silent dropping of values bugs me, but in this case I think the > cure is worse than the disease... > > We could add a hook to make it easier for plugins to handle this > situation, but at present I think it''s just a little too annoying for > enabling it by default.Yeah, I''m not sure this "issue" warrants an exception either. Still, debugging problems that arise due to about attempts to assign protected (and perhaps read-only) attributes seems unnecessarily difficult to me at present. I think I''ve learned my lesson by now, but perhaps this is a good target for framework polishing? I''m happy to work on a patch if there''s agreement on a smart way to approach the problem. Thanks for your responses all, - Trevor --~--~---------~--~----~------------~-------~--~----~ 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 rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
Michael Koziarski
2008-Aug-06 10:52 UTC
Re: Consider re-implementing ProtectedAttributeAssignmentError
> Yeah, I''m not sure this "issue" warrants an exception either. Still, > debugging problems that arise due to about attempts to assign > protected (and perhaps read-only) attributes seems unnecessarily > difficult to me at present. I think I''ve learned my lesson by now, but > perhaps this is a good target for framework polishing? I''m happy to > work on a patch if there''s agreement on a smart way to approach the > problem.I think the first step might be to prise apart that code so that the handling of protected attributes is handled in a single method like def handle_unprotected_attribute_assignment(*attributes) logger.something end You could then override that method with a plugin to aid with debugging during dev and test.> Thanks for your responses all, > - Trevor > > >-- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ 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 rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
Trevor Turk
2008-Aug-06 22:30 UTC
Re: Consider re-implementing ProtectedAttributeAssignmentError
On Aug 6, 5:52 am, "Michael Koziarski" <mich...@koziarski.com> wrote:> I think the first step might be to prise apart that code so that the > handling of protected attributes is handled in a single method like > > def handle_unprotected_attribute_assignment(*attributes) > logger.something > endI''ll give this a shot. Thanks for your reply, - Trevor --~--~---------~--~----~------------~-------~--~----~ 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 rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
Trevor Turk
2008-Aug-12 02:53 UTC
Re: Consider re-implementing ProtectedAttributeAssignmentError
On Aug 6, 5:52 am, "Michael Koziarski" <mich...@koziarski.com> wrote:> I think the first step might be to prise apart that code so that the > handling of protected attributes is handled in a single method like > > > def handle_unprotected_attribute_assignment(*attributes) > > logger.something > > endI''ve attached a patch and example monkey patch over here: http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/804-refactor-protected-attribute-assignment-warnings#ticket-804-2 I think this would meet my needs, but it should open up the possibility for doing other interesting things as well. Thanks again for your responses all, - Trevor --~--~---------~--~----~------------~-------~--~----~ 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 rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---