Do not padding signed bit when encode integers (#1)

* return errors if L of TLV less than 0
* Do not padding signed bit when encode number values

Makefile

@@ -12,7 +12,8 @@ vet:
 	$(GO) vet $(VETPACKAGES)
 
 lint:
-	$(GOLINT) $(GOFILES)
+	#$(GOLINT) $(GOFILES)
+	revive -formatter friendly ./...
 
 test:
 	$(GO) test $(VETPACKAGES)

base_packet.go

@@ -6,29 +6,23 @@ import (
 
 // basePacket is the base type of the NodePacket and PrimitivePacket
 type basePacket struct {
-	tag *Tag
-	// tagbuf []byte
+	tag    *Tag
 	length int
-	// lenbuf []byte
 	valbuf []byte
 	buf    *bytes.Buffer
 }
 
-// func (bp *basePacket) buildBuf() {
-// 	bp.buf = append(bp.tagbuf, bp.lenbuf...)
-// 	bp.buf = append(bp.buf, bp.valbuf...)
-// }
-
-// GetRawBytes get raw bytes of this packet
+// GetRawBytes get all raw bytes of this packet
 func (bp *basePacket) GetRawBytes() []byte {
 	return bp.buf.Bytes()
 }
 
+// Length return the length of Val this packet
 func (bp *basePacket) Length() int {
 	return bp.length
 }
 
-// SeqID returns Tag Key
+// SeqID returns Tag of this packet
 func (bp *basePacket) SeqID() byte {
 	return bp.tag.SeqID()
 }
@@ -38,7 +32,7 @@ func (bp *basePacket) IsSlice() bool {
 	return bp.tag.IsSlice()
 }
 
-// GetValBuf get raw buffer of NodePacket
+// GetValBuf get raw buffer of Val of this packet
 func (bp *basePacket) GetValBuf() []byte {
 	return bp.valbuf
 }

encoder.go

@@ -2,6 +2,7 @@ package y3
 
 import (
 	"bytes"
+	"fmt"
 
 	"github.com/yomorun/y3/encoding"
 )
@@ -39,7 +40,7 @@ func (enc *encoder) addRawPacket(en iEncoder) {
 // setTag write tag as seqID
 func (enc *encoder) writeTag() {
 	if enc.seqID > 0x3F {
-		panic("sid should be in [0..0x7F]")
+		panic(fmt.Errorf("sid should be in [0..0x3F]"))
 	}
 	if enc.isNode {
 		enc.seqID = enc.seqID | 0x80
@@ -51,12 +52,7 @@ func (enc *encoder) writeTag() {
 }
 
 func (enc *encoder) writeLengthBuf() {
-	// vallen := enc.valBuf.Len()
 	vallen := len(enc.valbuf)
-	if vallen < 0 {
-		panic("length must greater than 0")
-	}
-
 	size := encoding.SizeOfPVarInt32(int32(vallen))
 	codec := encoding.VarCodec{Size: size}
 	tmp := make([]byte, size)

encoder_test.go

@@ -0,0 +1,44 @@
+package y3
+
+import (
+	"bytes"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestEncoderWriteTagErrorSeqID(t *testing.T) {
+	enc := &encoder{
+		seqID: 0x40,
+	}
+	assert.PanicsWithError(t, "sid should be in [0..0x3F]", enc.writeTag)
+}
+
+func TestEncoderWriteTagIsNode(t *testing.T) {
+	enc := &encoder{
+		seqID:  0x00,
+		isNode: true,
+		buf:    new(bytes.Buffer),
+	}
+	enc.writeTag()
+	assert.EqualValues(t, 0x80, enc.seqID)
+}
+
+func TestEncoderWriteTagIsPrimitive(t *testing.T) {
+	enc := &encoder{
+		seqID: 0x00,
+		buf:   new(bytes.Buffer),
+	}
+	enc.writeTag()
+	assert.EqualValues(t, 0x00, enc.seqID)
+}
+
+func TestEncoderWriteTagIsSlice(t *testing.T) {
+	enc := &encoder{
+		seqID:   0x00,
+		isArray: true,
+		buf:     new(bytes.Buffer),
+	}
+	enc.writeTag()
+	assert.EqualValues(t, 0x40, enc.seqID)
+}

encoding/pvarbool.go

@@ -11,6 +11,11 @@ func (codec *VarCodec) EncodePVarBool(buffer []byte, value bool) error {
 
 // DecodePVarBool decode to value as PVarBool from buffer
 func (codec *VarCodec) DecodePVarBool(buffer []byte, value *bool) error {
+	if len(buffer) == 0 {
+		*value = false
+		return nil
+	}
+
 	var tmp int64
 	var err = codec.decodePVarInt(buffer, &tmp)
 	if tmp == 1 {

node_decoder.go

@@ -26,7 +26,7 @@ func parsePayload(b []byte) (consumedBytes int, ifNodePacket bool, np *NodePacke
 	return state.ConsumedBytes, false, nil, pp, err
 }
 
-// DecodeNodePacket parse out whole buffer to a NodePacket
+// DecodeToNodePacket parse out whole buffer to a NodePacket
 func DecodeToNodePacket(buf []byte, pct *NodePacket) (consumedBytes int, err error) {
 	if len(buf) == 0 {
 		return 0, errors.New("empty buf")

node_encoder.go

@@ -6,13 +6,13 @@ import (
 
 // NodePacketEncoder used for encode a node packet
 type NodePacketEncoder struct {
-	encoder
+	*encoder
 }
 
 // NewNodePacketEncoder returns an Encoder for node packet
 func NewNodePacketEncoder(sid byte) *NodePacketEncoder {
 	nodeEnc := &NodePacketEncoder{
-		encoder: encoder{
+		encoder: &encoder{
 			isNode: true,
 			buf:    new(bytes.Buffer),
 		},

node_test.go

@@ -57,11 +57,11 @@ func TestSubEmptyNode(t *testing.T) {
 func TestSimple1Node(t *testing.T) {
 	sub := NewPrimitivePacketEncoder(0x01)
 	sub.SetInt32Value(-1)
-	assert.Equal(t, []byte{0x01, 0x01, 0x7F}, sub.Encode())
+	assert.Equal(t, []byte{0x01, 0x01, 0xFF}, sub.Encode())
 
 	node := NewNodePacketEncoder(0x04)
 	node.AddPrimitivePacket(sub)
-	assert.Equal(t, []byte{0x84, 0x03, 0x01, 0x01, 0x7F}, node.Encode())
+	assert.Equal(t, []byte{0x84, 0x03, 0x01, 0x01, 0xFF}, node.Encode())
 
 	res := &NodePacket{}
 	consumedBytes, err := DecodeToNodePacket(node.Encode(), res)
@@ -74,9 +74,10 @@ func TestSimple1Node(t *testing.T) {
 	val, ok := res.PrimitivePackets[1]
 	assert.EqualValues(t, true, ok)
 	if ok {
+		assert.Equal(t, []byte{0x01, 0x01, 0xFF}, val.GetRawBytes())
 		v, err := val.ToInt32()
 		assert.NoError(t, err)
-		assert.Equal(t, []byte{0x01, 0x01, 0x7F}, val.GetRawBytes())
+		assert.Equal(t, []byte{0x01, 0x01, 0xFF}, val.GetRawBytes())
 		assert.Equal(t, int32(-1), v)
 	}
 	assert.Equal(t, 5, consumedBytes)
@@ -91,7 +92,7 @@ func TestSimple1Node(t *testing.T) {
 //   0x03 (node value length is 4 bytes)
 //     0x01, 0x01, 0x7F (pvarint: -1)
 func TestSimpleNodes(t *testing.T) {
-	buf := []byte{0x85, 0x05, 0x84, 0x03, 0x01, 0x01, 0x7F}
+	buf := []byte{0x85, 0x05, 0x84, 0x03, 0x01, 0x01, 0xFF}
 	res := &NodePacket{}
 	consumedBytes, err := DecodeToNodePacket(buf, res)
 	assert.NoError(t, err)
@@ -102,14 +103,14 @@ func TestSimpleNodes(t *testing.T) {
 
 	n, ok := res.NodePackets[0x04]
 	assert.Equal(t, true, ok)
-	assert.Equal(t, []byte{0x84, 0x03, 0x01, 0x01, 0x7F}, n.GetRawBytes())
+	assert.Equal(t, []byte{0x84, 0x03, 0x01, 0x01, 0xFF}, n.GetRawBytes())
 	assert.Equal(t, 0, len(n.NodePackets))
 	assert.Equal(t, 1, len(n.PrimitivePackets))
 	assert.EqualValues(t, 0x04, n.SeqID())
 
 	val, ok := n.PrimitivePackets[0x01]
 	assert.EqualValues(t, true, ok)
-	assert.Equal(t, []byte{0x01, 0x01, 0x7F}, val.GetRawBytes())
+	assert.Equal(t, []byte{0x01, 0x01, 0xFF}, val.GetRawBytes())
 	if ok {
 		v, err := val.ToInt32()
 		assert.NoError(t, err)
@@ -129,7 +130,7 @@ func TestSimpleNodes(t *testing.T) {
 //     0x01, 0x01, 0x7F (pvarint: -1)
 //     0x02, 0x01, 0x01 (pvarint: 1)
 func TestSimple2Nodes(t *testing.T) {
-	buf := []byte{0x83, 0x06, 0x01, 0x01, 0x7F, 0x02, 0x01, 0x01}
+	buf := []byte{0x83, 0x06, 0x01, 0x01, 0xFF, 0x02, 0x01, 0x01}
 	res := &NodePacket{}
 	consumedBytes, err := DecodeToNodePacket(buf, res)
 	assert.NoError(t, err)
@@ -140,16 +141,16 @@ func TestSimple2Nodes(t *testing.T) {
 
 	v1, ok := res.PrimitivePackets[0x01]
 	assert.EqualValues(t, true, ok)
-	v, err := v1.ToInt()
+	v, err := v1.ToInt32()
 	assert.NoError(t, err)
-	assert.Equal(t, []byte{0x01, 0x01, 0x7F}, v1.GetRawBytes())
+	assert.Equal(t, []byte{0x01, 0x01, 0xFF}, v1.GetRawBytes())
 	assert.EqualValues(t, -1, v)
 	assert.NoError(t, err)
 
 	v2, ok := res.PrimitivePackets[0x02]
 	assert.EqualValues(t, true, ok)
 	assert.Equal(t, []byte{0x02, 0x01, 0x01}, v2.GetRawBytes())
-	v, err = v2.ToInt()
+	v, err = v2.ToInt32()
 	assert.NoError(t, err)
 	assert.EqualValues(t, 1, v)
 }
@@ -175,7 +176,7 @@ func TestSimple2Nodes(t *testing.T) {
 //       0x03 (node value length is 4 bytes)
 //         0x01, 0x01, 0x7E (varint: -2)
 func TestComplexNodes(t *testing.T) {
-	buf := []byte{0x85, 0x0D, 0x84, 0x06, 0x01, 0x01, 0x7F, 0x02, 0x01, 0x43, 0x83, 0x03, 0x01, 0x01, 0x7E}
+	buf := []byte{0x85, 0x0D, 0x84, 0x06, 0x01, 0x01, 0xFF, 0x02, 0x01, 0x43, 0x83, 0x03, 0x01, 0x01, 0xFE}
 	res := &NodePacket{}
 	consumedBytes, err := DecodeToNodePacket(buf, res)
 	assert.NoError(t, err)
@@ -186,13 +187,13 @@ func TestComplexNodes(t *testing.T) {
 
 	n1, ok := res.NodePackets[0x04]
 	assert.EqualValues(t, true, ok)
-	assert.Equal(t, []byte{0x84, 0x06, 0x01, 0x01, 0x7F, 0x02, 0x01, 0x43}, n1.GetRawBytes())
+	assert.Equal(t, []byte{0x84, 0x06, 0x01, 0x01, 0xFF, 0x02, 0x01, 0x43}, n1.GetRawBytes())
 	assert.Equal(t, 2, len(n1.PrimitivePackets))
 
 	n1p1, ok := n1.PrimitivePackets[0x01]
 	assert.EqualValues(t, true, ok)
-	assert.Equal(t, []byte{0x01, 0x01, 0x7F}, n1p1.GetRawBytes())
-	vn1p1, err := n1p1.ToInt()
+	assert.Equal(t, []byte{0x01, 0x01, 0xFF}, n1p1.GetRawBytes())
+	vn1p1, err := n1p1.ToInt32()
 	assert.NoError(t, err)
 	assert.EqualValues(t, -1, vn1p1)
 
@@ -205,13 +206,13 @@ func TestComplexNodes(t *testing.T) {
 
 	n2, ok := res.NodePackets[0x03]
 	assert.EqualValues(t, true, ok)
-	assert.Equal(t, []byte{0x83, 0x03, 0x01, 0x01, 0x7E}, n2.GetRawBytes())
+	assert.Equal(t, []byte{0x83, 0x03, 0x01, 0x01, 0xFE}, n2.GetRawBytes())
 	assert.Equal(t, 1, len(n2.PrimitivePackets))
 
 	n2p1, ok := n2.PrimitivePackets[0x01]
 	assert.EqualValues(t, true, ok)
-	assert.Equal(t, []byte{0x01, 0x01, 0x7E}, n2p1.GetRawBytes())
-	vn2p1, err := n2p1.ToInt()
+	assert.Equal(t, []byte{0x01, 0x01, 0xFE}, n2p1.GetRawBytes())
+	vn2p1, err := n2p1.ToInt32()
 	assert.NoError(t, err)
 	assert.EqualValues(t, -2, vn2p1)
 }

parser.go

@@ -3,11 +3,13 @@ package y3
 import (
 	"bytes"
 	"errors"
+	"fmt"
 	"io"
 
 	"github.com/yomorun/y3/encoding"
 )
 
+// ReadPacket will try to read a Y3 encoded packet from the reader
 func ReadPacket(reader io.Reader) ([]byte, error) {
 	tag, err := readByte(reader)
 	if err != nil {
@@ -20,7 +22,7 @@ func ReadPacket(reader io.Reader) ([]byte, error) {
 	// write y3.Tag bytes
 	buf.WriteByte(tag)
 
-	// read y3.Length bytes, a varint format.
+	// read y3.Length bytes, a varint format
 	lenbuf := bytes.Buffer{}
 	for {
 		b, err := readByte(reader)
@@ -39,6 +41,12 @@ func ReadPacket(reader io.Reader) ([]byte, error) {
 	if err != nil {
 		return nil, err
 	}
+
+	// validate len decoded from stream
+	if len < 0 {
+		return nil, fmt.Errorf("y3.ReadPacket() get lenbuf=(%# x), decode len=(%v)", lenbuf.Bytes(), len)
+	}
+
 	// write y3.Length bytes
 	buf.Write(lenbuf.Bytes())
 

primitive_decoder.go

@@ -8,18 +8,21 @@ import (
 	"github.com/yomorun/y3/encoding"
 )
 
-type decodeState struct {
+// DecodeState represents the state of decoding
+type DecodeState struct {
+	// ConsumedBytes is the bytes consumed by decoder
 	ConsumedBytes int
-	SizeL         int
+	// SizeL is the bytes length of value
+	SizeL int
 }
 
-// DecodePrimitivePacket parse out whole buffer to a PrimitivePacket
+// DecodeToPrimitivePacket parse out whole buffer to a PrimitivePacket
 //
 // Examples:
 // [0x01, 0x01, 0x01] -> Key=0x01, Value=0x01
 // [0x41, 0x06, 0x03, 0x01, 0x61, 0x04, 0x01, 0x62] -> key=0x03, value=0x61; key=0x04, value=0x62
-func DecodeToPrimitivePacket(buf []byte, p *PrimitivePacket) (decodeState, error) {
-	decoder := decodeState{
+func DecodeToPrimitivePacket(buf []byte, p *PrimitivePacket) (*DecodeState, error) {
+	decoder := &DecodeState{
 		ConsumedBytes: 0,
 		SizeL:         0,
 	}
@@ -29,15 +32,15 @@ func DecodeToPrimitivePacket(buf []byte, p *PrimitivePacket) (decodeState, error
 	}
 
 	p.basePacket = &basePacket{
-		valbuf: buf,
+		valbuf: []byte{},
 		buf:    &bytes.Buffer{},
 	}
 
 	var pos = 0
 	// first byte is `Tag`
 	p.tag = NewTag(buf[pos])
 	p.buf.WriteByte(buf[pos])
-	pos += 1
+	pos++
 	decoder.ConsumedBytes = pos
 
 	// read `Varint` from buf for `Length of value`
@@ -59,10 +62,15 @@ func DecodeToPrimitivePacket(buf []byte, p *PrimitivePacket) (decodeState, error
 	decoder.ConsumedBytes = pos
 	decoder.SizeL = codec.Size
 
+	// if length<0, error on decoding
+	if bufLen < 0 {
+		return decoder, errors.New("invalid y3 packet, negative length")
+	}
+
 	// the length of value
 	p.length = int(bufLen)
 	if p.length == 0 {
-		p.valbuf = []byte{}
+		p.valbuf = nil
 		return decoder, nil
 	}