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 (file)
--- a/lib/go/thrift/binary_protocol.go
+++ b/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)

diff --git a/lib/go/thrift/compact_protocol.go b/lib/go/thrift/compact_protocol.go
index 14bf62d..c275cf4 100644 (file)
--- a/lib/go/thrift/compact_protocol.go
+++ b/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)

diff --git a/lib/go/thrift/simple_server.go b/lib/go/thrift/simple_server.go
index eb8eb95..9a27215 100644 (file)
--- a/lib/go/thrift/simple_server.go
+++ b/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 {

diff --git a/test/go/src/common/mock_handler.go b/test/go/src/common/mock_handler.go
index 0aed38b..d736ed9 100644 (file)
--- 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