-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8876/#review15166
-----------------------------------------------------------


Likitha - this could be done a little better, a few comments if I may...

* You're implementing a blacklisting pattern. That's a good start, but the 
problem is it's hard to figure out every character which should be disallowed.  
A whitelisting pattern is a more comprehensive solution, although it may take a 
little longer to write.

* Ideally, this filtering code should be in a centralized place, like 
cloud-utils, so that we don't have to re-write several times within CloudStack.

* More ideally, this might be a good first case to start using OWASP's ESAPI 
library, in particular something like the getValidSafeHTML() method. You can 
read more about that at 
https://www.owasp.org/index.php/Input_Validation_Cheat_Sheet



- John Kinsella


On Jan. 8, 2013, 5:27 a.m., Likitha Shetty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8876/
> -----------------------------------------------------------
> 
> (Updated Jan. 8, 2013, 5:27 a.m.)
> 
> 
> Review request for cloudstack, Alena Prokharchyk and Rohit Yadav.
> 
> 
> Description
> -------
> 
> Non-printable characters results in empty pages for all users loading the 
> corrupted object in the web interface. It also results in the API call 
> results getting truncated with an error when it encounters the non-printable 
> characters.
> To find if a parameter value contains a control character, every decoded 
> parameter value was matched with the regex [\000-\037\177] as the ASCII 
> non-printable characters are numbers 0 to 31 and 127 decimal.
> 
> 
> This addresses bug CLOUDSTACK-863.
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/api/ApiServer.java f42025c 
> 
> Diff: https://reviews.apache.org/r/8876/diff/
> 
> 
> Testing
> -------
> 
> Manual Testing done, 
> For sample API's (updateVirtualMachine, createVolume, 
> authorizeSecurityGroupIngress etc), provide input values containing 
> character(s) that are
> • ASCII printable - pass 
> • ASCII non-printable - fail with error code 431 and error 'Received value 
> <parameter-value> for parameter <parameter-name> is invalid, contains illegal 
> ASCII non-printable characters' 
> • non-english - pass
> 
> 
> Thanks,
> 
> Likitha Shetty
> 
>

Reply via email to