In addition to bug #23899, reported separately, there is one other Classpath bug that I've run into while attempting to compile and test BouncyCastle (http://www.bouncycastle.org/) with a free JVM. Currently two of the test cases fail, and as with the previously referenced bug, the fault appears to lie in Classpath. To see the failed test cases, download the latest beta releases of bctest*.jar and bcprov*.jar from http://www.bouncycastle.org/betas/ and then execute the following command: gij -cp bctest-jdk14-133b08.jar:bcprov-jdk14-133b08.jar org.bouncycastle.crypto.test.RegressionTest The errors obtained are: EC: r component wrong. expecting: 308636143175167811492622547300668018854959378758531778147462058306432176 got : 245735575167810545793756990803068481047855238384222907462905357612977107 ECNR: r component wrong. Expected: 308636143175167811492623515537541734843573549327605293463169625072911693 Found : 245735575167810545793757959039942197036469408953296422778612924379456624 I'll follow up with additional details as they become available.
Additional Comments from David Hook <dgh@bund.com.au>: As an additional note, the root of the problem with the test is that by using nextInt the BigInteger class is consuming extra bytes in the data stream which it discards. This makes it very difficult to create a general purpose predictable class based on SecureRandom (or Random for that matter) which can be used for regression testing, as the standard implementation only consumes as many bytes as required, and it is not simple to write a SecureRandom class that can second guess why nextInt is being called. BC already has some tests where nextInt, nextLong, and nextBytes are all used. It is likely more will be added in the future. It's probably also worth noting that the use of nextInt also makes the constructor a lot less efficient than the standard one when a SecureRandom is used as SecureRandom, unlike Random, uses nextBytes() as its principal source.
I see what you mean, but to be blunt about it, the API docs don't specify how many random numbers are to be consumed by that constructor, so it'd be foolish to rely on a certain number. The fact that our implementation is different from Sun's in ways that are not specified is not a bug in itself, even if it makes implementing regression tests harder.
Don't forget the part that "the BigInteger class is consuming extra bytes in the data stream which it discards". I can see no justification for this.
That doesn't matter if it's justified or not, as long as it's not in violation of spec. Which it isn't. The specification says nothing on the number of bytes consumed by that method. If you're running tests written to assume a certain internal, unspecified, behaviour of the class library, then that is simply bad programming practice. You'll have to expect that code to break on different platforms, JVMs and implementations, and that doesn't mean those implementations are necessarily buggy because of it.
Comments from David Hook of the BouncyCastle project: The comment ignores the fact that other BigInteger implementations, including the reference implementation, already conform to the behavior of only consuming what bytes are necessary. It is fair enough to argue that the spec may not document this fact, however the correct answer may be that the spec should be extended to include this fact, as ignoring it makes it almost impossible to write a test which incorporates a number of uses of a SecureRandom class including that of generating BigIntegers as you would need to implement the test class to recognize why nextInt() is being called in addition to just providing a mock random data stream. Implementation is not the only thing that may have holes in it. Having said that, perhaps describing this as an enhancement request would be a better way of putting it, "bug report" is perhaps a bit harsh as it's beginning to sound more like a bug in the spec. While this difference exists it will not be true to say that the BigInteger class in Classpath is compatible with the one in the Sun reference implementation, but it will be true to say that there is nothing to prevent anyone from working with BigInteger making the same "mistake" that is in some of the Bouncy Castle tests, so this problem will continue to rear its head simply because the Classpath project has chosen to be different and it will cause considerably more work in the wider community to deal with this than simply making the BigInteger implementation conformant will, from that point while the stance of sticking with nextInt() may not be necessarily wrong, it is clearly not going to be helpful. To date Classpath is the only VM implementation with this difference that Bouncy Castle is aware of.
i'm changing this to an enhancement as suggested by Charles --the original reported.
Subject: Bug 27372 CVSROOT: /cvsroot/classpath Module name: classpath Changes by: Raif S. Naffah <raif> 06/08/13 00:34:57 Modified files: java/math : BigInteger.java . : ChangeLog Log message: 2006-08-13 Raif S. Naffah <raif@swiftdsl.com.au> PR Classpath/27372 * java/math/BigInteger.java: Updated copyright year. (init): Consume as little bytes as possible. (BigInteger(int, int, Random)): Ensure bitLength bits are used. (valueOf(String, int)): Throw NumberFormatException for malformed strings as per RI's documentation. CVSWeb URLs: http://cvs.savannah.gnu.org/viewcvs/classpath/java/math/BigInteger.java?cvsroot=classpath&r1=1.28&r2=1.29 http://cvs.savannah.gnu.org/viewcvs/classpath/ChangeLog?cvsroot=classpath&r1=1.8372&r2=1.8373
Charles, can we close this one now?
as agreed with the OP (Charles Fry --private communication), i'm closing this PR for now. the Mauve test TestOfPR27372 (in gnu.testlet.java.math.BigInteger) highlights the problem and validates the fix.