From c9178993037dcd44bc3c64ab6031bec0baa9bf8a Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Sun, 30 Mar 2014 15:56:52 -0500 Subject: [PATCH] Allow optional fields in MsgVersion decode. This commit modifies the MsgVersion.BtcDecode function to match the behavior where fields after the first address field (AddrYou) are optional and only read if the buffer contains remaining bytes. Unfortunately this means the reader for MsgVersion.BtcDecode must be a *bytes.Buffer or an error is returned. This is not an issue for the vast majority of cases since all of the message reading code which is the main way messages are read is already using a *bytes.Buffer, however, this change might affect external callers if they are doing something special with custom readers. Fixes #14. --- msgversion.go | 71 ++++++++++++++++++++++++++++++---------------- msgversion_test.go | 22 +++++++++----- test_coverage.txt | 8 +++--- 3 files changed, 66 insertions(+), 35 deletions(-) diff --git a/msgversion.go b/msgversion.go index 0fc18b3f..15a4e212 100644 --- a/msgversion.go +++ b/msgversion.go @@ -5,6 +5,7 @@ package btcwire import ( + "bytes" "fmt" "io" "net" @@ -66,44 +67,66 @@ func (msg *MsgVersion) AddService(service ServiceFlag) { } // BtcDecode decodes r using the bitcoin protocol encoding into the receiver. +// The version message is special in that the protocol version hasn't been +// negotiated yet. As a result, the pver field is ignored and any fields which +// are added in new versions are optional. This also mean that r must be a +// *bytes.Buffer so the number of remaining bytes can be ascertained. +// // This is part of the Message interface implementation. func (msg *MsgVersion) BtcDecode(r io.Reader, pver uint32) error { + buf, ok := r.(*bytes.Buffer) + if !ok { + return fmt.Errorf("MsgVersion.BtcDecode reader is not a " + + "*bytes.Buffer") + } + var sec int64 - err := readElements(r, &msg.ProtocolVersion, &msg.Services, &sec) + err := readElements(buf, &msg.ProtocolVersion, &msg.Services, &sec) if err != nil { return err } msg.Timestamp = time.Unix(sec, 0) - err = readNetAddress(r, pver, &msg.AddrYou, false) + err = readNetAddress(buf, pver, &msg.AddrYou, false) if err != nil { return err } - err = readNetAddress(r, pver, &msg.AddrMe, false) - if err != nil { - return err + // Protocol versions >= 106 added a from address, nonce, and user agent + // field and they are only considered present if there are bytes + // remaining in the message. + if buf.Len() > 0 { + err = readNetAddress(buf, pver, &msg.AddrMe, false) + if err != nil { + return err + } + } + if buf.Len() > 0 { + err = readElement(buf, &msg.Nonce) + if err != nil { + return err + } + } + if buf.Len() > 0 { + userAgent, err := readVarString(buf, pver) + if err != nil { + return err + } + if len(userAgent) > MaxUserAgentLen { + str := fmt.Sprintf("user agent too long [len %v, max %v]", + len(userAgent), MaxUserAgentLen) + return messageError("MsgVersion.BtcDecode", str) + } + msg.UserAgent = userAgent } - err = readElement(r, &msg.Nonce) - if err != nil { - return err - } - - userAgent, err := readVarString(r, pver) - if err != nil { - return err - } - if len(userAgent) > MaxUserAgentLen { - str := fmt.Sprintf("user agent too long [len %v, max %v]", - len(userAgent), MaxUserAgentLen) - return messageError("MsgVersion.BtcDecode", str) - } - msg.UserAgent = userAgent - - err = readElement(r, &msg.LastBlock) - if err != nil { - return err + // Protocol versions >= 209 added a last known block field. It is only + // considered present if there are bytes remaining in the message. + if buf.Len() > 0 { + err = readElement(buf, &msg.LastBlock) + if err != nil { + return err + } } return nil diff --git a/msgversion_test.go b/msgversion_test.go index d12e7044..45561d13 100644 --- a/msgversion_test.go +++ b/msgversion_test.go @@ -236,6 +236,14 @@ func TestVersionWireErrors(t *testing.T) { pver := uint32(60002) btcwireErr := &btcwire.MessageError{} + // Ensure calling MsgVersion.BtcDecode with a non *bytes.Buffer returns + // error. + fr := newFixedReader(0, []byte{}) + if err := baseVersion.BtcDecode(fr, pver); err == nil { + t.Errorf("Did not received error when calling " + + "MsgVersion.BtcDecode with non *bytes.Buffer") + } + // Copy the base version and change the user agent to exceed max limits. bvc := *baseVersion exceedUAVer := &bvc @@ -277,15 +285,15 @@ func TestVersionWireErrors(t *testing.T) { // Force error in remote address. {baseVersion, baseVersionEncoded, pver, 20, io.ErrShortWrite, io.EOF}, // Force error in local address. - {baseVersion, baseVersionEncoded, pver, 46, io.ErrShortWrite, io.EOF}, + {baseVersion, baseVersionEncoded, pver, 47, io.ErrShortWrite, io.ErrUnexpectedEOF}, // Force error in nonce. - {baseVersion, baseVersionEncoded, pver, 72, io.ErrShortWrite, io.EOF}, + {baseVersion, baseVersionEncoded, pver, 73, io.ErrShortWrite, io.ErrUnexpectedEOF}, // Force error in user agent length. - {baseVersion, baseVersionEncoded, pver, 80, io.ErrShortWrite, io.EOF}, - // Force error in user agent. {baseVersion, baseVersionEncoded, pver, 81, io.ErrShortWrite, io.EOF}, + // Force error in user agent. + {baseVersion, baseVersionEncoded, pver, 82, io.ErrShortWrite, io.ErrUnexpectedEOF}, // Force error in last block. - {baseVersion, baseVersionEncoded, pver, 97, io.ErrShortWrite, io.EOF}, + {baseVersion, baseVersionEncoded, pver, 98, io.ErrShortWrite, io.ErrUnexpectedEOF}, // Force error due to user agent too big. {exceedUAVer, exceedUAVerEncoded, pver, newLen, btcwireErr, btcwireErr}, } @@ -313,8 +321,8 @@ func TestVersionWireErrors(t *testing.T) { // Decode from wire format. var msg btcwire.MsgVersion - r := newFixedReader(test.max, test.buf) - err = msg.BtcDecode(r, test.pver) + buf := bytes.NewBuffer(test.buf[0:test.max]) + err = msg.BtcDecode(buf, test.pver) if reflect.TypeOf(err) != reflect.TypeOf(test.readErr) { t.Errorf("BtcDecode #%d wrong error got: %v, want: %v", i, err, test.readErr) diff --git a/test_coverage.txt b/test_coverage.txt index 6b9fac8b..4672a003 100644 --- a/test_coverage.txt +++ b/test_coverage.txt @@ -1,11 +1,11 @@ -github.com/conformal/btcwire/common.go writeElement 100.00% (61/61) github.com/conformal/btcwire/common.go readElement 100.00% (61/61) +github.com/conformal/btcwire/common.go writeElement 100.00% (61/61) github.com/conformal/btcwire/message.go ReadMessageN 100.00% (42/42) github.com/conformal/btcwire/message.go WriteMessageN 100.00% (38/38) github.com/conformal/btcwire/msgtx.go MsgTx.BtcDecode 100.00% (36/36) +github.com/conformal/btcwire/msgversion.go MsgVersion.BtcDecode 100.00% (32/32) github.com/conformal/btcwire/msgtx.go MsgTx.BtcEncode 100.00% (26/26) -github.com/conformal/btcwire/msgversion.go MsgVersion.BtcDecode 100.00% (25/25) github.com/conformal/btcwire/msgtx.go MsgTx.Copy 100.00% (24/24) github.com/conformal/btcwire/msgtx.go readTxIn 100.00% (22/22) github.com/conformal/btcwire/msgversion.go MsgVersion.BtcEncode 100.00% (22/22) @@ -48,7 +48,7 @@ github.com/conformal/btcwire/common.go randomUint64 100.00% (7/7) github.com/conformal/btcwire/msgversion.go NewMsgVersionFromConn 100.00% (7/7) github.com/conformal/btcwire/msgpong.go MsgPong.BtcEncode 100.00% (7/7) github.com/conformal/btcwire/msgpong.go MsgPong.BtcDecode 100.00% (7/7) -github.com/conformal/btcwire/common.go varIntSerializeSize 100.00% (7/7) +github.com/conformal/btcwire/common.go VarIntSerializeSize 100.00% (7/7) github.com/conformal/btcwire/blockheader.go readBlockHeader 100.00% (6/6) github.com/conformal/btcwire/common.go DoubleSha256 100.00% (6/6) github.com/conformal/btcwire/msgtx.go MsgTx.SerializeSize 100.00% (6/6) @@ -165,5 +165,5 @@ github.com/conformal/btcwire/msggetdata.go NewMsgGetData 100.00% (1/1) github.com/conformal/btcwire/msgmempool.go MsgMemPool.Command 100.00% (1/1) github.com/conformal/btcwire/msgmempool.go MsgMemPool.MaxPayloadLength 100.00% (1/1) github.com/conformal/btcwire/msgmempool.go NewMsgMemPool 100.00% (1/1) -github.com/conformal/btcwire --------------------------------- 100.00% (1153/1153) +github.com/conformal/btcwire --------------------------------- 100.00% (1160/1160)