From ce14b65b4b029b746790e53f43fc0733efcebe9a Mon Sep 17 00:00:00 2001 From: Marek Urbaniak <8502071+marekoid@users.noreply.github.com> Date: Thu, 26 Mar 2020 15:32:37 +0000 Subject: [PATCH 1/2] Add Base64 char validation Improve naming Follow up Mike's feedback: https://github.com/pusher/pusher-websocket-java/pull/236#discussion_r397986501 --- .../java/com/pusher/client/util/Base64.java | 31 ++++++++++++------- .../com/pusher/client/util/Base64Test.java | 27 ++++++++++++++++ 2 files changed, 47 insertions(+), 11 deletions(-) create mode 100644 src/test/java/com/pusher/client/util/Base64Test.java diff --git a/src/main/java/com/pusher/client/util/Base64.java b/src/main/java/com/pusher/client/util/Base64.java index 0da3d12c..67350935 100644 --- a/src/main/java/com/pusher/client/util/Base64.java +++ b/src/main/java/com/pusher/client/util/Base64.java @@ -1,37 +1,46 @@ package com.pusher.client.util; -// copied from: https://stackoverflow.com/a/4265472/501940 +import static java.util.Arrays.fill; + +// copied from: https://stackoverflow.com/a/4265472/501940 and improved (naming, char validation) public class Base64 { - private final static char[] ALPHABET = + private final static char[] CHAR_INDEX_TABLE = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/".toCharArray(); - private static int[] toInt = new int[128]; + private static int[] charToIndexSparseMappingArray = new int[128]; static { - for (int i = 0; i < ALPHABET.length; i++) { - toInt[ALPHABET[i]] = i; + fill(charToIndexSparseMappingArray, -1); + for (int i = 0; i < CHAR_INDEX_TABLE.length; i++) { + charToIndexSparseMappingArray[CHAR_INDEX_TABLE[i]] = i; } } + private static int toInt(char character) { + int retVal = charToIndexSparseMappingArray[character]; + if (retVal == -1) throw new IllegalArgumentException("invalid char: " + character); + return retVal; + } + public static byte[] decode(String base64String) { - int delta = base64String.endsWith("==") ? 2 : base64String.endsWith("=") ? 1 : 0; - byte[] buffer = new byte[base64String.length() * 3 / 4 - delta]; + int paddingSize = base64String.endsWith("==") ? 2 : base64String.endsWith("=") ? 1 : 0; + byte[] buffer = new byte[base64String.length() * 3 / 4 - paddingSize]; int mask = 0xFF; int index = 0; for (int i = 0; i < base64String.length(); i += 4) { - int c0 = toInt[base64String.charAt(i)]; - int c1 = toInt[base64String.charAt(i + 1)]; + int c0 = toInt(base64String.charAt(i)); + int c1 = toInt(base64String.charAt(i + 1)); buffer[index++] = (byte) (((c0 << 2) | (c1 >> 4)) & mask); if (index >= buffer.length) { return buffer; } - int c2 = toInt[base64String.charAt(i + 2)]; + int c2 = toInt(base64String.charAt(i + 2)); buffer[index++] = (byte) (((c1 << 4) | (c2 >> 2)) & mask); if (index >= buffer.length) { return buffer; } - int c3 = toInt[base64String.charAt(i + 3)]; + int c3 = toInt(base64String.charAt(i + 3)); buffer[index++] = (byte) (((c2 << 6) | c3) & mask); } return buffer; diff --git a/src/test/java/com/pusher/client/util/Base64Test.java b/src/test/java/com/pusher/client/util/Base64Test.java new file mode 100644 index 00000000..28439fc7 --- /dev/null +++ b/src/test/java/com/pusher/client/util/Base64Test.java @@ -0,0 +1,27 @@ +package com.pusher.client.util; + +import static com.google.common.truth.Truth.assertThat; + +import org.junit.Test; + +public class Base64Test { + + @Test + public void decodeValidChars() { + String validChars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + assertThat(Base64.decode(validChars)).isNotEmpty(); + } + + // https://en.wikipedia.org/wiki/Base64#URL_applications + @Test(expected = IllegalArgumentException.class) + public void failDecodingMinusChar() { + Base64.decode("-"); + } + + // https://en.wikipedia.org/wiki/Base64#URL_applications + @Test(expected = IllegalArgumentException.class) + public void failDecodingUnderscoreChar() { + Base64.decode("_"); + } + +} From 1d43fdba6750a6af6907b2c2c20aa94717132e19 Mon Sep 17 00:00:00 2001 From: Marek Urbaniak <8502071+marekoid@users.noreply.github.com> Date: Thu, 26 Mar 2020 16:03:37 +0000 Subject: [PATCH 2/2] Use a better term As it's not a temporary store for I/O. --- .../java/com/pusher/client/util/Base64.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/pusher/client/util/Base64.java b/src/main/java/com/pusher/client/util/Base64.java index 67350935..44d5431a 100644 --- a/src/main/java/com/pusher/client/util/Base64.java +++ b/src/main/java/com/pusher/client/util/Base64.java @@ -25,24 +25,24 @@ private static int toInt(char character) { public static byte[] decode(String base64String) { int paddingSize = base64String.endsWith("==") ? 2 : base64String.endsWith("=") ? 1 : 0; - byte[] buffer = new byte[base64String.length() * 3 / 4 - paddingSize]; + byte[] retVal = new byte[base64String.length() * 3 / 4 - paddingSize]; int mask = 0xFF; int index = 0; for (int i = 0; i < base64String.length(); i += 4) { int c0 = toInt(base64String.charAt(i)); int c1 = toInt(base64String.charAt(i + 1)); - buffer[index++] = (byte) (((c0 << 2) | (c1 >> 4)) & mask); - if (index >= buffer.length) { - return buffer; + retVal[index++] = (byte) (((c0 << 2) | (c1 >> 4)) & mask); + if (index >= retVal.length) { + return retVal; } int c2 = toInt(base64String.charAt(i + 2)); - buffer[index++] = (byte) (((c1 << 4) | (c2 >> 2)) & mask); - if (index >= buffer.length) { - return buffer; + retVal[index++] = (byte) (((c1 << 4) | (c2 >> 2)) & mask); + if (index >= retVal.length) { + return retVal; } int c3 = toInt(base64String.charAt(i + 3)); - buffer[index++] = (byte) (((c2 << 6) | c3) & mask); + retVal[index++] = (byte) (((c2 << 6) | c3) & mask); } - return buffer; + return retVal; } }