JDK-8215633 : Base64.Encoder.encode and Base64.Decoder.decode should specify OOME if the output bytes array/buffer of the needed size can not be allocated
  • Type: CSR
  • Component: core-libs
  • Sub-Component: java.util
  • Priority: P4
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 13
  • Submitted: 2018-12-19
  • Updated: 2019-01-16
  • Resolved: 2019-01-16
Related Reports
CSR :  
Description
Summary
-------

Base64.Encoder.encode and Base64.Decoder.decode should throw OutOfMemoryError if the output bytes array/buffer of the needed size can not be allocated.

Problem
-------

The existing Base64.Encoder.encode and Base64.Decoder.decode methods do not specify the error to be thrown when the encoded/decoded byte array of needed size can not be allocated. The current implementation throws an unspecified exception, for example, NegativeArraySizeException.


Solution
--------

Add an `OutOfMemoryError` statement to `Base64.Encoder` and `Base64.Decoder`, if `encode` and `decode` methods fail to allocate the output array/buffer or memory.

Specification
-------------

Update Base64.Encoder spec

from:

    * <p> Unless otherwise noted, passing a {@code null} argument to
    * a method of this class will cause a
    * {@link java.lang.NullPointerException NullPointerException} to
    * be thrown.

to:

    * <p> Unless otherwise noted, passing a {@code null} argument to
    * a method of this class will cause a
    * {@link java.lang.NullPointerException NullPointerException} to
    * be thrown.
    * <p> If the encoded byte output of the needed size can not
    *     be allocated, the encode methods of this class will
    *     cause an {@link java.lang.OutOfMemoryError OutOfMemoryError}
    *     to be thrown.

Update Base64.Decoder spec

from:

    * <p> Unless otherwise noted, passing a {@code null} argument to
    * a method of this class will cause a
    * {@link java.lang.NullPointerException NullPointerException} to
    * be thrown.

to:

    * <p> Unless otherwise noted, passing a {@code null} argument to
    * a method of this class will cause a
    * {@link java.lang.NullPointerException NullPointerException} to
    * be thrown.
    * <p> If the decoded byte output of the needed size can not
    *     be allocated, the decode methods of this class will
    *     cause an {@link java.lang.OutOfMemoryError OutOfMemoryError}
    *     to be thrown.


Comments
Moving to Approved.
16-01-2019

Updated the spec by adding a statement to Base64.Encoder and Base64.Decoder about the OOME to be thrown, if encode/decode fail to allocate memory to the encoded/decoded output bytes
16-01-2019

Just as we'll often include a type-level or package-level note "BTW, unless otherwise specified, if you method methods here a null, you should expect an NPE", I think it would be reasonable to include a type-level statement "if you need a buffer that would be larger than what can be allocated, you can get a OOME." I think the readability of the spec is degraded if there are too many "throws NPE for a null input" statements and likewise the implied constraint that a too-large buffer should throw an OOME should not be explicitly repeated too often IMO. As the maximum array size on an implementation is not documented, if a buffer larger than Integer.MAX_VALUE would be needed than OOME would be specified, there would be a gray area for smaller values.
14-01-2019

I think it could be useful for conformance test to have these encode methods specify their behavior when call with input that encodes to an array larger than the maximum allowed.
11-01-2019

I don't get the reason of not specifying it, want to ask a question for the sake of my understanding, when should an API specify an explicit OOME? For example, `Files.read(InputStream, int)`, `InputStream.readAllBytes()`, `InputStream.readNBytes(int)` specify it in API doc, but `Base64.Encoder.encode`/`Base64.Decoder.decode` APIs should not, even though all these APIs failure is on same condition that they fail to allocate the memory to a byte[]/buffer because of the large size?
11-01-2019

I don't think it is necessary or helpful to have explicit @throws documentation for the out of memory situations discussed here. The CSR can still cover the behavioral change. Marking the request as pended while the utility of explicit @throws clauses is discussed.
10-01-2019

OOM is the better of the choices when new memory is to be allocated, the input size is not directly the cause of the problem, since the size needed is computed based on the encoding. The exception message in the implementation seems to be clear about the cause being due to the encoding or decoding function.
10-01-2019

[~darcy] [~alanb], Not sure if that was the correct thought, but IAE was chosen because the large size of source "byte[]" or "ByteBuffer" (input argument) is the reason why the output encoded/decoded bytes length is going beyond allocatable range.
10-01-2019

Thanks for the feedback, updated it to throw OOME rather than IAE
10-01-2019

I suspect IAE was chosen here because the 2-arg encode method throws IAE when called with an output array isn't large enough for the encoded bytes. In that case, it's the argument passed to the method that is at fault and so IAE is appropriate. In the case of methods that return an array and the array to encode the input is >2G then throw OOME seem more appropriate. There are several other examples of this (e.g. InputStream.readAllBytes). These other examples throw OOME for size just below Integer.MAX_VALUE too because the VM is limited to about MAX_VALUE-8 (depends on VM options of course).
09-01-2019

IllegalArgumentException doesn't strike me as the most reasonable failure mode in this case. Some kind of OutOfMemoryError seems more reasonable as there is not an easy way to validate the byte array being processed. Especially since the max array size in practice is an implementation-dependent property. Pending the request until this matter is more fully discussed.
09-01-2019