[CODEC-105] ArrayIndexOutOfBoundsException when doing multiple reads() on encoding b64InputStream. https://issues.apache.org/jira/browse/CODEC-105

git-svn-id: https://svn.apache.org/repos/asf/commons/proper/codec/trunk@1062592 13f79535-47bb-0310-9956-ffa450edef68

Author: garydgregory

src/java/org/apache/commons/codec/binary/Base64.java

@@ -406,43 +406,16 @@ private void resizeBuffer() {
     int readResults(byte[] b, int bPos, int bAvail) {
         if (buffer != null) {
             int len = Math.min(avail(), bAvail);
-            if (buffer != b) {
-                System.arraycopy(buffer, readPos, b, bPos, len);
-                readPos += len;
-                if (readPos >= pos) {
-                    buffer = null;
-                }
-            } else {
-                // Re-using the original consumer's output array is only
-                // allowed for one round.
+            System.arraycopy(buffer, readPos, b, bPos, len);
+            readPos += len;
+            if (readPos >= pos) {
                 buffer = null;
             }
             return len;
         }
         return eof ? -1 : 0;
     }
 
-    /**
-     * Sets the streaming buffer. This is a small optimization where we try to buffer directly to the consumer's output
-     * array for one round (if the consumer calls this method first) instead of starting our own buffer.
-     * 
-     * @param out
-     *            byte[] array to buffer directly to.
-     * @param outPos
-     *            Position to start buffering into.
-     * @param outAvail
-     *            Amount of bytes available for direct buffering.
-     */
-    void setInitialBuffer(byte[] out, int outPos, int outAvail) {
-        // We can re-use consumer's original output array under
-        // special circumstances, saving on some System.arraycopy().
-        if (out != null && out.length == outAvail) {
-            buffer = out;
-            pos = outPos;
-            readPos = outPos;
-        }
-    }
-
     /**
      * <p>
      * Encodes all of the provided data, starting at inPos, for inAvail bytes. Must be called at least twice: once with
@@ -493,7 +466,10 @@ void encode(byte[] in, int inPos, int inAvail) {
                     }
                     break;
             }
-            if (lineLength > 0 && pos > 0) {
+            // Don't want to append the CRLF two times in a row, so make sure previous
+            // character is not from CRLF!
+            byte b = lineSeparator[lineSeparator.length - 1];
+            if (lineLength > 0 && pos > 0 && buffer[pos-1] != b) {
                 System.arraycopy(lineSeparator, 0, buffer, pos, lineSeparator.length);
                 pos += lineSeparator.length;
             }
@@ -751,18 +727,8 @@ public byte[] decode(byte[] pArray) {
         if (pArray == null || pArray.length == 0) {
             return pArray;
         }
-        long len = (pArray.length * 3) / 4;
-        byte[] buf = new byte[(int) len];
-        setInitialBuffer(buf, 0, buf.length);
         decode(pArray, 0, pArray.length);
         decode(pArray, 0, -1); // Notify decoder of EOF.
-
-        // Would be nice to just return buf (like we sometimes do in the encode
-        // logic), but we have no idea what the line-length was (could even be
-        // variable).  So we cannot determine ahead of time exactly how big an
-        // array is necessary.  Hence the need to construct a 2nd byte array to
-        // hold the final result:
-
         byte[] result = new byte[pos];
         readResults(result, 0, result.length);
         return result;
@@ -946,23 +912,11 @@ public byte[] encode(byte[] pArray) {
         if (pArray == null || pArray.length == 0) {
             return pArray;
         }
-        long len = getEncodeLength(pArray, lineLength, lineSeparator);
-        byte[] buf = new byte[(int) len];
-        setInitialBuffer(buf, 0, buf.length);
         encode(pArray, 0, pArray.length);
         encode(pArray, 0, -1); // Notify encoder of EOF.
-        // Encoder might have resized, even though it was unnecessary.
-        if (buffer != buf) {
-            readResults(buf, 0, buf.length);
-        }
-        // In URL-SAFE mode we skip the padding characters, so sometimes our
-        // final length is a bit smaller.
-        if (isUrlSafe() && pos < buf.length) {
-            byte[] smallerBuf = new byte[pos];
-            System.arraycopy(buf, 0, smallerBuf, 0, pos);
-            buf = smallerBuf;
-        }
-        return buf;        
+        byte[] buf = new byte[pos - readPos];
+        readResults(buf, 0, buf.length);
+        return buf;
     }
 
     /**

src/java/org/apache/commons/codec/binary/Base64InputStream.java

@@ -166,11 +166,6 @@ public int read(byte b[], int offset, int len) throws IOException {
                 if (!base64.hasData()) {
                     byte[] buf = new byte[doEncode ? 4096 : 8192];
                     int c = in.read(buf);
-                    // A little optimization to avoid System.arraycopy()
-                    // when possible.
-                    if (c > 0 && b.length == len) {
-                        base64.setInitialBuffer(b, offset, len);
-                    }
                     if (doEncode) {
                         base64.encode(buf, 0, c);
                     } else {

src/test/org/apache/commons/codec/binary/Base64InputStreamTest.java

@@ -19,6 +19,7 @@
 
 import java.io.BufferedReader;
 import java.io.ByteArrayInputStream;
+import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.util.Arrays;
@@ -48,6 +49,16 @@ public Base64InputStreamTest(String name) {
         super(name);
     }
 
+    /**
+     * Tests the bug reported in CODEC-105. Bad interactions with InputStream when reading one byte at a time.
+     */
+    public void testCodec105() throws IOException {
+        Base64InputStream in = new Base64InputStream(new Codec105ErrorInputStream(), true, 0, null);
+        for (int i = 0; i < 5; i++) {
+            in.read();
+        }
+    }
+
     /**
      * Test for the CODEC-101 bug:  InputStream.read(byte[]) should never return 0
      * because Java's builtin InputStreamReader hates that.

src/test/org/apache/commons/codec/binary/Codec105ErrorInputStream.java

@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.codec.binary;
+
+import java.io.IOException;
+import java.io.InputStream;
+
+/**
+ * Emits three line-feeds '\n' in a row, one at a time, and then EOF.
+ * 
+ * Recreates the bug described in CODEC-105.
+ * 
+ * @author Apache Software Foundation
+ * @version $Id $
+ * @since 1.5
+ */
+public class Codec105ErrorInputStream extends InputStream {
+    private static final int EOF = -1;
+
+    int countdown = 3;
+
+    public int read() throws IOException {
+        if (this.countdown-- > 0) {
+            return '\n';
+        } else {
+            return EOF;
+        }
+    }
+
+    public int read(byte b[], int pos, int len) throws IOException {
+        if (this.countdown-- > 0) {
+            b[pos] = '\n';
+            return 1;
+        } else {
+            return EOF;
+        }
+    }
+}