Throw IllegalArgumentException for characters outside of alphabet

Author: adamretter

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

@@ -151,7 +151,7 @@ void decode(final byte[] data, int offset, final int length, final Context conte
 
         // small optimisation to short-cut the rest of this method when it is fed byte-by-byte
         if (availableChars == 1 && availableChars == dataLen) {
-            context.ibitWorkArea = data[offset];   // store 1/2 byte for next invocation of decode
+            context.ibitWorkArea = decodeOctet(data[offset]) + 1;   // store 1/2 byte for next invocation of decode, we offset by +1 as empty-value is 0
             return;
         }
 
@@ -164,29 +164,42 @@ void decode(final byte[] data, int offset, final int length, final Context conte
         int i = 0;
         if (dataLen < availableChars) {
             // we have 1/2 byte from previous invocation to decode
-            result = decodeTable[context.ibitWorkArea] << BITS_PER_ENCODED_BYTE;
-            result |= decodeTable[data[offset++]];
+            result = (context.ibitWorkArea - 1) << BITS_PER_ENCODED_BYTE;
+            result |= decodeOctet(data[offset++]);
             i = 2;
 
             buffer[context.pos++] = (byte)result;
 
-            // reset for next invocation!
-            context.ibitWorkArea = -1;
+            // reset to empty-value for next invocation!
+            context.ibitWorkArea = 0;
         }
 
         while (i < charsToProcess) {
-            result = decodeTable[data[offset++]] << BITS_PER_ENCODED_BYTE;
-            result |= decodeTable[data[offset++]];
+            result = decodeOctet(data[offset++]) << BITS_PER_ENCODED_BYTE;
+            result |= decodeOctet(data[offset++]);
             i += 2;
             buffer[context.pos++] = (byte)result;
         }
 
         // we have one char of a hex-pair left over
         if (i < dataLen) {
-            context.ibitWorkArea = data[i];   // store 1/2 byte for next invocation of decode
+            context.ibitWorkArea = decodeOctet(data[i]) + 1;   // store 1/2 byte for next invocation of decode, we offset by +1 as empty-value is 0
         }
     }
 
+    private int decodeOctet(final byte octet) {
+        int decoded = -1;
+        if (octet >= 0 && octet < decodeTable.length) {
+            decoded = decodeTable[octet];
+        }
+
+        if (decoded == -1) {
+            throw new IllegalArgumentException("Invalid octet in encoded value: " + (int)octet);
+        }
+
+        return decoded;
+    }
+
     @Override
     void encode(final byte[] data, final int offset, final int length, final Context context) {
         if (context.eof) {

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

@@ -205,16 +205,17 @@ public void testKnownEncodings() {
 
     @Test
     public void testNonBase16Test() {
-        final byte[] bArray = { '%' };
-
-        try {
-            final Base16 b16 = new Base16();
-            final byte[] result = b16.decode(bArray);
-
-            assertEquals("The result should be empty as the test encoded content did "
-                    + "not contain any valid base 16 characters", 0, result.length);
-        } catch (final Exception e) {
-            fail("Exception was thrown when trying to decode invalid Base16 encoded data");
+        final byte[] invalidEncodedChars = { '/', ':', '@', 'G', '%', '`', 'g' };
+
+        final byte[] encoded = new byte[1];
+        for (final byte invalidEncodedChar : invalidEncodedChars) {
+            try {
+                encoded[0] = invalidEncodedChar;
+                new Base16().decode(encoded);
+                fail("IllegalArgumentException should have been thrown when trying to decode invalid Base16 char: " + (char)invalidEncodedChar);
+            } catch (final Exception e) {
+                assertTrue(e instanceof IllegalArgumentException);
+            }
         }
     }
 
@@ -591,14 +592,15 @@ public void testDecodeSingleBytesOptimisation() {
         final byte[] data = new byte[1];
 
         final Base16 b16 = new Base16();
+        assertEquals(0, context.ibitWorkArea);
 
         data[0] = (byte) 'E';
         b16.decode(data, 0, 1, context);
-        assertEquals(69, context.ibitWorkArea);
+        assertEquals(15, context.ibitWorkArea);
 
         data[0] = (byte) 'F';
         b16.decode(data, 0, 1, context);
-        assertEquals(-1, context.ibitWorkArea);
+        assertEquals(0, context.ibitWorkArea);
 
         assertEquals(-17, context.buffer[0]);
     }