THRIFT-1562 Fix issue with TFramedTransport::readSlow
Patch: Dave Watson
git-svn-id: https://svn.apache.org/repos/asf/thrift/trunk@1325034 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/lib/cpp/src/transport/TBufferTransports.cpp b/lib/cpp/src/transport/TBufferTransports.cpp
index 197a7ab..b8a7ec3 100644
--- a/lib/cpp/src/transport/TBufferTransports.cpp
+++ b/lib/cpp/src/transport/TBufferTransports.cpp
@@ -34,7 +34,7 @@
// with the data already in the buffer.
assert(have < len);
- // If we have some date in the buffer, copy it out and return it.
+ // If we have some data in the buffer, copy it out and return it.
// We have to return it without attempting to read more, since we aren't
// guaranteed that the underlying transport actually has more data, so
// attempting to read from it could block.
@@ -140,11 +140,14 @@
// with the data already in the buffer.
assert(have < want);
- // Copy out whatever we have.
+ // If we have some data in the buffer, copy it out and return it.
+ // We have to return it without attempting to read more, since we aren't
+ // guaranteed that the underlying transport actually has more data, so
+ // attempting to read from it could block.
if (have > 0) {
memcpy(buf, rBase_, have);
- want -= have;
- buf += have;
+ setReadBuffer(rBuf_.get(), 0);
+ return have;
}
// Read another frame.
diff --git a/lib/cpp/src/transport/TZlibTransport.h b/lib/cpp/src/transport/TZlibTransport.h
index 6bbff31..db76523 100644
--- a/lib/cpp/src/transport/TZlibTransport.h
+++ b/lib/cpp/src/transport/TZlibTransport.h
@@ -252,6 +252,24 @@
struct z_stream_s* wstream_;
};
+
+/**
+ * Wraps a transport into a zlibbed one.
+ *
+ */
+class TZlibTransportFactory : public TTransportFactory {
+ public:
+ TZlibTransportFactory() {}
+
+ virtual ~TZlibTransportFactory() {}
+
+ virtual boost::shared_ptr<TTransport> getTransport(
+ boost::shared_ptr<TTransport> trans) {
+ return boost::shared_ptr<TTransport>(new TZlibTransport(trans));
+ }
+};
+
+
}}} // apache::thrift::transport
#endif // #ifndef _THRIFT_TRANSPORT_TZLIBTRANSPORT_H_
diff --git a/lib/cpp/test/TransportTest.cpp b/lib/cpp/test/TransportTest.cpp
index 767e563..bc0529d 100644
--- a/lib/cpp/test/TransportTest.cpp
+++ b/lib/cpp/test/TransportTest.cpp
@@ -561,6 +561,7 @@
BOOST_CHECK_EQUAL(memcmp(rbuf.get(), wbuf.get(), totalSize), 0);
}
+
template <class CoupledTransports>
void test_read_part_available() {
CoupledTransports transports;
@@ -584,6 +585,33 @@
}
template <class CoupledTransports>
+void test_read_part_available_in_chunks() {
+ CoupledTransports transports;
+ BOOST_REQUIRE(transports.in != NULL);
+ BOOST_REQUIRE(transports.out != NULL);
+
+ uint8_t write_buf[16];
+ uint8_t read_buf[16];
+ memset(write_buf, 'a', sizeof(write_buf));
+
+ // Write 10 bytes (in a single frame, for transports that use framing)
+ transports.out->write(write_buf, 10);
+ transports.out->flush();
+
+ // Read 1 byte, to force the transport to read the frame
+ uint32_t bytes_read = transports.in->read(read_buf, 1);
+ BOOST_CHECK_EQUAL(bytes_read, 1);
+
+ // Read more than what is remaining and verify the transport does not block
+ set_trigger(3, transports.out, 1);
+ bytes_read = transports.in->read(read_buf, 10);
+ BOOST_CHECK_EQUAL(numTriggersFired, 0);
+ BOOST_CHECK_EQUAL(bytes_read, 9);
+
+ clear_triggers();
+}
+
+template <class CoupledTransports>
void test_read_partial_midframe() {
CoupledTransports transports;
BOOST_REQUIRE(transports.in != NULL);
@@ -912,6 +940,12 @@
test_read_part_available<CoupledTransports>, name);
suite_->add(tc, expectedFailures);
+ snprintf(name, sizeof(name), "%s::test_read_part_available_in_chunks()",
+ transportName);
+ tc = boost::unit_test::make_test_case(
+ test_read_part_available_in_chunks<CoupledTransports>, name);
+ suite_->add(tc, expectedFailures);
+
snprintf(name, sizeof(name), "%s::test_read_partial_midframe()",
transportName);
tc = boost::unit_test::make_test_case(