[
https://issues.apache.org/jira/browse/THRIFT-4626?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16601177#comment-16601177
]
ASF GitHub Bot commented on THRIFT-4626:
----------------------------------------
jeking3 closed pull request #1589: THRIFT-4626: Remove checking of remaining
bytes in the Go library.
URL: https://github.com/apache/thrift/pull/1589
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/lib/go/thrift/binary_protocol.go b/lib/go/thrift/binary_protocol.go
index de0f6a7a5c..1f90bf4351 100644
--- a/lib/go/thrift/binary_protocol.go
+++ b/lib/go/thrift/binary_protocol.go
@@ -448,9 +448,6 @@ func (p *TBinaryProtocol) ReadBinary() ([]byte, error) {
if size < 0 {
return nil, invalidDataLength
}
- if uint64(size) > p.trans.RemainingBytes() {
- return nil, invalidDataLength
- }
isize := int(size)
buf := make([]byte, isize)
@@ -481,9 +478,6 @@ func (p *TBinaryProtocol) readStringBody(size int32) (value
string, err error) {
if size < 0 {
return "", nil
}
- if uint64(size) > p.trans.RemainingBytes() {
- return "", invalidDataLength
- }
var (
buf bytes.Buffer
diff --git a/lib/go/thrift/compact_protocol.go
b/lib/go/thrift/compact_protocol.go
index 66fbf5c335..1900d50c3b 100644
--- a/lib/go/thrift/compact_protocol.go
+++ b/lib/go/thrift/compact_protocol.go
@@ -562,9 +562,6 @@ func (p *TCompactProtocol) ReadString() (value string, err
error) {
if length < 0 {
return "", invalidDataLength
}
- if uint64(length) > p.trans.RemainingBytes() {
- return "", invalidDataLength
- }
if length == 0 {
return "", nil
@@ -591,9 +588,6 @@ func (p *TCompactProtocol) ReadBinary() (value []byte, err
error) {
if length < 0 {
return nil, invalidDataLength
}
- if uint64(length) > p.trans.RemainingBytes() {
- return nil, invalidDataLength
- }
buf := make([]byte, length)
_, e = io.ReadFull(p.trans, buf)
diff --git a/lib/go/thrift/compact_protocol_test.go
b/lib/go/thrift/compact_protocol_test.go
index f940b4e15a..65f77f2c42 100644
--- a/lib/go/thrift/compact_protocol_test.go
+++ b/lib/go/thrift/compact_protocol_test.go
@@ -26,11 +26,18 @@ import (
func TestReadWriteCompactProtocol(t *testing.T) {
ReadWriteProtocolTest(t, NewTCompactProtocolFactory())
+
transports := []TTransport{
NewTMemoryBuffer(),
NewStreamTransportRW(bytes.NewBuffer(make([]byte, 0, 16384))),
NewTFramedTransport(NewTMemoryBuffer()),
}
+
+ zlib0, _ := NewTZlibTransport(NewTMemoryBuffer(), 0)
+ zlib6, _ := NewTZlibTransport(NewTMemoryBuffer(), 6)
+ zlib9, _ := NewTZlibTransport(NewTFramedTransport(NewTMemoryBuffer()),
9)
+ transports = append(transports, zlib0, zlib6, zlib9)
+
for _, trans := range transports {
p := NewTCompactProtocol(trans)
ReadWriteBool(t, p, trans)
diff --git a/lib/go/thrift/protocol_test.go b/lib/go/thrift/protocol_test.go
index e9118da88f..944055c0b4 100644
--- a/lib/go/thrift/protocol_test.go
+++ b/lib/go/thrift/protocol_test.go
@@ -32,7 +32,6 @@ import (
const PROTOCOL_BINARY_DATA_SIZE = 155
var (
- data string // test data for writing
protocol_bdata []byte // test data for writing; same as data
BOOL_VALUES []bool
BYTE_VALUES []int8
@@ -48,7 +47,6 @@ func init() {
for i := 0; i < PROTOCOL_BINARY_DATA_SIZE; i++ {
protocol_bdata[i] = byte((i + 'a') % 255)
}
- data = string(protocol_bdata)
BOOL_VALUES = []bool{false, true, false, false, true}
BYTE_VALUES = []int8{117, 0, 1, 32, 127, -128, -1}
INT16_VALUES = []int16{459, 0, 1, -1, -128, 127, 32767, -32768}
@@ -121,6 +119,9 @@ func ReadWriteProtocolTest(t *testing.T, protocolFactory
TProtocolFactory) {
NewTMemoryBufferTransportFactory(1024),
NewStreamTransportFactory(buf, buf, true),
NewTFramedTransportFactory(NewTMemoryBufferTransportFactory(1024)),
+ NewTZlibTransportFactoryWithFactory(0,
NewTMemoryBufferTransportFactory(1024)),
+ NewTZlibTransportFactoryWithFactory(6,
NewTMemoryBufferTransportFactory(1024)),
+ NewTZlibTransportFactoryWithFactory(9,
NewTFramedTransportFactory(NewTMemoryBufferTransportFactory(1024))),
NewTHttpPostClientTransportFactory("http://"; + addr.String()),
}
for _, tf := range transports {
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> Communication crash when using binary/compact protocol and zlib transport
> -------------------------------------------------------------------------
>
> Key: THRIFT-4626
> URL: https://issues.apache.org/jira/browse/THRIFT-4626
> Project: Thrift
> Issue Type: Bug
> Components: Go - Library
> Affects Versions: 0.11.0
> Environment: Ubuntu 18.04
> Go 1.10.1
> Reporter: Vyacheslav Kulakov
> Priority: Major
> Fix For: 0.12.0
>
>
> Reading a request on the server side or reading a response on the client side
> always fail with the "Invalid data length" error when using the
> binary/compact protocol and the zlib transport, which wraps the framed
> transport.
> In my project, I use the following code on the server side (only for testing):
> {code:go}
> processor := flume.NewThriftSourceProtocolProcessor(protocol)
> serverTransport, _ := thrift.NewTServerSocketTimeout(address, timeout)
> protocolFactory := thrift.NewTBinaryProtocolFactoryDefault()
> transportFactory :=
> thrift.NewTFramedTransportFactory(thrift.NewTTransportFactory())
> transportFactory = thrift.NewTZlibTransportFactoryWithFactory(level,
> transportFactory)
> server := thrift.NewTSimpleServer4(
> processor,
> serverTransport,
> transportFactory,
> protocolFactory,
> )
> {code}
> and following code on the client side:
> {code:go}
> factory := thrift.NewTBinaryProtocolFactoryDefault()
> transport := thrift.TTransport(thrift.NewTFramedTransport(socket))
> transport, err = thrift.NewTZlibTransport(transport, compressionLevel)
> if err != nil {
> return err
> }
> err = transport.Open()
> if err != nil {
> return err
> }
> client := flume.NewThriftSourceProtocolClient(
> thrift.NewTStandardClient(
> factory.GetProtocol(transport),
> factory.GetProtocol(transport),
> ),
> )
> {code}
> When I send data from the client to the server I always get the "EOF" error
> on the client and the "Invalid data length" error on the server. If I use the
> compact protocol the errors stay at their places.
> I investigated Go library code and found a reason of that errors: the
> protocol invoke the RemainingBytes method on the zlib transport and it always
> return zero because all bytes were already read from a frame and were stored
> in a decompressor buffer. But we can't access to this buffer to obtain
> correct number of remaining bytes. So this combination of protocols and
> transports can't work together at all.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
