I agree with 'withOutputParam' comment. It does seem to require some
explanation. Same for the newly introduced return value -1.
The test:
46 // A separate output array is not required, as it is not
used and
47 // the exception is thrown beforehand while calculating
the size
48 // of the encoded bytes using input array
This seems to make an assumption on the implementation. Test should not
depend on the internal impl.
Naoto
On 1/17/19 8:14 AM, Roger Riggs wrote:
Hi Nishit,
In the test, there are a couple of RuntimeExceptions with messages that
start with "As expected"...
That's counter intuitive, that a failure of the test is reported with a
message saying, its normal!
I would use a message like: "XXException should have been thrown..."
The size of Integer.MAX_VALUE - 2 is implementation specific,
I think more typically max -8 is used to get the biggest allocation.
java/util/Base64.java:
Lines 335-342: This optimization is not mentioned in the bug report and
should be a separate review.
245, 686: in outLength(), the 2nd parameter would be easier to understand
as 'throwOOME', meaning to throw OOM if length is too large.
The 'withOutputParam' only has any meaning in the context of the caller.
And even for the private method write the javadoc describing the behavior.
It also makes the call sites clearer, the argument will be true to throw.
Thanks, Roger
On 01/17/2019 02:07 AM, Nishit Jain wrote:
Hi,
Please review the fix for 8210583
Bug: https://bugs.openjdk.java.net/browse/JDK-8210583
Webrev: http://cr.openjdk.java.net/~nishjain/8210583/webrev.03/
CSR: https://bugs.openjdk.java.net/browse/JDK-8215633
Issue: Base64.Encoder.encode and Base64.Decoder.decode methods
incorrectly throw unspecified exception e.g.
NegativeArraySizeException, when the input byte array size is too
large such that the calculated output byte size goes beyond the
max-integer boundary and wraps around.
Fix: Throw an OutOfMemoryError if the output byte array/buffer or
memory can not be allocated. There is an unrelated change in
encodeToString(byte[]) where a string instance is created using
JavaLangAccess.newStringNoRepl(byte[], ISO_8859_1)instead of string
constructor, to save memory.
Regards,
Nishit Jain
