[Pki-devel] [PATCH] 0016-3 - Fix BasicConstraints min/max path length check

[Endi Sukma Dewata]

On 11/26/2014 1:02 PM, Fraser Tweedale wrote:
> v3 patch.
>
> In this patch, validateConfig() is short-circuited until all params
> have been set, to avoid repetition of logic in the validity check.
>
> Also, a config that causes validation to fail is now reverted on any
> exception, not just EPropertyException.
>
> Finally, the profile still gets created when there is bad config so
> I filed another ticket for that:
> https://fedorahosted.org/pki/ticket/1215

This is getting much more complicated than I anticipated :)
The patch still has some issues:

1. To ensure we revert all invalid config changes, the code should catch 
Throwable instead of Exception when calling the validateConfig(), but 
see below.

2. I'm not sure if allConfigsSet() is the right way to do this. Let's 
say we add a config with an invalid value, it may actually accept the 
invalid value because the validation only happens after all configs are 
set. If a validation error happens later, it will only revert the last 
config set, not the config with the invalid value. Let's say the profile 
doesn't specify all defined configs, the validation may never happen. 
It's also kind of inefficient because it's iterating through all defined 
configs on each config set.

I think ideally the validation should happen after the loop that calls 
setConfig(), for example:

   for (String name : names) {
       String value = ...
       constraint.setConfig(name, value);
   }
   constraint.validateConfig();

We may not even need to revert the invalid config if the code discards 
the constraint object.

If you'd like, feel free to push the original patch since it's already 
ACKed. We can improve the constraint config validation as a separate ticket.

-- 
Endi S. Dewata