----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21375/#review45480 -----------------------------------------------------------
Commit 9771e79b1bb9662fee4b95dd59432a9d77d42cd9 in cloudstack's branch refs/heads/4.4 from Saksham Srivastava [ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=9771e79 ] CLOUDSTACK-6654: Configkey parameters are not validated (cherry picked from commit 5bcd017de6f421a6125406120b39fb8602276dc7) - ASF Subversion and Git Services On May 13, 2014, 1:01 p.m., Saksham Srivastava wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21375/ > ----------------------------------------------------------- > > (Updated May 13, 2014, 1:01 p.m.) > > > Review request for cloudstack, Abhinandan Prateek and Devdeep Singh. > > > Bugs: CLOUDSTACK-6654 > https://issues.apache.org/jira/browse/CLOUDSTACK-6654 > > > Repository: cloudstack-git > > > Description > ------- > > ConfigKey variables values are not validated. So anything like -5.6 or “abc” > as the value of cpu/memory/storage overprovision factors can be set. > Similarly for all of the variables in ConfigKey. > We have a verification mechanism but it is never executed. The code is > unreachable in the preset 4.4 > > In ConfigurationManagerImpl.java: validateConfigurationValue() > > Config c = Config.getConfig(name); > if (c == null) { > s_logger.warn("Did not find configuration " + name + " in Config.java. > Perhaps moved to ConfigDepot?"); > - return null; > } > Since for the ConfigKey parameters ‘c’ is always null, we return null and do > not further validate. > > Fix is to make sure type is validated by using _configDepot.get(name) > > Note: Configkey does not have a range flag. Each range param has to be > considered as per case basis. > Added comments for the same. > > > Diffs > ----- > > server/src/com/cloud/configuration/ConfigurationManagerImpl.java 231b5e1 > > Diff: https://reviews.apache.org/r/21375/diff/ > > > Testing > ------- > > Tested both Configkey variables as well as old Config parameters. > ConfigKey values are now validated before setting them in db. > > The following status message appears when cpu.overprovisioning.factor is set > to incorrect value. > There was an error trying to parse the float value for: > cpu.overprovisioning.factor > > Build passes. > Findbug is clean. > > > Thanks, > > Saksham Srivastava > >
