THRIFT-3568 THeader server crashes on readSlow
Client: C++
Patch: Nobuaki Sukegawa
This closes #807
diff --git a/lib/cpp/src/thrift/transport/TBufferTransports.h b/lib/cpp/src/thrift/transport/TBufferTransports.h
index 013c6e0..e690d0c 100644
--- a/lib/cpp/src/thrift/transport/TBufferTransports.h
+++ b/lib/cpp/src/thrift/transport/TBufferTransports.h
@@ -372,7 +372,7 @@
* TVirtualTransport provides a default implementation of readAll().
* We want to use the TBufferBase version instead.
*/
- uint32_t readAll(uint8_t* buf, uint32_t len) { return TBufferBase::readAll(buf, len); }
+ using TBufferBase::readAll;
/**
* Returns the origin of the underlying transport
@@ -396,7 +396,7 @@
* Returns true if a frame was read successfully, or false on EOF.
* (Raises a TTransportException if EOF occurs after a partial frame.)
*/
- bool readFrame();
+ virtual bool readFrame();
void initPointers() {
setReadBuffer(NULL, 0);
diff --git a/lib/cpp/src/thrift/transport/THeaderTransport.cpp b/lib/cpp/src/thrift/transport/THeaderTransport.cpp
index 79bc5ea..fd24fed 100644
--- a/lib/cpp/src/thrift/transport/THeaderTransport.cpp
+++ b/lib/cpp/src/thrift/transport/THeaderTransport.cpp
@@ -40,14 +40,7 @@
using namespace apache::thrift::protocol;
using apache::thrift::protocol::TBinaryProtocol;
-uint32_t THeaderTransport::readAll(uint8_t* buf, uint32_t len) {
- // We want to call TBufferBase's version here, because
- // TFramedTransport would try and call its own readFrame function
- return TBufferBase::readAll(buf, len);
-}
-
uint32_t THeaderTransport::readSlow(uint8_t* buf, uint32_t len) {
-
if (clientType == THRIFT_UNFRAMED_DEPRECATED) {
return transport_->read(buf, len);
}
@@ -70,7 +63,7 @@
}
}
-bool THeaderTransport::readFrame(uint32_t minFrameSize) {
+bool THeaderTransport::readFrame() {
// szN is network byte order of sz
uint32_t szN;
uint32_t sz;
@@ -99,6 +92,7 @@
sz = ntohl(szN);
+ uint32_t minFrameSize = 0;
ensureReadBuffer(minFrameSize + 4);
if ((sz & TBinaryProtocol::VERSION_MASK) == (uint32_t)TBinaryProtocol::VERSION_1) {
@@ -368,7 +362,7 @@
clientType = THRIFT_HEADER_CLIENT_TYPE;
// Read the header and decide which protocol to go with
- readFrame(0);
+ readFrame();
}
uint32_t THeaderTransport::getWriteBytes() {
diff --git a/lib/cpp/src/thrift/transport/THeaderTransport.h b/lib/cpp/src/thrift/transport/THeaderTransport.h
index 94135ea..a125632 100644
--- a/lib/cpp/src/thrift/transport/THeaderTransport.h
+++ b/lib/cpp/src/thrift/transport/THeaderTransport.h
@@ -22,6 +22,7 @@
#include <bitset>
#include <vector>
+#include <stdexcept>
#include <string>
#include <map>
@@ -69,7 +70,7 @@
/// Use default buffer sizes.
explicit THeaderTransport(const boost::shared_ptr<TTransport>& transport)
- : transport_(transport),
+ : TVirtualTransport(transport),
outTransport_(transport),
protoId(T_COMPACT_PROTOCOL),
clientType(THRIFT_HEADER_CLIENT_TYPE),
@@ -77,12 +78,13 @@
flags(0),
tBufSize_(0),
tBuf_(NULL) {
+ if (!transport_) throw std::invalid_argument("transport is empty");
initBuffers();
}
THeaderTransport(const boost::shared_ptr<TTransport> inTransport,
const boost::shared_ptr<TTransport> outTransport)
- : transport_(inTransport),
+ : TVirtualTransport(inTransport),
outTransport_(outTransport),
protoId(T_COMPACT_PROTOCOL),
clientType(THRIFT_HEADER_CLIENT_TYPE),
@@ -90,34 +92,16 @@
flags(0),
tBufSize_(0),
tBuf_(NULL) {
+ if (!transport_) throw std::invalid_argument("inTransport is empty");
+ if (!outTransport_) throw std::invalid_argument("outTransport is empty");
initBuffers();
}
- void open() { transport_->open(); }
-
- bool isOpen() { return transport_->isOpen(); }
-
- bool peek() { return (this->rBase_ < this->rBound_) || transport_->peek(); }
-
- void close() {
- flush();
- transport_->close();
- }
-
virtual uint32_t readSlow(uint8_t* buf, uint32_t len);
- virtual uint32_t readAll(uint8_t* buf, uint32_t len);
virtual void flush();
void resizeTransformBuffer(uint32_t additionalSize = 0);
- boost::shared_ptr<TTransport> getUnderlyingTransport() { return transport_; }
-
- /*
- * TVirtualTransport provides a default implementation of readAll().
- * We want to use the TBufferBase version instead.
- */
- using TBufferBase::readAll;
-
uint16_t getProtocolId() const;
void setProtocolId(uint16_t protoId) { this->protoId = protoId; }
@@ -191,7 +175,7 @@
* Returns true if a frame was read successfully, or false on EOF.
* (Raises a TTransportException if EOF occurs after a partial frame.)
*/
- bool readFrame(uint32_t minFrameSize);
+ virtual bool readFrame();
void ensureReadBuffer(uint32_t sz);
uint32_t getWriteBytes();
@@ -201,7 +185,6 @@
setWriteBuffer(wBuf_.get(), wBufSize_);
}
- boost::shared_ptr<TTransport> transport_;
boost::shared_ptr<TTransport> outTransport_;
// 0 and 16th bits must be 0 to differentiate from framed & unframed
diff --git a/test/features/known_failures_Linux.json b/test/features/known_failures_Linux.json
index fd96250..9bf600d 100644
--- a/test/features/known_failures_Linux.json
+++ b/test/features/known_failures_Linux.json
@@ -1,8 +1,6 @@
[
"c_glib-limit_container_length_binary_buffered-ip",
"c_glib-limit_string_length_binary_buffered-ip",
- "cpp-theader_framed_binary_header_buffered-ip",
- "cpp-theader_unframed_binary_header_buffered-ip",
"csharp-limit_container_length_binary_buffered-ip",
"csharp-limit_container_length_compact_buffered-ip",
"csharp-limit_string_length_binary_buffered-ip",
diff --git a/test/known_failures_Linux.json b/test/known_failures_Linux.json
index 25eea2d..ac409bf 100644
--- a/test/known_failures_Linux.json
+++ b/test/known_failures_Linux.json
@@ -9,14 +9,8 @@
"cpp-cpp_compact_http-domain",
"cpp-cpp_compact_http-ip",
"cpp-cpp_compact_http-ip-ssl",
- "cpp-cpp_header_buffered-domain",
- "cpp-cpp_header_buffered-ip",
"cpp-cpp_header_buffered-ip-ssl",
- "cpp-cpp_header_framed-domain",
- "cpp-cpp_header_framed-ip",
"cpp-cpp_header_framed-ip-ssl",
- "cpp-cpp_header_http-domain",
- "cpp-cpp_header_http-ip",
"cpp-cpp_header_http-ip-ssl",
"cpp-cpp_json_buffered-ip-ssl",
"cpp-cpp_json_framed-ip",
@@ -60,6 +54,12 @@
"csharp-nodejs_json_framed-ip-ssl",
"csharp-perl_binary_buffered-ip-ssl",
"csharp-perl_binary_framed-ip-ssl",
+ "csharp-py3_binary_buffered-ip-ssl",
+ "csharp-py3_binary_framed-ip-ssl",
+ "csharp-py3_compact_buffered-ip-ssl",
+ "csharp-py3_compact_framed-ip-ssl",
+ "csharp-py3_json_buffered-ip-ssl",
+ "csharp-py3_json_framed-ip-ssl",
"csharp-py_binary-accel_buffered-ip-ssl",
"csharp-py_binary-accel_framed-ip-ssl",
"csharp-py_binary_buffered-ip-ssl",
@@ -68,12 +68,6 @@
"csharp-py_compact_framed-ip-ssl",
"csharp-py_json_buffered-ip-ssl",
"csharp-py_json_framed-ip-ssl",
- "csharp-py3_binary_buffered-ip-ssl",
- "csharp-py3_binary_framed-ip-ssl",
- "csharp-py3_compact_buffered-ip-ssl",
- "csharp-py3_compact_framed-ip-ssl",
- "csharp-py3_json_buffered-ip-ssl",
- "csharp-py3_json_framed-ip-ssl",
"erl-cpp_compact_buffered-ip",
"erl-cpp_compact_buffered-ip-ssl",
"erl-cpp_compact_framed-ip",