CODEC-289: Add strict decoding to BaseNCodecInput/OutputStream

Author: aherbert

src/changes/changes.xml

@@ -45,6 +45,7 @@ The <action> type attribute can be add,update,fix,remove.
     <release version="1.15" date="YYYY-MM-DD" description="Feature and fix release.">
       <action issue="CODEC-264" dev="aherbert" due-to="Andy Seaborne" type="fix">MurmurHash3: Ensure hash128 maintains the sign extension bug.</action>
       <action issue="CODEC-280" dev="aherbert" type="update">Base32/Base64/BCodec: Added strict decoding property to control handling of trailing bits. Default lenient mode discards them without error. Strict mode raise an exception.</action>
+      <action issue="CODEC-289" dev="aherbert" type="update">Base32/Base64 Input/OutputStream: Added strict decoding property to control handling of trailing bits. Default lenient mode discards them without error. Strict mode raise an exception.</action>
     </release>
 
     <release version="1.14" date="2019-12-30" description="Feature and fix release.">

src/main/java/org/apache/commons/codec/binary/BaseNCodecInputStream.java

@@ -47,6 +47,40 @@ protected BaseNCodecInputStream(final InputStream input, final BaseNCodec baseNC
         this.baseNCodec = baseNCodec;
     }
 
+    /**
+     * Sets the decoding behavior when the input bytes contain leftover trailing bits that
+     * cannot be created by a valid encoding. This setting is transferred to the instance
+     * of {@link BaseNCodec} used to perform decoding.
+     *
+     * <p>The default is false for lenient encoding. Decoding will compose trailing bits
+     * into 8-bit bytes and discard the remainder.
+     *
+     * <p>Set to true to enable strict decoding. Decoding will raise an
+     * {@link IllegalArgumentException} if trailing bits are not part of a valid encoding.
+     *
+     * @param strictDecoding Set to true to enable strict decoding; otherwise use lenient decoding.
+     * @see BaseNCodec#setStrictDecoding(boolean)
+     * @since 1.15
+     */
+    public void setStrictDecoding(boolean strictDecoding) {
+        baseNCodec.setStrictDecoding(strictDecoding);
+    }
+
+    /**
+     * Returns true if decoding behavior is strict. Decoding will raise an
+     * {@link IllegalArgumentException} if trailing bits are not part of a valid encoding.
+     *
+     * <p>The default is false for lenient encoding. Decoding will compose trailing bits
+     * into 8-bit bytes and discard the remainder.
+     *
+     * @return true if using strict decoding
+     * @see #setStrictDecoding(boolean)
+     * @since 1.15
+     */
+    public boolean isStrictDecoding() {
+        return baseNCodec.isStrictDecoding();
+    }
+
     /**
      * {@inheritDoc}
      *

src/main/java/org/apache/commons/codec/binary/BaseNCodecOutputStream.java

@@ -60,6 +60,40 @@ public BaseNCodecOutputStream(final OutputStream output, final BaseNCodec basedC
         this.doEncode = doEncode;
     }
 
+    /**
+     * Sets the decoding behavior when the input bytes contain leftover trailing bits that
+     * cannot be created by a valid encoding. This setting is transferred to the instance
+     * of {@link BaseNCodec} used to perform decoding.
+     *
+     * <p>The default is false for lenient encoding. Decoding will compose trailing bits
+     * into 8-bit bytes and discard the remainder.
+     *
+     * <p>Set to true to enable strict decoding. Decoding will raise an
+     * {@link IllegalArgumentException} if trailing bits are not part of a valid encoding.
+     *
+     * @param strictDecoding Set to true to enable strict decoding; otherwise use lenient decoding.
+     * @see BaseNCodec#setStrictDecoding(boolean)
+     * @since 1.15
+     */
+    public void setStrictDecoding(boolean strictDecoding) {
+        baseNCodec.setStrictDecoding(strictDecoding);
+    }
+
+    /**
+     * Returns true if decoding behavior is strict. Decoding will raise an
+     * {@link IllegalArgumentException} if trailing bits are not part of a valid encoding.
+     *
+     * <p>The default is false for lenient encoding. Decoding will compose trailing bits
+     * into 8-bit bytes and discard the remainder.
+     *
+     * @return true if using strict decoding
+     * @see #setStrictDecoding(boolean)
+     * @since 1.15
+     */
+    public boolean isStrictDecoding() {
+        return baseNCodec.isStrictDecoding();
+    }
+
     /**
      * Writes the specified {@code byte} to this output stream.
      *

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

@@ -557,4 +557,32 @@ public void testSkipWrongArgument() throws Throwable {
             b32stream.skip(-10);
         }
     }
+
+    /**
+     * Test strict decoding.
+     *
+     * @throws Exception
+     *             for some failure scenarios.
+     */
+    @Test
+    public void testStrictDecoding() throws Exception {
+        for (final String s : Base32Test.BASE32_IMPOSSIBLE_CASES) {
+            final byte[] encoded = StringUtils.getBytesUtf8(s);
+            Base32InputStream in = new Base32InputStream(new ByteArrayInputStream(encoded), false);
+            // Default is lenient decoding; it should not throw
+            assertFalse(in.isStrictDecoding());
+            Base32TestData.streamToBytes(in);
+
+            // Strict decoding should throw
+            in = new Base32InputStream(new ByteArrayInputStream(encoded), false);
+            in.setStrictDecoding(true);
+            assertTrue(in.isStrictDecoding());
+            try {
+                Base32TestData.streamToBytes(in);
+                fail();
+            } catch (final IllegalArgumentException ex) {
+                // expected
+            }
+        }
+    }
 }

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

@@ -17,6 +17,7 @@
 
 package org.apache.commons.codec.binary;
 
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -333,4 +334,36 @@ public void testWriteToNullCoverage() throws Exception {
         }
     }
 
+    /**
+     * Test strict decoding.
+     *
+     * @throws Exception
+     *             for some failure scenarios.
+     */
+    @Test
+    public void testStrictDecoding() throws Exception {
+        for (final String s : Base32Test.BASE32_IMPOSSIBLE_CASES) {
+            final byte[] encoded = StringUtils.getBytesUtf8(s);
+            ByteArrayOutputStream bout = new ByteArrayOutputStream();
+            Base32OutputStream out = new Base32OutputStream(bout, false);
+            // Default is lenient decoding; it should not throw
+            assertFalse(out.isStrictDecoding());
+            out.write(encoded);
+            out.close();
+            assertTrue(bout.size() > 0);
+
+            // Strict decoding should throw
+            bout = new ByteArrayOutputStream();
+            out = new Base32OutputStream(bout, false);
+            out.setStrictDecoding(true);
+            assertTrue(out.isStrictDecoding());
+            try {
+                out.write(encoded);
+                out.close();
+                fail();
+            } catch (final IllegalArgumentException ex) {
+                // expected
+            }
+        }
+    }
 }

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

@@ -48,7 +48,11 @@ public class Base32Test {
         {"foobar" ,"MZXW6YTBOI======"},
     };
 
-    private static final String[] BASE32_IMPOSSIBLE_CASES = {
+    /**
+     * Example test cases with valid characters but impossible combinations of
+     * trailing characters (i.e. cannot be created during encoding).
+     */
+    static final String[] BASE32_IMPOSSIBLE_CASES = {
         "MC======",
         "MZXE====",
         "MZXWB===",

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

@@ -570,4 +570,32 @@ public void testSkipWrongArgument() throws Throwable {
             b64stream.skip(-10);
         }
     }
+
+    /**
+     * Test strict decoding.
+     *
+     * @throws Exception
+     *             for some failure scenarios.
+     */
+    @Test
+    public void testStrictDecoding() throws Exception {
+        for (final String s : Base64Test.BASE64_IMPOSSIBLE_CASES) {
+            final byte[] encoded = StringUtils.getBytesUtf8(s);
+            Base64InputStream in = new Base64InputStream(new ByteArrayInputStream(encoded), false);
+            // Default is lenient decoding; it should not throw
+            assertFalse(in.isStrictDecoding());
+            Base64TestData.streamToBytes(in);
+
+            // Strict decoding should throw
+            in = new Base64InputStream(new ByteArrayInputStream(encoded), false);
+            in.setStrictDecoding(true);
+            assertTrue(in.isStrictDecoding());
+            try {
+                Base64TestData.streamToBytes(in);
+                fail();
+            } catch (final IllegalArgumentException ex) {
+                // expected
+            }
+        }
+    }
 }

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

@@ -18,6 +18,7 @@
 package org.apache.commons.codec.binary;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -341,4 +342,36 @@ public void testWriteToNullCoverage() throws Exception {
         }
     }
 
+    /**
+     * Test strict decoding.
+     *
+     * @throws Exception
+     *             for some failure scenarios.
+     */
+    @Test
+    public void testStrictDecoding() throws Exception {
+        for (final String s : Base64Test.BASE64_IMPOSSIBLE_CASES) {
+            final byte[] encoded = StringUtils.getBytesUtf8(s);
+            ByteArrayOutputStream bout = new ByteArrayOutputStream();
+            Base64OutputStream out = new Base64OutputStream(bout, false);
+            // Default is lenient decoding; it should not throw
+            assertFalse(out.isStrictDecoding());
+            out.write(encoded);
+            out.close();
+            assertTrue(bout.size() > 0);
+
+            // Strict decoding should throw
+            bout = new ByteArrayOutputStream();
+            out = new Base64OutputStream(bout, false);
+            out.setStrictDecoding(true);
+            assertTrue(out.isStrictDecoding());
+            try {
+                out.write(encoded);
+                out.close();
+                fail();
+            } catch (final IllegalArgumentException ex) {
+                // expected
+            }
+        }
+    }
 }

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

@@ -45,7 +45,11 @@ public class Base64Test {
 
     private static final Charset CHARSET_UTF8 = StandardCharsets.UTF_8;
 
-    private static final String[] BASE64_IMPOSSIBLE_CASES = {
+    /**
+     * Example test cases with valid characters but impossible combinations of
+     * trailing characters (i.e. cannot be created during encoding).
+     */
+    static final String[] BASE64_IMPOSSIBLE_CASES = {
         "ZE==",
         "ZmC=",
         "Zm9vYE==",