[PATCH] Strict controller parameters
Claudio Martínez
http://trac.turbogears.org/turbogears/ticket/694/
"""Unexpected keyword arguments never get to a controller or validator
because they are cleared in the way by adapt_call. This function only
leaves the parameters used by the controller.
This can cause many problems, for example if your controller has an
optional keyword argument called limit and someone somewhere mistypes
limit as linit, you would be using the default value. Depending on the
situation, this can cost a lot of time.
When you enable tg.strict_parameters = True in app.cfg, passing
unexpected arguments to a controller will raise an exception, just like
a normal python function.
"""
I have to add that validators clear the unexpected parameters so you
won't get an exception, but the validation fails and the corresponding
message is added to cherrypy.request.validation_errors. I didn't touch
this, this is the current behavior of the validators.
Kevin Dangoor
>
> I just submitted ticket with a patch attached.
>
> http://trac.turbogears.org/turbogears/ticket/694/
>
> """Unexpected keyword arguments never get to a controller or validator
> because they are cleared in the way by adapt_call. This function only
> leaves the parameters used by the controller.
>
> This can cause many problems, for example if your controller has an
> optional keyword argument called limit and someone somewhere mistypes
> limit as linit, you would be using the default value. Depending on the
> situation, this can cost a lot of time.
I think you're right that this can make troubleshooting more
difficult. This behavior in the controller is fairly new. Rather than
just going ahead and adding a switch, it'd be good to discuss whether
people want this kind of behavior.
I'm pretty sure that Simon and I talked about it a little bit when he
implemented that code, but it's an important enough point to bring up
with the group here.
Should unexpected parameters raise an exception every time?
Kevin
Simon Belak
The idea was we should not allow something as simple as an incomplete
link resulting in errors (500).
Having said that, I agree with Claudio, making this behaviour optional
can save time during development.
Cheers,
Simon
Baruch
plan was to add the ugly *args, **kw to all methods to avoid getting
such exceptions on the production server. My code should be robust
enough to handle missing parameters, but I don't want to care about
extra ones.
Maybe during develpment we could log extra variables that we throw away
so that the developer can see in the log the reason for his problems
and not need to guess it. I don't see any real use to get the
exceptions besides to help debugging and the log should be a nice way
to get this information and it can also be turned on the production
server if the python logging is redirected to a file there.
Baruch
Jorge Godoy
> Should unexpected parameters raise an exception every time?
Hmmm... I'd try applying the saying "be liberal with what you get and strict
with what you send". What I think is the best approach is:
- if using something like **kw accept everything (even with validators
missing for unexpected parameters)
- if using keywords accept just what is declared in keywords and raise an
exception on unexpected parameters, as Python does.
>>> def testing(a = None, b = None):
... print a
... print b
...
>>> testing()
None
None
>>> testing(a = 123)
123
None
>>> testing(b = 123)
None
123
>>> testing(a = 123, b = 234)
123
234
>>> testing(c = 123)
Traceback (most recent call last):
File "<stdin>", line 1, in ?
TypeError: testing() got an unexpected keyword argument 'c'
>>>
Why? Because one might pass things outside of the declared form for some
other purpose. If it is not declared in the validator, it is the developer's
responsability to validate it by hand (using formencode or not) and when you
declare just some keywords on your method's signature you are explicitly
stating that you don't want anything else.
--
Jorge Godoy <jgo...@gmail.com>
Jorge Godoy
> The idea was we should not allow something as simple as an incomplete
> link resulting in errors (500).
You mean missing parameters? If it is that, validators should be used to
avoid that as well. And for mandatory keywords you should provide no default,
so that you also get the error.
Aren't none of the error_handler and exception_handler decorators triggered by
this?
--
Jorge Godoy <jgo...@gmail.com>
Simon Belak
> Simon Belak <simon...@hruska.si> writes:
>
>> The idea was we should not allow something as simple as an incomplete
>> link resulting in errors (500).
>
> You mean missing parameters? If it is that, validators should be used to
> avoid that as well. And for mandatory keywords you should provide no default,
> so that you also get the error.
Missing, misspelled, obsolite, ...
Throwing everything unexpected away is relatively safe and still saves
some time. For me there is little appeal in a framework as heavy-weight
as TG, if I still have to handle all eventualities myself.
>
> Aren't none of the error_handler and exception_handler decorators triggered by
> this?
exception_handler is.
Cheers,
Simon
Kevin Dangoor
> Kevin wrote:
> > Should unexpected parameters raise an exception every time?
>
> The idea was we should not allow something as simple as an incomplete
> link resulting in errors (500).
>
> Having said that, I agree with Claudio, making this behaviour optional
> can save time during development.
OK. We can take this patch. Should we turn this on by default in
dev.cfg (and off in prod.cfg)?
Kevin
Jorge Vargas
Simon Belak
Sounds good.
Cheers,
Simon