You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Saksham Srivastava <sa...@citrix.com> on 2014/05/13 15:01:41 UTC
Review Request 21375: CLOUDSTACK-6654: Configkey parameters are not validated
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21375/
-----------------------------------------------------------
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
Re: Review Request 21375: CLOUDSTACK-6654: Configkey parameters are not
validated
Posted by ASF Subversion and Git Services <as...@urd.zones.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21375/#review45223
-----------------------------------------------------------
Commit 5bcd017de6f421a6125406120b39fb8602276dc7 in cloudstack's branch refs/heads/4.4-forward from Saksham Srivastava
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=5bcd017 ]
CLOUDSTACK-6654: Configkey parameters are not validated
- 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
>
>
Re: Review Request 21375: CLOUDSTACK-6654: Configkey parameters are not
validated
Posted by ASF Subversion and Git Services <as...@urd.zones.apache.org>.
-----------------------------------------------------------
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
>
>
Re: Review Request 21375: CLOUDSTACK-6654: Configkey parameters are not
validated
Posted by ASF Subversion and Git Services <as...@urd.zones.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21375/#review45471
-----------------------------------------------------------
Commit 41030e4e3e39de694837d1a6dc2f4905da228d98 in cloudstack's branch refs/heads/master from Saksham Srivastava
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=41030e4 ]
CLOUDSTACK-6654: Configkey parameters are not validated
- 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
>
>