perl-catalyst / catalyst-runtime

The Elegant MVC Web Application Framework
265 stars 102 forks source link

immutable $c->req (Catalyst::Request) #85

Open jhannah opened 9 years ago

jhannah commented 9 years ago

It seems to me life would be better if $c->req could not be altered. It's very tempting for Catalyst newbs (myself included, years ago) to alter the http request (filling it with lies), and then regret it later when MyApp grows. Better to stop people from shooting themselves in their future feet. Thanks.

belden commented 9 years ago

Maybe a plugin that makes the request immutable?

jhannah commented 9 years ago

-shrug- When I was learning Catalyst I had already shot myself in the foot long before I would have ever thought to go looking for plugins to stop me from doing something I didn't know was Bad. I would think it best for Catalyst to ship with the safety on. :)

jjn1056 commented 9 years ago

@jhannah I agree the point of catalyst (or one of its reasons to exist) is to guide newbies toward the right thing. I'd like to see Req fully immutable. We might start with a config switch on post params and headers and the PSGI env. I could default to sane mode but let people set a evil switch, something like $app->config->{mutable_request} =1 in case it breaks code for people.

I think that's a pretty doable thing.

jhannah commented 9 years ago

++ I'm all for MYAPP_MUTABLE_REQUEST=1 or whatever. This is Perl after all, people should be free to shoot themselves however they see fit. :) If I should hack something as a pull request, just point me at wherever you want me and I'm happy to do the footwork. (I have not dabbled in PSGI guts before.)

perl-catalyst-sync commented 9 years ago

I think even just setting the request attributes to 'ro' and making sure you can't change request headers and parameters would be a good first step. maybe even just start with test cases like like would be good

On Thu, Feb 19, 2015 at 6:39 PM, Jay Hannah notifications@github.com wrote:

++ I'm all for MYAPP_MUTABLE_REQUEST=1 or whatever. This is Perl after all, people should be free to shoot themselves however they see fit. :) If I should hack something as a pull request, just point me at wherever you want me and I'm happy to do the footwork. (I have not dabbled in PSGI guts before.)

— Reply to this email directly or view it on GitHub https://github.com/perl-catalyst/catalyst-runtime/issues/85#issuecomment-75169390 .

vanstyn commented 9 years ago

There are cases where it is useful and/or needed to modify the request. One such example was in the advent calendar: http://www.catalystframework.org/calendar/2014/18

I'm fine with a config option, but as there are projects and code which rely on the current behavior, I can't see this ever being made a default

perl-catalyst-sync commented 9 years ago

I think we could still make request immutable but make it more obivous how to create a new, clean context from the application object, rather than modify the existing one. but I agree that use case needs to be supported one way or another.

On Fri, Feb 20, 2015 at 1:17 PM, Henry Van Styn notifications@github.com wrote:

There are cases where it is useful and/or needed to modify the request. One such example was in the advent calendar: http://www.catalystframework.org/calendar/2014/18

I'm fine with a config option, but as there are projects and code which rely on the current behavior, I can't see this ever being made a default

— Reply to this email directly or view it on GitHub https://github.com/perl-catalyst/catalyst-runtime/issues/85#issuecomment-75301319 .