You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by ro...@apache.org on 2015/10/11 01:08:02 UTC

thrift git commit: THRIFT-3276 Binary data does not decode correctly using the TJSONProtocol when the base64 encoded data is padded.

Repository: thrift
Updated Branches:
  refs/heads/master 5d93b04f9 -> a175437f6


THRIFT-3276 Binary data does not decode correctly using the TJSONProtocol when the base64 encoded data is padded.

This closes #645


Project: http://git-wip-us.apache.org/repos/asf/thrift/repo
Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/a175437f
Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/a175437f
Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/a175437f

Branch: refs/heads/master
Commit: a175437f66fa1a0b36233e7dd40b061d471276ff
Parents: 5d93b04
Author: Nobuaki Sukegawa <ns...@gmail.com>
Authored: Sat Oct 10 10:44:07 2015 +0900
Committer: Roger Meier <ro...@apache.org>
Committed: Sun Oct 11 00:55:58 2015 +0200

----------------------------------------------------------------------
 lib/cpp/src/thrift/protocol/TJSONProtocol.cpp    |  5 +++++
 lib/cpp/src/thrift/protocol/TJSONProtocol.h      |  4 ++++
 lib/csharp/src/Protocol/TJSONProtocol.cs         |  5 +++++
 .../apache/thrift/protocol/TJSONProtocol.java    |  5 +++++
 .../apache/thrift/protocol/TJSONProtocol.java    |  5 +++++
 test/known_failures_Linux.json                   | 19 -------------------
 6 files changed, 24 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/thrift/blob/a175437f/lib/cpp/src/thrift/protocol/TJSONProtocol.cpp
----------------------------------------------------------------------
diff --git a/lib/cpp/src/thrift/protocol/TJSONProtocol.cpp b/lib/cpp/src/thrift/protocol/TJSONProtocol.cpp
index e4077bc..6865fdc 100644
--- a/lib/cpp/src/thrift/protocol/TJSONProtocol.cpp
+++ b/lib/cpp/src/thrift/protocol/TJSONProtocol.cpp
@@ -761,6 +761,11 @@ uint32_t TJSONProtocol::readJSONBase64(std::string& str) {
     throw TProtocolException(TProtocolException::SIZE_LIMIT);
   uint32_t len = static_cast<uint32_t>(tmp.length());
   str.clear();
+  // Ignore padding
+  uint32_t bound = len >= 2 ? len - 2 : 0;
+  for (uint32_t i = len - 1; i >= bound && b[i] == '='; --i) {
+    --len;
+  }
   while (len >= 4) {
     base64_decode(b, 4);
     str.append((const char*)b, 3);

http://git-wip-us.apache.org/repos/asf/thrift/blob/a175437f/lib/cpp/src/thrift/protocol/TJSONProtocol.h
----------------------------------------------------------------------
diff --git a/lib/cpp/src/thrift/protocol/TJSONProtocol.h b/lib/cpp/src/thrift/protocol/TJSONProtocol.h
index 3c7b9d7..80d68a4 100644
--- a/lib/cpp/src/thrift/protocol/TJSONProtocol.h
+++ b/lib/cpp/src/thrift/protocol/TJSONProtocol.h
@@ -53,6 +53,10 @@ class TJSONContext;
  *    The readBinary() method is written such that it will properly skip if
  *    called on a Thrift string (although it will decode garbage data).
  *
+ *    NOTE: Base64 padding is optional for Thrift binary value encoding. So
+ *    the readBinary() method needs to decode both input strings with padding
+ *    and those without one.
+ *
  * 5. Thrift structs are represented as JSON objects, with the field ID as the
  *    key, and the field value represented as a JSON object with a single
  *    key-value pair. The key is a short string identifier for that type,

http://git-wip-us.apache.org/repos/asf/thrift/blob/a175437f/lib/csharp/src/Protocol/TJSONProtocol.cs
----------------------------------------------------------------------
diff --git a/lib/csharp/src/Protocol/TJSONProtocol.cs b/lib/csharp/src/Protocol/TJSONProtocol.cs
index 2e8d99f..4081d6e 100644
--- a/lib/csharp/src/Protocol/TJSONProtocol.cs
+++ b/lib/csharp/src/Protocol/TJSONProtocol.cs
@@ -544,6 +544,11 @@ namespace Thrift.Protocol
             int len = b.Length;
             int off = 0;
 
+            // Ignore padding
+            int bound = len >= 2 ? len - 2 : 0;
+            for (int i = len - 1; i >= bound && b[i] == '='; --i) {
+                --len;
+            }
             while (len >= 3)
             {
                 // Encode 3 bytes at a time

http://git-wip-us.apache.org/repos/asf/thrift/blob/a175437f/lib/java/src/org/apache/thrift/protocol/TJSONProtocol.java
----------------------------------------------------------------------
diff --git a/lib/java/src/org/apache/thrift/protocol/TJSONProtocol.java b/lib/java/src/org/apache/thrift/protocol/TJSONProtocol.java
index f51322c..9876e13 100644
--- a/lib/java/src/org/apache/thrift/protocol/TJSONProtocol.java
+++ b/lib/java/src/org/apache/thrift/protocol/TJSONProtocol.java
@@ -769,6 +769,11 @@ public class TJSONProtocol extends TProtocol {
     int len = arr.len();
     int off = 0;
     int size = 0;
+    // Ignore padding
+    int bound = len >= 2 ? len - 2 : 0;
+    for (int i = len - 1; i >= bound && b[i] == '='; --i) {
+      --len;
+    }
     while (len >= 4) {
       // Decode 4 bytes at a time
       TBase64Utils.decode(b, off, 4, b, size); // NB: decoded in place

http://git-wip-us.apache.org/repos/asf/thrift/blob/a175437f/lib/javame/src/org/apache/thrift/protocol/TJSONProtocol.java
----------------------------------------------------------------------
diff --git a/lib/javame/src/org/apache/thrift/protocol/TJSONProtocol.java b/lib/javame/src/org/apache/thrift/protocol/TJSONProtocol.java
index de7086f..99e3d2a 100644
--- a/lib/javame/src/org/apache/thrift/protocol/TJSONProtocol.java
+++ b/lib/javame/src/org/apache/thrift/protocol/TJSONProtocol.java
@@ -722,6 +722,11 @@ public class TJSONProtocol extends TProtocol {
     int len = arr.len();
     int off = 0;
     int size = 0;
+    // Ignore padding
+    int bound = len >= 2 ? len - 2 : 0;
+    for (int i = len - 1; i >= bound && b[i] == '='; --i) {
+      --len;
+    }
     while (len >= 4) {
       // Decode 4 bytes at a time
       TBase64Utils.decode(b, off, 4, b, size); // NB: decoded in place

http://git-wip-us.apache.org/repos/asf/thrift/blob/a175437f/test/known_failures_Linux.json
----------------------------------------------------------------------
diff --git a/test/known_failures_Linux.json b/test/known_failures_Linux.json
index cf17cae..dbc97cc 100644
--- a/test/known_failures_Linux.json
+++ b/test/known_failures_Linux.json
@@ -22,7 +22,6 @@
   "cpp-java_json_http-ip",
   "cpp-java_json_http-ip-ssl",
   "cpp-rb_json_buffered-ip",
-  "cpp-rb_json_framed-ip",
   "csharp-cpp_binary_buffered-ip-ssl",
   "csharp-cpp_binary_framed-ip-ssl",
   "csharp-cpp_compact_buffered-ip-ssl",
@@ -45,20 +44,10 @@
   "csharp-nodejs_json_buffered-ip-ssl",
   "csharp-nodejs_json_framed-ip",
   "csharp-nodejs_json_framed-ip-ssl",
-  "go-cpp_json_buffered-ip",
-  "go-cpp_json_buffered-ip-ssl",
-  "go-cpp_json_framed-ip",
-  "go-cpp_json_framed-ip-ssl",
   "go-dart_binary_framed-ip",
   "go-dart_json_framed-ip",
   "go-hs_json_buffered-ip",
   "go-hs_json_framed-ip",
-  "go-java_json_buffered-ip",
-  "go-java_json_buffered-ip-ssl",
-  "go-java_json_framed-fastframed-ip",
-  "go-java_json_framed-fastframed-ip-ssl",
-  "go-java_json_framed-ip",
-  "go-java_json_framed-ip-ssl",
   "go-perl_binary_buffered-ip-ssl",
   "go-perl_binary_framed-ip-ssl",
   "hs-cpp_json_buffered-ip",
@@ -86,9 +75,6 @@
   "java-hs_json_framed-ip",
   "java-nodejs_json_buffered-ip",
   "java-nodejs_json_buffered-ip-ssl",
-  "java-rb_json_buffered-ip",
-  "java-rb_json_fastframed-framed-ip",
-  "java-rb_json_framed-ip",
   "nodejs-cpp_compact_buffered-ip",
   "nodejs-cpp_compact_buffered-ip-ssl",
   "nodejs-cpp_compact_framed-ip",
@@ -131,13 +117,8 @@
   "py-nodejs_json_buffered-ip-ssl",
   "py-nodejs_json_framed-ip",
   "py-nodejs_json_framed-ip-ssl",
-  "rb-cpp_json_buffered-ip",
-  "rb-cpp_json_framed-ip",
   "rb-hs_json_buffered-ip",
   "rb-hs_json_framed-ip",
-  "rb-java_json_buffered-ip",
-  "rb-java_json_framed-fastframed-ip",
-  "rb-java_json_framed-ip",
   "rb-nodejs_json_buffered-ip",
   "rb-nodejs_json_framed-ip"
 ]
\ No newline at end of file