Unbound deferred validators are ignored?

[dairiki]

On IRC this morning, subsu pointed out to me that unbound deferred validators are silently ignored when deserializing. Indeed, this behavior is documented.

This seems completely backwards to me. The validators are supposed to make particular assertions about the integrity of the unserialized data. If I forget to bind one, it is a programming error, and I want an exception thrown. I certainly don't want untrusted input to be passed through unvalidated. This is freaking me out, man!

Is this really what was intended? (Why?) Anyhow, I propose either: changing this behavior (which might be problematic, given that it is documented and all); or adding a second deferred (sub)class (compulsory_deferred or some such) for which errors will be thrown if an attempt is made to use them before they are bound.

[mcdonc]

The intent had something to do with the code in this class: http://svn.repoze.org/repoze.itory/trunk/repoze/itory/content.py ... but that code no longer is operative, it was just an experiment... I don't even really remember the true use case. I'd be fine defaulting Field to raising an error if a deferred value is found, with a flag to the Field constructor to ignore.

[dairiki]

I just spent a few minutes looking through the repoze.itory (awesome name) code, and I'm not much the wiser for it. (I'll take your word for it — I couldn't find a reason to ignore deferred validators in there.)

I will work up a pull request. Here's the plan:

• Add a flag SchemaNode.ignore_unbound_validators, default False.
• Add a new exception deform.UnboundDeferredError which will be raised if a deferred validator is encountered during deserialization. (I think this is right, right? It is a programming error, not a validation error, so deform.Invalid is not the appropriate exception.)

[mcdonc]

Validators aren't the only things that can be deferred (the missing value, the default value, and actually any other attribute of a SchemaNode can be deferred). Now that I've looked at it a bit, I'd probably just ditch the existing behavior and always raise an exception when a deferred is unexpectly encountered. UnboundSchemaError or something. My crazy code can lose.

[dairiki]

Good timing. I just finished hacking up the first plan.

The current behavior w.r.t. deferred missings and defaults is sane (in that unexpected deferreds are unlikely to cause security issues.) (A deferred missing is treated as a required. A deferred default is treated as a null.) Personally, I wouldn't mind if exceptions were thrown in all those cases, but it does change documented behavior, so I suspect it would cheese off someone's code.

I've just grepped the source (deform's too), and those appear to be the only places (besides validator) that explicit checks are made for deferreds. So while it is possible to use deferreds on other attributes, there are no other obvious places to throw errors. (I guess a __getattr__ could be added deferred which would throw an error if a deferred were treated in a non-deferred manner. That seems like a lot of magic, though.)

Anyhow, here's what I came up with: dairiki/colander@b958cc565afb

I'm happy to rework it, if you think it needs it.

[dairiki]

Another option occurred to me while sleeping (fitfully) last night.

We could add a SchemaNode.__getattribute__() which throws UnboundDeferredError whenever an unbound deferred attribute is accessed. (To bypass the check, one would then have to get at the attributes via .__dict__.) UnboundDeferredError would be made a subclass of AttributeError if we go this route.

Optionally, if we want to provide a method to specify "default" values for unbound deferreds, __getattribute__ could try to bind deferreds with an empty kw dict before giving up and raising the error. This has the disadvantage of possibly leading to confusing tracebacks — e.g. KeyErrors from within the users deferred code, outside of a a call to .bind(). (With some effort, most of these errors could maybe be caught and rebranded as UnboundDeferredErrors by __getattribute__.)

[inklesspen]

Any news on this?

[mmerickel]

Perhaps @deferred(required=True) could be implemented next to the basic @deferred. Although colander tends to treat serialization and deserialization separately so it might not be adequate to make the deferred required on both sides so some other options to the decorator can be thought up.

[mcdonc]

I think it just needs to be required. I'm sorry I've not been able to do much with it. Can't promise any ETA, sorry.

[latteier]

I agree that an exception should be raised when deserializing with an unbound validator.

I've run into this and found it confusing that my unbound validator was being ignored.

What exception should be raised? I guess we could add UnboundDeferredError as @dairiki suggests.

[latteier]

I added a PR #172

It's basically what @dairiki did without an option to turn off raising an exception.

[tseaver]

via #172