[commons-codec] 03/18: Add support for strict decoding - addresses review comments by @garydgregory and @aherbert

[aherbert]

This is an automated email from the ASF dual-hosted git repository.

aherbert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-codec.git

commit 564c47cbcbe3ade57c106ed83fa83d910d30d408
Author: Adam Retter <adam.ret...@googlemail.com>
AuthorDate: Tue Apr 21 12:19:33 2020 +0200

    Add support for strict decoding - addresses review comments by 
@garydgregory and @aherbert
---
 .../org/apache/commons/codec/binary/Base16.java    | 36 +++++++++++++++++++---
 .../apache/commons/codec/binary/Base16Test.java    | 20 ++++++++++++
 2 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/src/main/java/org/apache/commons/codec/binary/Base16.java 
b/src/main/java/org/apache/commons/codec/binary/Base16.java
index f94c115..1732f69 100644
--- a/src/main/java/org/apache/commons/codec/binary/Base16.java
+++ b/src/main/java/org/apache/commons/codec/binary/Base16.java
@@ -17,6 +17,7 @@
 
 package org.apache.commons.codec.binary;
 
+import org.apache.commons.codec.CodecPolicy;
 import org.apache.commons.codec.DecoderException;
 
 import java.nio.charset.Charset;
@@ -66,6 +67,19 @@ public class Base16 extends BaseNCodec {
         this.charset = charset;
     }
 
+    /**
+     * Creates a Base16 codec used for decoding and encoding.
+     *
+     * @param toLowerCase {@code true} converts to lowercase, {@code false} to 
uppercase.
+     * @param charset the charset.
+     * @param decodingPolicy Decoding policy.
+     */
+    protected Base16(final boolean toLowerCase, final Charset charset, final 
CodecPolicy decodingPolicy) {
+        super(BYTES_PER_UNENCODED_BLOCK, BYTES_PER_ENCODED_BLOCK, 0, 0,
+                PAD_DEFAULT, decodingPolicy);
+        this.toLowerCase = toLowerCase;
+        this.charset = charset;
+    }
 
     @Override
     void encode(final byte[] data, final int offset, final int length, final 
Context context) {
@@ -88,11 +102,11 @@ public class Base16 extends BaseNCodec {
 
     @Override
     void decode(final byte[] data, final int offset, final int length, final 
Context context) {
-        if (context.eof) {
-            return;
-        }
-        if (length < 0) {
+        if (context.eof || length < 0) {
             context.eof = true;
+            if (context.ibitWorkArea > 0) {
+                validateTrailingCharacter();
+            }
             return;
         }
 
@@ -137,6 +151,20 @@ public class Base16 extends BaseNCodec {
         }
     }
 
+    /**
+     * Validates whether decoding allows an entire final trailing character 
that cannot be
+     * used for a complete byte.
+     *
+     * @throws IllegalArgumentException if strict decoding is enabled
+     */
+    private void validateTrailingCharacter() {
+        if (isStrictDecoding()) {
+            throw new IllegalArgumentException("Strict decoding: Last encoded 
character is a valid base 16 alphabet" +
+                    "character but not a possible encoding. " +
+                    "Decoding requires at least two characters to create one 
byte.");
+        }
+    }
+
     @Override
     protected boolean isInAlphabet(final byte value) {
         if (value >= '0' && value <= '9') {
diff --git a/src/test/java/org/apache/commons/codec/binary/Base16Test.java 
b/src/test/java/org/apache/commons/codec/binary/Base16Test.java
index e0bf357..eb273fa 100644
--- a/src/test/java/org/apache/commons/codec/binary/Base16Test.java
+++ b/src/test/java/org/apache/commons/codec/binary/Base16Test.java
@@ -17,6 +17,7 @@
 
 package org.apache.commons.codec.binary;
 
+import org.apache.commons.codec.CodecPolicy;
 import org.apache.commons.codec.DecoderException;
 import org.apache.commons.codec.EncoderException;
 import org.apache.commons.lang3.ArrayUtils;
@@ -28,6 +29,7 @@ import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.Random;
 
+import static org.apache.commons.codec.CharEncoding.UTF_8;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -617,4 +619,22 @@ public class Base16Test {
 
         assertEquals("Until next time!", decoded);
     }
+
+    @Test(expected=IllegalArgumentException.class)
+    public void testStrictDecoding() {
+        final String encoded = "aabbccdde";  // Note the trailing `e` which 
does not make up a hex-pair and so is only 1/2 byte
+
+        final Base16 b16 = new Base16(true, CHARSET_UTF8, CodecPolicy.STRICT);
+        b16.decode(StringUtils.getBytesUtf8(encoded));
+    }
+
+    @Test
+    public void testLenientDecoding() {
+        final String encoded = "aabbccdde";  // Note the trailing `e` which 
does not make up a hex-pair and so is only 1/2 byte
+
+        final Base16 b16 = new Base16(true, CHARSET_UTF8, CodecPolicy.LENIENT);
+
+        final byte[] decoded = b16.decode(StringUtils.getBytesUtf8(encoded));
+        assertArrayEquals(new byte[] {(byte)0xaa, (byte)0xbb, (byte)0xcc, 
(byte)0xdd}, decoded);
+    }
 }