THRIFT-2500 sending random data crashes thrift(golang) service
Client: Go
Patch: Aleksey Pesternikov
This closes #117
commit 1bb25c4a48845e112847ca8293402f0294d8f597
Author: Aleksey Pesternikov <ap@alekseys-mbp.att.net>
Date: 2014-05-02T21:40:59Z
recover from panic in processor
commit 8d1427a2c3c183d499442dc1f0437292e6641ac3
Author: Aleksey Pesternikov <ap@alekseys-mbp.att.net>
Date: 2014-05-02T21:41:52Z
some sanity checks in binary protocol
commit 666cc87a51f86ca5940225c36716bbad467c6e73
Author: Aleksey Pesternikov <ap@alekseys-mbp.att.net>
Date: 2014-05-02T21:53:59Z
some sanity checks in compact protocol
diff --git a/lib/go/thrift/binary_protocol.go b/lib/go/thrift/binary_protocol.go
index abbe0bc..09f94d4 100644
--- a/lib/go/thrift/binary_protocol.go
+++ b/lib/go/thrift/binary_protocol.go
@@ -21,6 +21,7 @@
import (
"encoding/binary"
+ "errors"
"fmt"
"io"
"math"
@@ -301,6 +302,8 @@
return nil
}
+var invalidDataLength = NewTProtocolExceptionWithType(INVALID_DATA, errors.New("Invalid data length"))
+
func (p *TBinaryProtocol) ReadMapBegin() (kType, vType TType, size int, err error) {
k, e := p.ReadByte()
if e != nil {
@@ -315,11 +318,15 @@
}
vType = TType(v)
size32, e := p.ReadI32()
- size = int(size32)
if e != nil {
err = NewTProtocolException(e)
return
}
+ if size32 < 0 {
+ err = invalidDataLength
+ return
+ }
+ size = int(size32)
return kType, vType, size, nil
}
@@ -335,12 +342,17 @@
}
elemType = TType(b)
size32, e := p.ReadI32()
- size = int(size32)
if e != nil {
err = NewTProtocolException(e)
return
}
- return elemType, size, nil
+ if size32 < 0 {
+ err = invalidDataLength
+ return
+ }
+ size = int(size32)
+
+ return
}
func (p *TBinaryProtocol) ReadListEnd() error {
@@ -355,11 +367,15 @@
}
elemType = TType(b)
size32, e := p.ReadI32()
- size = int(size32)
if e != nil {
err = NewTProtocolException(e)
return
}
+ if size32 < 0 {
+ err = invalidDataLength
+ return
+ }
+ size = int(size32)
return elemType, size, nil
}
@@ -413,6 +429,11 @@
if e != nil {
return "", e
}
+ if size < 0 {
+ err = invalidDataLength
+ return
+ }
+
return p.readStringBody(int(size))
}
@@ -421,6 +442,10 @@
if e != nil {
return nil, e
}
+ if size < 0 {
+ return nil, invalidDataLength
+ }
+
isize := int(size)
buf := make([]byte, isize)
_, err := io.ReadFull(p.trans, buf)
diff --git a/lib/go/thrift/compact_protocol.go b/lib/go/thrift/compact_protocol.go
index 14bf62d..c275cf4 100644
--- a/lib/go/thrift/compact_protocol.go
+++ b/lib/go/thrift/compact_protocol.go
@@ -421,11 +421,16 @@
// "correct" types.
func (p *TCompactProtocol) ReadMapBegin() (keyType TType, valueType TType, size int, err error) {
size32, e := p.readVarint32()
- size = int(size32)
if e != nil {
err = NewTProtocolException(e)
return
}
+ if size32 < 0 {
+ err = invalidDataLength
+ return
+ }
+ size = int(size32)
+
keyAndValueType := byte(STOP)
if size != 0 {
keyAndValueType, err = p.ReadByte()
@@ -456,6 +461,10 @@
err = NewTProtocolException(e)
return
}
+ if size2 < 0 {
+ err = invalidDataLength
+ return
+ }
size = int(size2)
}
elemType, e := p.getTType(tCompactType(size_and_type))
@@ -541,6 +550,10 @@
if e != nil {
return "", NewTProtocolException(e)
}
+ if length < 0 {
+ return "", invalidDataLength
+ }
+
if length == 0 {
return "", nil
}
@@ -561,7 +574,10 @@
return nil, NewTProtocolException(e)
}
if length == 0 {
- return nil, nil //nil == empty slice
+ return []byte{}, nil
+ }
+ if length < 0 {
+ return nil, invalidDataLength
}
buf := make([]byte, length)
diff --git a/lib/go/thrift/simple_server.go b/lib/go/thrift/simple_server.go
index eb8eb95..9a27215 100644
--- a/lib/go/thrift/simple_server.go
+++ b/lib/go/thrift/simple_server.go
@@ -21,6 +21,7 @@
import (
"log"
+ "runtime/debug"
)
// Simple, non-concurrent server for testing.
@@ -131,7 +132,7 @@
}
if client != nil {
go func() {
- if err := p.processRequest(client); err != nil {
+ if err := p.processRequests(client); err != nil {
log.Println("error processing request:", err)
}
}()
@@ -154,12 +155,17 @@
return nil
}
-func (p *TSimpleServer) processRequest(client TTransport) error {
+func (p *TSimpleServer) processRequests(client TTransport) error {
processor := p.processorFactory.GetProcessor(client)
inputTransport := p.inputTransportFactory.GetTransport(client)
outputTransport := p.outputTransportFactory.GetTransport(client)
inputProtocol := p.inputProtocolFactory.GetProtocol(inputTransport)
outputProtocol := p.outputProtocolFactory.GetProtocol(outputTransport)
+ defer func() {
+ if e := recover(); e != nil {
+ log.Printf("panic in processor: %s: %s", e, debug.Stack())
+ }
+ }()
if inputTransport != nil {
defer inputTransport.Close()
}
@@ -171,6 +177,7 @@
if err, ok := err.(TTransportException); ok && err.TypeId() == END_OF_FILE {
return nil
} else if err != nil {
+ log.Printf("error processing request: %s", err)
return err
}
if !ok {
diff --git a/test/go/src/common/mock_handler.go b/test/go/src/common/mock_handler.go
index 0aed38b..d736ed9 100644
--- a/test/go/src/common/mock_handler.go
+++ b/test/go/src/common/mock_handler.go
@@ -23,8 +23,8 @@
package common
import (
- thrifttest "gen/thrifttest"
gomock "code.google.com/p/gomock/gomock"
+ thrifttest "gen/thrifttest"
)
// Mock of ThriftTest interface