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

lib/go/thrift/binary_protocol.go

@@ -21,6 +21,7 @@ package thrift
 
 import (
 	"encoding/binary"
+	"errors"
 	"fmt"
 	"io"
 	"math"
@@ -301,6 +302,8 @@ func (p *TBinaryProtocol) ReadFieldEnd() error {
 	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 @@ func (p *TBinaryProtocol) ReadMapBegin() (kType, vType TType, size int, err erro
 	}
 	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 @@ func (p *TBinaryProtocol) ReadListBegin() (elemType TType, size int, err error)
 	}
 	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 @@ func (p *TBinaryProtocol) ReadSetBegin() (elemType TType, size int, err error) {
 	}
 	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 @@ func (p *TBinaryProtocol) ReadString() (value string, err error) {
 	if e != nil {
 		return "", e
 	}
+	if size < 0 {
+		err = invalidDataLength
+		return
+	}
+
 	return p.readStringBody(int(size))
 }
 
@@ -421,6 +442,10 @@ func (p *TBinaryProtocol) ReadBinary() ([]byte, error) {
 	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)

lib/go/thrift/compact_protocol.go

@@ -421,11 +421,16 @@ func (p *TCompactProtocol) ReadFieldEnd() error { return nil }
 // "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 @@ func (p *TCompactProtocol) ReadListBegin() (elemType TType, size int, err error)
 			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 @@ func (p *TCompactProtocol) ReadString() (value string, err error) {
 	if e != nil {
 		return "", NewTProtocolException(e)
 	}
+	if length < 0 {
+		return "", invalidDataLength
+	}
+
 	if length == 0 {
 		return "", nil
 	}
@@ -561,7 +574,10 @@ func (p *TCompactProtocol) ReadBinary() (value []byte, err error) {
 		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)

lib/go/thrift/simple_server.go

@@ -21,6 +21,7 @@ package thrift
 
 import (
 	"log"
+	"runtime/debug"
 )
 
 // Simple, non-concurrent server for testing.
@@ -131,7 +132,7 @@ func (p *TSimpleServer) AcceptLoop() error {
 		}
 		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 @@ func (p *TSimpleServer) Stop() error {
 	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 @@ func (p *TSimpleServer) processRequest(client TTransport) error {
 		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 {

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