[Pki-devel] [PATCH] 36 - Fix for track ticket 493 in Dogtag 10.0.2 - No interpolation for password fields.

[Endi Sukma Dewata]

On 3/4/2013 10:03 PM, Abhishek Koneru wrote:
> Please review the patch attached for fixing the ticket 493 in Dogtag
> 10.0.2.
>
> Tested in both in both interactive and file-passing (-f option) of
> pkispawn.
>
> --Abhishek

Some comments:

1. In set_property() the new code checks whether the new property is 
sensitive. If it is the value will be added directly into the dict, 
otherwise it will use the ConfigParser.set(). This code might not be 
necessary. To my understanding the interpolation will be evaluated 
during retrieval only (in get() and items()), so changing how the value 
is stored would not affect the interpolation.

2. In the new code the sensitive_parameters will be parsed each time 
flatten_master_dict() is called. It might be better to parse the list 
once right after the file is loaded in read_pki_configuration_file(), 
and store the parsed value as an attribute so it can be used by other 
methods without parsing it again.

3. The list_items() could be simplified. It's not necessary to merge the 
section with the default section because interpolation only works in the 
same section. I think you could call ConfigParser.items(section, True) 
to get all (param, raw_value) pairs. If the parameter is not sensitive 
then call ConfigParser.get(section, param) to get the interpolated value 
and use it to replace the raw value. Then either return all tuples as a 
list like the original items() or put them in a dict like in your patch.

4. What does the 'list' in list_items() mean? It might be better to use 
the same name as the original method it's trying to replace (e.g. items()).

-- 
Endi S. Dewata