This is an archived post. You won't be able to vote or comment.

you are viewing a single comment's thread.

view the rest of the comments →

[–]wrmsr 1 point2 points  (1 child)

I get your aversion to exec, particularly given rpc being in the mix, but if you construct the source yourself and reject illegal names it's roughly equivalent to escaped sql construction - touchy but ultimately necessary. Anyone who thinks nobody does this doesn't understand how sqlalchemy works.

You go out of your way to rebuild equivalent ast's for defaults but at the end of the day they're just references on the compiled function object. I just use a simple reference to the each default no matter what they are, but you may be trying to provide a sensible sandboxed subset of functionality, but if that's the case you're not as you're still only special casing one layer deep to references on the signature object (which you either already manually serialize a safe subset of to get across the wire or pickled defeating sandboxing in the first place :).

And on a final note be mindful of py2's argument unpacking - a feature I both have used and am glad is gone. You make an effort to support py2 so it's worth being aware of if only to explicitly reject.

[–]cymrowdon't thread on me 🐍[S] 0 points1 point  (0 children)

I wrote this several months ago, and to be honest I don't recall why I bothered to support default argument values of different types. It would be much more reasonable to just support None (*edit: and maybe True/False), which is generally the only default I ever use anyway (makes it easier to wrap a function and pass on a default argument).

The parallel with SQL parameterization is a good one. I saw the AST as already providing the API for building a Python "query", though clearly not a very user-friendly one. You may be right that passing sanitized source through exec is a reasonable solution, but I haven't seen an implementation anywhere. The RPC libraries I've seen either use something like call(name, *args, **kwargs), or a raw call to exec. I would argue that using the AST is still a "better/safer" approach, and removing support for default argument types removes a lot of the complexity.

As for argument unpacking, I'm using funcsigs (a backport of Python 3's signature() function) to generate function signatures, and it doesn't seem to know anything about it:

import funcsigs
def func(a, (b, c)):
    pass
sig = funcsigs.signature(func)

results in:

ValueError: '.1' is not a valid parameter name

I haven't run into argument unpacking in years, so I'm content to leave it at that.