Consider re-implementing ProtectedAttributeAssignmentError

[Trevor Turk]

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

[Chris Cruft]

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...

[Michael Koziarski]

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

[Steven A Bristol]

> 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

[Trevor Turk]

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

[Michael Koziarski]

> 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

[Trevor Turk]

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
> end

I'll give this a shot.

Thanks for your reply,
- Trevor

[Trevor Turk]

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
> > end

I'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