diff options
author | Ryan Petrello <lists@ryanpetrello.com> | 2014-09-23 18:17:48 -0400 |
---|---|---|
committer | Ryan Petrello <lists@ryanpetrello.com> | 2014-09-24 18:00:08 -0400 |
commit | 9358e59af80d9c5f88d5713ca7d1e648bfad1403 (patch) | |
tree | 21f029f24b8a4b89db3e4c6c4acc52b0d99e7630 /pecan | |
parent | 548ac35e3bc85ce8a872c4ec9adf4e39557187ce (diff) | |
download | pecan-9358e59af80d9c5f88d5713ca7d1e648bfad1403.tar.gz |
Improve argspec detection and leniency for wrapped controllers.
Pecan makes abundant use of `inspect.getargspec`, but unless you're very
meticulous in the decorators you wrap your controllers with, the original
argspec is not persisted (and pecan functionality can break in various ways).
When a controller is decorated in a way that breaks argspec, we should instead
attempt to locate the *actual* argspec for the method (not the wrapped
function) and use it.
Additionally, when controllers are missing **kwargs in the method signature to
map optional GET and POST arguments, we shouldn't consider that
a non-routable offense (an HTTP 400); instead, we should just *not* pass
extraneous arguments to the function.
Change-Id: I47fe0496ff6aa105359ee8e5b99f6c80476cc2e9
Diffstat (limited to 'pecan')
-rw-r--r-- | pecan/core.py | 11 | ||||
-rw-r--r-- | pecan/decorators.py | 4 | ||||
-rw-r--r-- | pecan/rest.py | 4 | ||||
-rw-r--r-- | pecan/routing.py | 3 | ||||
-rw-r--r-- | pecan/tests/test_util.py | 68 | ||||
-rw-r--r-- | pecan/util.py | 28 |
6 files changed, 111 insertions, 7 deletions
diff --git a/pecan/core.py b/pecan/core.py index 9174c50..3a6f684 100644 --- a/pecan/core.py +++ b/pecan/core.py @@ -19,7 +19,7 @@ from .compat import urlparse, unquote_plus, izip from .secure import handle_security from .templating import RendererFactory from .routing import lookup_controller, NonCanonicalPath -from .util import _cfg, encode_if_needed +from .util import _cfg, encode_if_needed, getargspec from .middleware.recursive import ForwardRequestException @@ -527,6 +527,15 @@ class PecanBase(object): resp = state.response pecan_state = req.pecan + # If a keyword is supplied via HTTP GET or POST arguments, but the + # function signature does not allow it, just drop it (rather than + # generating a TypeError). + argspec = getargspec(controller) + keys = kwargs.keys() + for key in keys: + if key not in argspec.args and not argspec.keywords: + kwargs.pop(key) + # get the result from the controller result = controller(*args, **kwargs) diff --git a/pecan/decorators.py b/pecan/decorators.py index 45ad635..ed054ec 100644 --- a/pecan/decorators.py +++ b/pecan/decorators.py @@ -1,8 +1,8 @@ -from inspect import getargspec, getmembers, isclass, ismethod, isfunction +from inspect import getmembers, isclass, ismethod, isfunction import six -from .util import _cfg +from .util import _cfg, getargspec __all__ = [ 'expose', 'transactional', 'accept_noncanonical', 'after_commit', diff --git a/pecan/rest.py b/pecan/rest.py index 48ca1f2..9406ad4 100644 --- a/pecan/rest.py +++ b/pecan/rest.py @@ -1,4 +1,4 @@ -from inspect import getargspec, ismethod +from inspect import ismethod import warnings from webob import exc @@ -7,7 +7,7 @@ import six from .core import abort from .decorators import expose from .routing import lookup_controller, handle_lookup_traversal -from .util import iscontroller +from .util import iscontroller, getargspec class RestController(object): diff --git a/pecan/routing.py b/pecan/routing.py index c6abb2e..7acff76 100644 --- a/pecan/routing.py +++ b/pecan/routing.py @@ -1,10 +1,9 @@ import warnings -from inspect import getargspec from webob import exc from .secure import handle_security, cross_boundary -from .util import iscontroller +from .util import iscontroller, getargspec __all__ = ['lookup_controller', 'find_object'] diff --git a/pecan/tests/test_util.py b/pecan/tests/test_util.py new file mode 100644 index 0000000..c1cdfbd --- /dev/null +++ b/pecan/tests/test_util.py @@ -0,0 +1,68 @@ +import functools +import inspect +import unittest + +from pecan import expose +from pecan import util + + +class TestArgSpec(unittest.TestCase): + + @property + def controller(self): + + class RootController(object): + + @expose() + def index(self, a, b, c=1, *args, **kwargs): + return 'Hello, World!' + + return RootController() + + def test_no_decorator(self): + expected = inspect.getargspec(self.controller.index.__func__) + actual = util.getargspec(self.controller.index.__func__) + assert expected == actual + + def test_simple_decorator(self): + def dec(f): + return f + + expected = inspect.getargspec(self.controller.index.__func__) + actual = util.getargspec(dec(self.controller.index.__func__)) + assert expected == actual + + def test_simple_wrapper(self): + def dec(f): + @functools.wraps(f) + def wrapped(*a, **kw): + return f(*a, **kw) + return wrapped + + expected = inspect.getargspec(self.controller.index.__func__) + actual = util.getargspec(dec(self.controller.index.__func__)) + assert expected == actual + + def test_multiple_decorators(self): + def dec(f): + @functools.wraps(f) + def wrapped(*a, **kw): + return f(*a, **kw) + return wrapped + + expected = inspect.getargspec(self.controller.index.__func__) + actual = util.getargspec(dec(dec(dec(self.controller.index.__func__)))) + assert expected == actual + + def test_decorator_with_args(self): + def dec(flag): + def inner(f): + @functools.wraps(f) + def wrapped(*a, **kw): + return f(*a, **kw) + return wrapped + return inner + + expected = inspect.getargspec(self.controller.index.__func__) + actual = util.getargspec(dec(True)(self.controller.index.__func__)) + assert expected == actual diff --git a/pecan/util.py b/pecan/util.py index aa2e683..168ff66 100644 --- a/pecan/util.py +++ b/pecan/util.py @@ -1,10 +1,38 @@ +import inspect import sys +import six + def iscontroller(obj): return getattr(obj, 'exposed', False) +def getargspec(method): + """ + Drill through layers of decorators attempting to locate the actual argspec + for a method. + """ + + argspec = inspect.getargspec(method) + args = argspec[0] + if args and args[0] == 'self': + return argspec + if hasattr(method, '__func__'): + method = method.__func__ + + func_closure = six.get_function_closure(method) + + closure = next( + ( + c for c in func_closure if six.callable(c.cell_contents) + ), + None + ) + method = closure.cell_contents + return getargspec(method) + + def _cfg(f): if not hasattr(f, '_pecan'): f._pecan = {} |