[Freeipa-devel] validating return values in XML-RPC

[John Dennis]

On 10/22/2009 08:38 PM, Rob Crittenden wrote:
> John Dennis wrote:
>> On 10/22/2009 10:45 AM, Jason Gerard DeRose wrote:
>>> So I've been thinking about this as I've been doing the UI
>>> "tuning" (extending meta-data and making the engine smarter). I agree
>>> with John that we need to describe the return values programatically.
>>> We can also kill two birds with one stone here because the description
>>> of the return values is a great way to provide some of the meta-data the
>>> UI needs (and the CLI... there is something in place now, but it's not
>>> easily plugable).
>>>
>>> I personally feel the design of the Param system has held up pretty well
>>> (Rob and Pavel, speak now or forever hold your peace), so I think we
>>> should use the Param classes to describe the return values. This will
>>> really help us reduce code duplication and allow for good plugability
>>> because, as usual, most of our commands are CRUD operations, so we can
>>> generally use some auto-magic to deduce the return values from the
>>> corresponding Object params.
>>>
>>> Thoughts?
>>
>> If you're asking if the description of the return values should be
>> expressed using the same Param classes as the input values then yes I
>> agree. I had pretty much expected we would reuse the Param
>> declarations for the return values.
>>
>> However if you're asking should the description of the return values
>> be magically deduced from the input parameters then no I don't agree,
>> I think both the input and output parameters should be explicitly
>> declared so a programmer can look at the code and see what they are
>> just like in any typed language.
>>
>
> I'm not entirely sure this is going to be possible, at least not at this
> level. The attributes returned will not be this predictable. At best it
> could include just some possible attributes returned and some vague data
> type information.
>
> Take the group plugin for an example.
>
> ipa group-show will return something like a string and a dict. The
> string is the DN of the entry and the dict is the list of attributes
> returned. Within the dict are any number of attributes, some of which we
> can define in advance. So we know that a group requires a cn and a
> description, so we can define that. But what about members? That is an
> optional attribute, do we define that? And a group can be a member of
> other groups, so I guess the same would apply. I guess that's fine, we
> can define optional incoming options, we can do outgoing ones in the
> same way.
>
> Ok, now the interesting part. One can also pass --all to the function
> which returns every attribute in the object. What do we do with these
> unexpected attributes? Now imagine a much more complex object and you
> can see how this quickly becomes unmanageable. Once you start allowing
> unknown things to be returned we are in pretty much the same boat we are
> now only with the false impression that we have control.
>
> On the bright side once the output of show gets defined the rest becomes
> a bit easier. add will return the same type of thing (it calls show on
> the newly added object and returns that). find will return a list of
> these things. delete I believe delete just returns the dn of the entry
> deleted, that's easy too.
>
> There are other commands that can return all kinds of other wacky stuff,
> see the env plugin for that.

O.K. There may be a subset of return values which are difficult to 
declare, in particular those which are the result of an ldap query. 
However in this specific instance we do know the types of the attributes 
because they're defined in the schema. This might be an example of where 
automatic checking makes sense (however for performance reasons we might 
only want to enable such checking during testing). I have not looked at 
the ldap extraction code, but I presume it automatically handles types 
correctly so the likelyhood the types returned in an ldap query dict 
would be wrong are low. Perhaps we can have a flag to mark such return 
values as "pre-validated" if we have the belief the return values are 
algorithmically generated.

However, that's not the primary case I'm concerned about. I'm concerned 
about when the programmer manually decides what is going to be returned. 
What are the permissible "top level" values? In other words at the top 
level what are the possible keys and the types of the matching values? 
If it's an ordered list of values (e.g. an arg list) then what is each 
member of the list supposed to be?

I think it's possible to check every item at the top level. As you point 
out recursive validation might be too much. Although it should be easy 
to recursively visit each value in a dict and make sure none of the 
values are str objects because we never return binary data (at least 
yet, and if we do it would be easy to add a rule which says this 
particular value has an exception).

So in your example you can certainly check that the return values 
consist of two items, the DN which must be a unicode object, and a dict. 
Neither one can be absent nor can there be anything else if so it's an 
error (unless the the top level has "optional" parameters of course).

The most serious problem we face at the moment is the ease with which we 
can incorrectly return a str object instead of unicode. Next is when a 
value is supposed to be integral that it's actually an integer and not a 
string representation of the integer (which works when the only use of 
the returned value is for presentation, but fails when it's used 
programmatically). Finally, at the top level, the plugin must return the 
set of values (no more, no less) of what it declares it will return 
(e.g. honor it's interface contract).

-- 
John Dennis <jdennis at redhat.com>

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/