3 ms·
Mass assignment is one of the worst practices in typical rails apps. And their fix is to move everything to the models. All so they can have "skinny controllers
by nestlequ1k 15y ago
Mass assignment is one of the worst practices in typical rails apps. And their fix is to move everything to the models. All so they can have "skinny controllers" with one liners like @user.update_attributes(params[:user]).
Seems crazy to me. I've never used attr_protected, or attr_accessible because controllers should actually parse out the params and then explicitly define what's needed to update the models.
It's more glue code, but you're making rails controllers do what they're supposed to do... parse http form input and return results.
- fryguy 15y agoAs someone that's never worked in rails before, the fact that rails allows you to take parameters from the url, and directly update a database object with them is shocking to me. That's like the first thing you learn about securing websites.
- eagsalazar 15y agoHere's another shocker, cars can be driven off cliffs!! Sue Ford!
- hythloday 15y agoAs someone who's never driven a Ford (or, indeed, a car), I would not dream of driving one off a cliff, nor would the Github team. It's entirely possible that I would forget to use attr_protected. Github certainly did.
- jiggy2011 15y agoIndeed , seems more akin to buying a second hand car and forgetting to ask if the brakes are actually connected before driving off.
- jiggy2011 15y agoNot a rails developer here, but shouldn't basic validation inside the model class on persist be able to prevent this from happening?