From 4cb8e40d8abbb480065461ac3fd27280cb9a53f6 Mon Sep 17 00:00:00 2001 From: Roger Meier Date: Sun, 27 May 2012 18:05:16 +0000 Subject: [PATCH] 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 --- lib/cpp/src/thrift/protocol/TBase64Utils.cpp | 12 ++-- lib/cpp/test/Base64Test.cpp | 71 +++++++++++++++++++ lib/cpp/test/Makefile.am | 3 +- lib/csharp/src/Protocol/TBase64Utils.cs | 10 +-- .../apache/thrift/protocol/TBase64Utils.java | 11 ++- .../thrift/protocol/ProtocolTestBase.java | 8 ++- .../apache/thrift/protocol/TBase64Utils.java | 11 ++- 7 files changed, 101 insertions(+), 25 deletions(-) create mode 100644 lib/cpp/test/Base64Test.cpp diff --git a/lib/cpp/src/thrift/protocol/TBase64Utils.cpp b/lib/cpp/src/thrift/protocol/TBase64Utils.cpp index 12ebaa93..452985cb 100644 --- a/lib/cpp/src/thrift/protocol/TBase64Utils.cpp +++ b/lib/cpp/src/thrift/protocol/TBase64Utils.cpp @@ -30,16 +30,16 @@ static const uint8_t *kBase64EncodeTable = (const uint8_t *) "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 00000000..5caaae87 --- /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 +#include + +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 644d09f4..f2ffa455 100755 --- a/lib/cpp/test/Makefile.am +++ b/lib/cpp/test/Makefile.am @@ -76,7 +76,8 @@ TESTS = \ 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 f857d6fc..78c37674 100644 --- a/lib/csharp/src/Protocol/TBase64Utils.cs +++ b/lib/csharp/src/Protocol/TBase64Utils.cs @@ -34,10 +34,10 @@ namespace Thrift.Protocol { 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 @@ namespace Thrift.Protocol { 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 37a9fd9f..abfc965b 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 @@ class TBase64Utils { 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 ef2a3927..2ac02113 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 abstract class ProtocolTestBase extends TestCase { } 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 37a9fd9f..abfc965b 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 @@ class TBase64Utils { 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); } } -- 2.17.1