search

harini-nagarajan-5578 / owasp-esapi-java

Automatically exported from code.google.com/p/owasp-esapi-java
Other
3 stars 0 forks source link

Javadoc Inaccuracy in getRandomInteger() and getRandomReal() #217

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago 
What steps will reproduce the problem?
1.Call ESAPI.randomizer().getRandomInteger(1, 2) multiple times

What is the expected output? 
According to 
http://owasp-esapi-java.googlecode.com/svn/trunk_doc/latest/org/owasp/esapi/refe
rence/DefaultRandomizer.html#getRandomInteger(int, int), the expected output is 
the random return of sometimes 1 (min) and sometimes 2 (max).

What do you see instead?
I always get 1 in return.

What version of the product are you using? On what operating system?
rc4, but the latest source code on the repository contains the same discrepancy 
vs. the javadoc

Does this issue affect only a specified browser or set of browsers?
no.

Please provide any additional information below.

---------

On

http://owasp-esapi-java.googlecode.com/svn/trunk_doc/latest/org/owasp/esapi/refe
rence/DefaultRandomizer.html#getRandomInteger(int, int)

we read, among other things:

    Parameters:
    min - the minimum integer that will be returned
    max - the maximum integer that will be returned

I believe that "max" is larger than the maximum integer that will be returned. 
In other words, a call to ESAPI.randomizer().getRandomInteger(1, 2) will never 
return anything else than 1.

A formulation like this would be more accurate (taken from 
http://download.oracle.com/javase/6/docs/api/java/util/Random.html#nextInt(int))
:

    Returns a pseudorandom, uniformly distributed int value between min (inclusive) and max (exclusive)

The ESAPI DefaultRandomizer uses the above mentioned JavaSE6 Random method 
(inherited by SecureRandom)

     public int getRandomInteger(int min, int max) {
            return secureRandom.nextInt(max - min) + min;
    }

I see however that the javadoc for the ESAPI Interface 
http://owasp-esapi-java.googlecode.com/svn/trunk_doc/latest/org/owasp/esapi/Rand
omizer.html#getRandomInteger(int, int) contains the same definition as does the 
DefaultRandomizer. In case the DefaultRandomizer is implementing the Interface 
incorrectly, then the reference implementation would need to be changed to meet 
the javadoc in stead. If I understand correctly, however, it is more 
conventional for the returned values of such methods to exclude the parameters' 
"max" value, rather than to include it.

Original issue reported on code.google.com by chris.fr...@gmail.com on 27 Mar 2011 at 10:31

GoogleCodeExporter commented 8 years ago 
Just bumped into this little gremlin in some unit testing. Would be really good 
to know which direction the code will go - I need to know the best form of 
cludge to put in my code.

Original comment by oliver.d...@gmail.com on 11 Feb 2013 at 2:29