From c005b1b65ed24bba18e843c85b9a2a281bfec265 Mon Sep 17 00:00:00 2001 From: David Reiss Date: Fri, 15 Feb 2008 01:38:18 +0000 Subject: [PATCH] Thrift: Distinguish between string and binary types in C++ and Java. Summary: The upcoming TJSONProtocol handles string and binary types quite differently. This change makes that distinction in all parts of the C++ binding. Java already distinguished between string and binary, but this change also updates the Java skip method to skip over strings as binary so we don't get encoding errors when skipping binary data. Reviewed By: mcslee Test Plan: make check Revert Plan: ok Other Notes: I just pulled this out of Chad Walters' JSON patch. The only other change was adding readBinary (or was it writeBinary) to TDenseProtocol. Maybe inheriting from TBinaryProtocol wasn't a good idea. git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@665481 13f79535-47bb-0310-9956-ffa450edef68 --- compiler/cpp/src/generate/t_cpp_generator.cc | 14 ++++++++++++-- lib/cpp/src/protocol/TBinaryProtocol.cpp | 8 ++++++++ lib/cpp/src/protocol/TBinaryProtocol.h | 5 ++++- lib/cpp/src/protocol/TDebugProtocol.cpp | 4 ++++ lib/cpp/src/protocol/TDebugProtocol.h | 2 ++ lib/cpp/src/protocol/TDenseProtocol.cpp | 8 ++++++++ lib/cpp/src/protocol/TDenseProtocol.h | 3 +++ lib/cpp/src/protocol/TOneWayProtocol.h | 4 ++++ lib/cpp/src/protocol/TProtocol.h | 6 +++++- lib/java/src/protocol/TProtocolUtil.java | 2 +- 10 files changed, 51 insertions(+), 5 deletions(-) diff --git a/compiler/cpp/src/generate/t_cpp_generator.cc b/compiler/cpp/src/generate/t_cpp_generator.cc index eb4b1dcf..1fb63b9c 100644 --- a/compiler/cpp/src/generate/t_cpp_generator.cc +++ b/compiler/cpp/src/generate/t_cpp_generator.cc @@ -2185,7 +2185,12 @@ void t_cpp_generator::generate_deserialize_field(ofstream& out, throw "compiler error: cannot serialize void field in a struct: " + name; break; case t_base_type::TYPE_STRING: - out << "readString(" << name << ");"; + if (((t_base_type*)type)->is_binary()) { + out << "readBinary(" << name << ");"; + } + else { + out << "readString(" << name << ");"; + } break; case t_base_type::TYPE_BOOL: out << "readBool(" << name << ");"; @@ -2401,7 +2406,12 @@ void t_cpp_generator::generate_serialize_field(ofstream& out, "compiler error: cannot serialize void field in a struct: " + name; break; case t_base_type::TYPE_STRING: - out << "writeString(" << name << ");"; + if (((t_base_type*)type)->is_binary()) { + out << "writeBinary(" << name << ");"; + } + else { + out << "writeString(" << name << ");"; + } break; case t_base_type::TYPE_BOOL: out << "writeBool(" << name << ");"; diff --git a/lib/cpp/src/protocol/TBinaryProtocol.cpp b/lib/cpp/src/protocol/TBinaryProtocol.cpp index 289d6147..929499b1 100644 --- a/lib/cpp/src/protocol/TBinaryProtocol.cpp +++ b/lib/cpp/src/protocol/TBinaryProtocol.cpp @@ -188,6 +188,10 @@ uint32_t TBinaryProtocol::writeString(const string& str) { return result + size; } +uint32_t TBinaryProtocol::writeBinary(const string& str) { + return TBinaryProtocol::writeString(str); +} + /** * Reading functions */ @@ -379,6 +383,10 @@ uint32_t TBinaryProtocol::readString(string& str) { return result + readStringBody(str, size); } +uint32_t TBinaryProtocol::readBinary(string& str) { + return TBinaryProtocol::readString(str); +} + uint32_t TBinaryProtocol::readStringBody(string& str, int32_t size) { uint32_t result = 0; diff --git a/lib/cpp/src/protocol/TBinaryProtocol.h b/lib/cpp/src/protocol/TBinaryProtocol.h index c7e2791a..92038655 100644 --- a/lib/cpp/src/protocol/TBinaryProtocol.h +++ b/lib/cpp/src/protocol/TBinaryProtocol.h @@ -119,9 +119,10 @@ class TBinaryProtocol : public TProtocol { uint32_t writeDouble(const double dub); - uint32_t writeString(const std::string& str); + uint32_t writeBinary(const std::string& str); + /** * Reading functions */ @@ -173,6 +174,8 @@ class TBinaryProtocol : public TProtocol { uint32_t readString(std::string& str); + uint32_t readBinary(std::string& str); + protected: uint32_t readStringBody(std::string& str, int32_t sz); diff --git a/lib/cpp/src/protocol/TDebugProtocol.cpp b/lib/cpp/src/protocol/TDebugProtocol.cpp index 2f23b112..a24bb5b1 100644 --- a/lib/cpp/src/protocol/TDebugProtocol.cpp +++ b/lib/cpp/src/protocol/TDebugProtocol.cpp @@ -312,4 +312,8 @@ uint32_t TDebugProtocol::writeString(const string& str) { return writeItem(output); } +uint32_t TDebugProtocol::writeBinary(const string& str) { + return TDebugProtocol::writeString(str); +} + }}} // facebook::thrift::protocol diff --git a/lib/cpp/src/protocol/TDebugProtocol.h b/lib/cpp/src/protocol/TDebugProtocol.h index 90b7688e..196cff20 100644 --- a/lib/cpp/src/protocol/TDebugProtocol.h +++ b/lib/cpp/src/protocol/TDebugProtocol.h @@ -104,6 +104,8 @@ class TDebugProtocol : public TWriteOnlyProtocol { uint32_t writeString(const std::string& str); + uint32_t writeBinary(const std::string& str); + private: void indentUp(); diff --git a/lib/cpp/src/protocol/TDenseProtocol.cpp b/lib/cpp/src/protocol/TDenseProtocol.cpp index 55f39028..dbb2d4a4 100644 --- a/lib/cpp/src/protocol/TDenseProtocol.cpp +++ b/lib/cpp/src/protocol/TDenseProtocol.cpp @@ -439,6 +439,10 @@ uint32_t TDenseProtocol::writeString(const std::string& str) { return subWriteString(str); } +uint32_t TDenseProtocol::writeBinary(const std::string& str) { + return TDenseProtocol::writeString(str); +} + inline uint32_t TDenseProtocol::subWriteI32(const int32_t i32) { return vlqWrite(i32); } @@ -718,6 +722,10 @@ uint32_t TDenseProtocol::readString(std::string& str) { return subReadString(str); } +uint32_t TDenseProtocol::readBinary(std::string& str) { + return TDenseProtocol::readString(str); +} + uint32_t TDenseProtocol::subReadI32(int32_t& i32) { uint64_t u64; uint32_t rv = vlqRead(u64); diff --git a/lib/cpp/src/protocol/TDenseProtocol.h b/lib/cpp/src/protocol/TDenseProtocol.h index dae2c18b..713b57a7 100644 --- a/lib/cpp/src/protocol/TDenseProtocol.h +++ b/lib/cpp/src/protocol/TDenseProtocol.h @@ -126,6 +126,8 @@ class TDenseProtocol : public TBinaryProtocol { virtual uint32_t writeString(const std::string& str); + virtual uint32_t writeBinary(const std::string& str); + /* * Helper writing functions (don't do state transitions). @@ -189,6 +191,7 @@ class TDenseProtocol : public TBinaryProtocol { uint32_t readString(std::string& str); + uint32_t readBinary(std::string& str); /* * Helper reading functions (don't do state transitions). diff --git a/lib/cpp/src/protocol/TOneWayProtocol.h b/lib/cpp/src/protocol/TOneWayProtocol.h index 245f8305..17654130 100644 --- a/lib/cpp/src/protocol/TOneWayProtocol.h +++ b/lib/cpp/src/protocol/TOneWayProtocol.h @@ -137,6 +137,10 @@ class TWriteOnlyProtocol : public TProtocol { subclass_ + " does not support reading (yet)."); } + uint32_t readBinary(std::string& str) { + throw TProtocolException(TProtocolException::NOT_IMPLEMENTED, + subclass_ + " does not support reading (yet)."); + } private: std::string subclass_; diff --git a/lib/cpp/src/protocol/TProtocol.h b/lib/cpp/src/protocol/TProtocol.h index 85cc48a9..aecc4e72 100644 --- a/lib/cpp/src/protocol/TProtocol.h +++ b/lib/cpp/src/protocol/TProtocol.h @@ -159,6 +159,8 @@ class TProtocol { virtual uint32_t writeString(const std::string& str) = 0; + virtual uint32_t writeBinary(const std::string& str) = 0; + /** * Reading functions */ @@ -209,6 +211,8 @@ class TProtocol { virtual uint32_t readString(std::string& str) = 0; + virtual uint32_t readBinary(std::string& str) = 0; + /** * Method to arbitrarily skip over data. */ @@ -247,7 +251,7 @@ class TProtocol { case T_STRING: { std::string str; - return readString(str); + return readBinary(str); } case T_STRUCT: { diff --git a/lib/java/src/protocol/TProtocolUtil.java b/lib/java/src/protocol/TProtocolUtil.java index 0b85d38b..5c665ab0 100644 --- a/lib/java/src/protocol/TProtocolUtil.java +++ b/lib/java/src/protocol/TProtocolUtil.java @@ -52,7 +52,7 @@ public class TProtocolUtil { } case TType.STRING: { - prot.readString(); + prot.readBinary(); break; } case TType.STRUCT: -- 2.17.1