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.