Re: [Pki-devel] [PATCH] 0016-3 - Fix BasicConstraints min/max path length check
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