[PATCH] CipherStream produces new byte array on every update or doFinal operation

[Valerie Peng]

Thanks! I have test/modified the patch locally but have yet to create a 
bug to track it.
Valerie

On 6/12/2015 4:52 AM, Sean Mullan wrote:
> FYI, I created a bug to track this: 
> https://bugs.openjdk.java.net/browse/JDK-8087327
>
> --Sean
>
> On 02/20/2015 02:50 PM, Valerie Peng wrote:
>>
>> I started with this, but I have a pressing higher priority tasks at 
>> hand.
>> So, it will take me a few weeks to wrap this up.
>> Regards,
>> Valerie
>>
>> On 2/17/2015 4:06 PM, Dai Nakanishi wrote:
>>> Hi,
>>>
>>> CipherInputStream and CipherOutputStream call the Cipher methods update
>>> and doFinal which produce new byte array. It may consume a large amount
>>> of memory.
>>>
>>> The purpose of my patch is to avoid this. Could you review the patch?
>>>
>>> Thanks,
>>> Dai
>>>
>>> --- old/src/share/classes/javax/crypto/CipherInputStream.java    Thu
>>> Feb 12 11:55:05 2015 +0900
>>> +++ new/src/share/classes/javax/crypto/CipherInputStream.java    Tue
>>> Feb 17 17:12:08 2015 +0900
>>> @@ -108,34 +108,50 @@
>>>           read = true;
>>>           if (readin == -1) {
>>>               done = true;
>>> +            ensureCapacity(0);
>>>               try {
>>> -                obuffer = cipher.doFinal();
>>> -            } catch (IllegalBlockSizeException | BadPaddingException
>>> e) {
>>> +                ofinish = cipher.doFinal(obuffer, 0);
>>> +            } catch (IllegalBlockSizeException | BadPaddingException
>>> +                    | ShortBufferException e) {
>>>                   obuffer = null;
>>>                   throw new IOException(e);
>>>               }
>>> -            if (obuffer == null)
>>> +            if (ofinish == 0)
>>>                   return -1;
>>>               else {
>>>                   ostart = 0;
>>> -                ofinish = obuffer.length;
>>>                   return ofinish;
>>>               }
>>>           }
>>> +        ensureCapacity(readin);
>>>           try {
>>> -            obuffer = cipher.update(ibuffer, 0, readin);
>>> +            ofinish = cipher.update(ibuffer, 0, readin, obuffer);
>>>           } catch (IllegalStateException e) {
>>>               obuffer = null;
>>>               throw e;
>>> +        } catch (ShortBufferException e) {
>>> +            obuffer = null;
>>> +            throw new IOException(e);
>>>           }
>>>           ostart = 0;
>>> -        if (obuffer == null)
>>> -            ofinish = 0;
>>> -        else ofinish = obuffer.length;
>>>           return ofinish;
>>>       }
>>>
>>>       /**
>>> +     * Ensure the capacity of the output buffer for the next
>>> +     * update or doFinal operation, given the input length
>>> +     *<code>inputLen</code>  (in bytes)
>>> +     *
>>> +     * @param inputLen the input length (in bytes)
>>> +     */
>>> +    private void ensureCapacity(int inputLen) {
>>> +        int outputLen = cipher.getOutputSize(inputLen);
>>> +        if (obuffer == null || obuffer.length<  outputLen) {
>>> +            obuffer = new byte[outputLen];
>>> +        }
>>> +    }
>>> +
>>> +    /**
>>>        * Constructs a CipherInputStream from an InputStream and a
>>>        * Cipher.
>>>        *<br>Note: if the specified input stream or cipher is
>>> @@ -311,10 +327,12 @@
>>>           try {
>>>               // throw away the unprocessed data
>>>               if (!done) {
>>> -                cipher.doFinal();
>>> +                ensureCapacity(0);
>>> +                cipher.doFinal(obuffer, 0);
>>>               }
>>>           }
>>> -        catch (BadPaddingException | IllegalBlockSizeException ex) {
>>> +        catch (BadPaddingException | IllegalBlockSizeException
>>> +                | ShortBufferException ex) {
>>>               /* If no data has been read from the stream to be
>>> en/decrypted,
>>>                  we supress any exceptions, and close quietly. */
>>>               if (read) {
>>>
>>>
>>> --- old/src/share/classes/javax/crypto/CipherOutputStream.java    Thu
>>> Feb 12 11:55:05 2015 +0900
>>> +++ new/src/share/classes/javax/crypto/CipherOutputStream.java    Tue
>>> Feb 17 18:48:01 2015 +0900
>>> @@ -74,6 +74,9 @@
>>>       // the buffer holding data ready to be written out
>>>       private byte[] obuffer;
>>>
>>> +    // the number of bytes stored in the output buffer
>>> +    private int ostored = 0;
>>> +
>>>       // stream status
>>>       private boolean closed = false;
>>>
>>> @@ -118,10 +121,15 @@
>>>        */
>>>       public void write(int b) throws IOException {
>>>           ibuffer[0] = (byte) b;
>>> -        obuffer = cipher.update(ibuffer, 0, 1);
>>> -        if (obuffer != null) {
>>> -            output.write(obuffer);
>>> -            obuffer = null;
>>> +        ensureCapacity(1);
>>> +        try {
>>> +            ostored = cipher.update(ibuffer, 0, 1, obuffer);
>>> +        } catch (ShortBufferException e) {
>>> +            throw new IOException(e);
>>> +        }
>>> +        if (ostored>  0) {
>>> +            output.write(obuffer, 0, ostored);
>>> +            ostored = 0;
>>>           }
>>>       };
>>>
>>> @@ -155,10 +163,15 @@
>>>        * @since      JCE1.2
>>>        */
>>>       public void write(byte b[], int off, int len) throws 
>>> IOException {
>>> -        obuffer = cipher.update(b, off, len);
>>> -        if (obuffer != null) {
>>> -            output.write(obuffer);
>>> -            obuffer = null;
>>> +        ensureCapacity(len);
>>> +        try {
>>> +            ostored = cipher.update(b, off, len, obuffer);
>>> +        } catch (ShortBufferException e) {
>>> +            throw new IOException(e);
>>> +        }
>>> +        if (ostored>  0) {
>>> +            output.write(obuffer, 0, ostored);
>>> +            ostored = 0;
>>>           }
>>>       }
>>>
>>> @@ -177,9 +190,9 @@
>>>        * @since      JCE1.2
>>>        */
>>>       public void flush() throws IOException {
>>> -        if (obuffer != null) {
>>> -            output.write(obuffer);
>>> -            obuffer = null;
>>> +        if (obuffer != null&&  ostored>  0) {
>>> +            output.write(obuffer, 0, ostored);
>>> +            ostored = 0;
>>>           }
>>>           output.flush();
>>>       }
>>> @@ -206,14 +219,31 @@
>>>           }
>>>
>>>           closed = true;
>>> +        ensureCapacity(0);
>>>           try {
>>> -            obuffer = cipher.doFinal();
>>> -        } catch (IllegalBlockSizeException | BadPaddingException e) {
>>> +            ostored = cipher.doFinal(obuffer, 0);
>>> +        } catch (IllegalBlockSizeException | BadPaddingException
>>> +                | ShortBufferException e) {
>>>               obuffer = null;
>>> +            ostored = 0;
>>>           }
>>>           try {
>>>               flush();
>>>           } catch (IOException ignored) {}
>>>           out.close();
>>> +    }
>>> +
>>> +    /**
>>> +     * Ensure the capacity of the output buffer for the next
>>> +     * update or doFinal operation, given the input length
>>> +     *<code>inputLen</code>  (in bytes)
>>> +     *
>>> +     * @param inputLen the input length (in bytes)
>>> +     */
>>> +    private void ensureCapacity(int inputLen) {
>>> +        int outputLen = cipher.getOutputSize(inputLen);
>>> +        if (obuffer == null || obuffer.length<  outputLen) {
>>> +            obuffer = new byte[outputLen];
>>> +        }
>>>       }
>>>   }
>>>