THRIFT-1612 Base64 encoding is broken
Patch: Andrew Cox
git-svn-id: https://svn.apache.org/repos/asf/thrift/trunk@1343074 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/lib/cpp/src/thrift/protocol/TBase64Utils.cpp b/lib/cpp/src/thrift/protocol/TBase64Utils.cpp
index 12ebaa9..452985c 100644
--- a/lib/cpp/src/thrift/protocol/TBase64Utils.cpp
+++ b/lib/cpp/src/thrift/protocol/TBase64Utils.cpp
@@ -30,16 +30,16 @@
"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
void base64_encode(const uint8_t *in, uint32_t len, uint8_t *buf) {
- buf[0] = kBase64EncodeTable[(in[0] >> 2) & 0x3F];
+ buf[0] = kBase64EncodeTable[(in[0] >> 2) & 0x3f];
if (len == 3) {
- buf[1] = kBase64EncodeTable[((in[0] << 4) + (in[1] >> 4)) & 0x3f];
- buf[2] = kBase64EncodeTable[((in[1] << 2) + (in[2] >> 6)) & 0x3f];
+ buf[1] = kBase64EncodeTable[((in[0] << 4) & 0x30) | ((in[1] >> 4) & 0x0f)];
+ buf[2] = kBase64EncodeTable[((in[1] << 2) & 0x3c) | ((in[2] >> 6) & 0x03)];
buf[3] = kBase64EncodeTable[in[2] & 0x3f];
} else if (len == 2) {
- buf[1] = kBase64EncodeTable[((in[0] << 4) + (in[1] >> 4)) & 0x3f];
- buf[2] = kBase64EncodeTable[(in[1] << 2) & 0x3f];
+ buf[1] = kBase64EncodeTable[((in[0] << 4) & 0x30) | ((in[1] >> 4) & 0x0f)];
+ buf[2] = kBase64EncodeTable[(in[1] << 2) & 0x3c];
} else { // len == 1
- buf[1] = kBase64EncodeTable[(in[0] << 4) & 0x3f];
+ buf[1] = kBase64EncodeTable[(in[0] << 4) & 0x30];
}
}
diff --git a/lib/cpp/test/Base64Test.cpp b/lib/cpp/test/Base64Test.cpp
new file mode 100644
index 0000000..5caaae8
--- /dev/null
+++ b/lib/cpp/test/Base64Test.cpp
@@ -0,0 +1,71 @@
+/*
+ * 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.
+ */
+
+#include <boost/test/auto_unit_test.hpp>
+#include <thrift/protocol/TBase64Utils.h>
+
+using apache::thrift::protocol::base64_encode;
+using apache::thrift::protocol::base64_decode;
+
+BOOST_AUTO_TEST_SUITE( Base64Test )
+
+void setupTestData(int i, uint8_t* data, int& len) {
+ len = 0;
+ do {
+ data[len] = (uint8_t)(i & 0xFF);
+ i >>= 8;
+ len++;
+ } while ((len < 3) && (i != 0));
+
+ BOOST_ASSERT(i == 0);
+}
+
+void checkEncoding(uint8_t* data, int len) {
+ for (int i = 0; i < len; i++) {
+ BOOST_ASSERT(isalnum(data[i]) || data[i] == '/' || data[i] == '+');
+ }
+}
+
+BOOST_AUTO_TEST_CASE( test_Base64_Encode_Decode ) {
+ int len;
+ uint8_t testInput[3];
+ uint8_t testOutput[4];
+
+ // Test all possible encoding / decoding cases given the
+ // three byte limit for base64_encode.
+
+ for (int i = 0xFFFFFF; i >= 0; i--) {
+
+ // fill testInput based on i
+ setupTestData(i, testInput, len);
+
+ // encode the test data, then decode it again
+ base64_encode(testInput, len, testOutput);
+
+ // verify each byte has a valid Base64 value (alphanumeric or either + or /)
+ checkEncoding(testOutput, len);
+
+ // decode output and check that it matches input
+ base64_decode(testOutput, len + 1);
+ BOOST_ASSERT(0 == memcmp(testInput, testOutput, len));
+
+ }
+}
+
+BOOST_AUTO_TEST_SUITE_END()
diff --git a/lib/cpp/test/Makefile.am b/lib/cpp/test/Makefile.am
index 644d09f..f2ffa45 100755
--- a/lib/cpp/test/Makefile.am
+++ b/lib/cpp/test/Makefile.am
@@ -76,7 +76,8 @@
UnitTests_SOURCES = \
UnitTestMain.cpp \
TMemoryBufferTest.cpp \
- TBufferBaseTest.cpp
+ TBufferBaseTest.cpp \
+ Base64Test.cpp
if !WITH_BOOSTTHREADS
UnitTests_SOURCES += \
diff --git a/lib/csharp/src/Protocol/TBase64Utils.cs b/lib/csharp/src/Protocol/TBase64Utils.cs
index f857d6f..78c3767 100644
--- a/lib/csharp/src/Protocol/TBase64Utils.cs
+++ b/lib/csharp/src/Protocol/TBase64Utils.cs
@@ -34,10 +34,10 @@
{
dst[dstOff + 1] =
(byte)ENCODE_TABLE[
- ((src[srcOff] << 4) + (src[srcOff + 1] >> 4)) & 0x3F];
+ ((src[srcOff] << 4) & 0x30) | ((src[srcOff + 1] >> 4) & 0x0F)];
dst[dstOff + 2] =
(byte)ENCODE_TABLE[
- ((src[srcOff + 1] << 2) + (src[srcOff + 2] >> 6)) & 0x3F];
+ ((src[srcOff + 1] << 2) & 0x3C) | ((src[srcOff + 2] >> 6) & 0x03)];
dst[dstOff + 3] =
(byte)ENCODE_TABLE[src[srcOff + 2] & 0x3F];
}
@@ -45,15 +45,15 @@
{
dst[dstOff + 1] =
(byte)ENCODE_TABLE[
- ((src[srcOff] << 4) + (src[srcOff + 1] >> 4)) & 0x3F];
+ ((src[srcOff] << 4) & 0x30) | ((src[srcOff + 1] >> 4) & 0x0F)];
dst[dstOff + 2] =
- (byte)ENCODE_TABLE[(src[srcOff + 1] << 2) & 0x3F];
+ (byte)ENCODE_TABLE[(src[srcOff + 1] << 2) & 0x3C];
}
else
{ // len == 1) {
dst[dstOff + 1] =
- (byte)ENCODE_TABLE[(src[srcOff] << 4) & 0x3F];
+ (byte)ENCODE_TABLE[(src[srcOff] << 4) & 0x30];
}
}
diff --git a/lib/java/src/org/apache/thrift/protocol/TBase64Utils.java b/lib/java/src/org/apache/thrift/protocol/TBase64Utils.java
index 37a9fd9..abfc965 100644
--- a/lib/java/src/org/apache/thrift/protocol/TBase64Utils.java
+++ b/lib/java/src/org/apache/thrift/protocol/TBase64Utils.java
@@ -56,24 +56,23 @@
if (len == 3) {
dst[dstOff + 1] =
(byte)ENCODE_TABLE.charAt(
- ((src[srcOff] << 4) + (src[srcOff+1] >> 4)) & 0x3F);
+ ((src[srcOff] << 4) & 0x30) | ((src[srcOff+1] >> 4) & 0x0F));
dst[dstOff + 2] =
(byte)ENCODE_TABLE.charAt(
- ((src[srcOff+1] << 2) + (src[srcOff+2] >> 6)) & 0x3F);
+ ((src[srcOff+1] << 2) & 0x3C) | ((src[srcOff+2] >> 6) & 0x03));
dst[dstOff + 3] =
(byte)ENCODE_TABLE.charAt(src[srcOff+2] & 0x3F);
}
else if (len == 2) {
dst[dstOff+1] =
(byte)ENCODE_TABLE.charAt(
- ((src[srcOff] << 4) + (src[srcOff+1] >> 4)) & 0x3F);
+ ((src[srcOff] << 4) & 0x30) | ((src[srcOff+1] >> 4) & 0x0F));
dst[dstOff + 2] =
- (byte)ENCODE_TABLE.charAt((src[srcOff+1] << 2) & 0x3F);
-
+ (byte)ENCODE_TABLE.charAt((src[srcOff+1] << 2) & 0x3C);
}
else { // len == 1) {
dst[dstOff + 1] =
- (byte)ENCODE_TABLE.charAt((src[srcOff] << 4) & 0x3F);
+ (byte)ENCODE_TABLE.charAt((src[srcOff] << 4) & 0x30);
}
}
diff --git a/lib/java/test/org/apache/thrift/protocol/ProtocolTestBase.java b/lib/java/test/org/apache/thrift/protocol/ProtocolTestBase.java
index ef2a392..2ac0211 100644
--- a/lib/java/test/org/apache/thrift/protocol/ProtocolTestBase.java
+++ b/lib/java/test/org/apache/thrift/protocol/ProtocolTestBase.java
@@ -74,7 +74,13 @@
}
public void testBinary() throws Exception {
- for (byte[] b : Arrays.asList(new byte[0], new byte[]{0,1,2,3,4,5,6,7,8,9,10}, new byte[]{0,1,2,3,4,5,6,7,8,9,10,11,12,13,14}, new byte[128])) {
+ for (byte[] b : Arrays.asList(new byte[0],
+ new byte[]{0,1,2,3,4,5,6,7,8,9,10},
+ new byte[]{0,1,2,3,4,5,6,7,8,9,10,11,12,13,14},
+ new byte[]{0x5D},
+ new byte[]{(byte)0xD5,(byte)0x5D},
+ new byte[]{(byte)0xFF,(byte)0xD5,(byte)0x5D},
+ new byte[128])) {
if (canBeUsedNaked()) {
internalTestNakedBinary(b);
}
diff --git a/lib/javame/src/org/apache/thrift/protocol/TBase64Utils.java b/lib/javame/src/org/apache/thrift/protocol/TBase64Utils.java
index 37a9fd9..abfc965 100644
--- a/lib/javame/src/org/apache/thrift/protocol/TBase64Utils.java
+++ b/lib/javame/src/org/apache/thrift/protocol/TBase64Utils.java
@@ -56,24 +56,23 @@
if (len == 3) {
dst[dstOff + 1] =
(byte)ENCODE_TABLE.charAt(
- ((src[srcOff] << 4) + (src[srcOff+1] >> 4)) & 0x3F);
+ ((src[srcOff] << 4) & 0x30) | ((src[srcOff+1] >> 4) & 0x0F));
dst[dstOff + 2] =
(byte)ENCODE_TABLE.charAt(
- ((src[srcOff+1] << 2) + (src[srcOff+2] >> 6)) & 0x3F);
+ ((src[srcOff+1] << 2) & 0x3C) | ((src[srcOff+2] >> 6) & 0x03));
dst[dstOff + 3] =
(byte)ENCODE_TABLE.charAt(src[srcOff+2] & 0x3F);
}
else if (len == 2) {
dst[dstOff+1] =
(byte)ENCODE_TABLE.charAt(
- ((src[srcOff] << 4) + (src[srcOff+1] >> 4)) & 0x3F);
+ ((src[srcOff] << 4) & 0x30) | ((src[srcOff+1] >> 4) & 0x0F));
dst[dstOff + 2] =
- (byte)ENCODE_TABLE.charAt((src[srcOff+1] << 2) & 0x3F);
-
+ (byte)ENCODE_TABLE.charAt((src[srcOff+1] << 2) & 0x3C);
}
else { // len == 1) {
dst[dstOff + 1] =
- (byte)ENCODE_TABLE.charAt((src[srcOff] << 4) & 0x3F);
+ (byte)ENCODE_TABLE.charAt((src[srcOff] << 4) & 0x30);
}
}