From 3a2f172ef9ac7d2f91f043a81d1b48fbf803e68d Mon Sep 17 00:00:00 2001 From: Paulo Avelar Date: Fri, 6 Jan 2023 22:16:20 -0300 Subject: [PATCH] fix(nodes): prevent nil pointers in node parsing (#23) * fix(nodes): prevent nil pointers in node parsing * chore(docs): improve go doc * chore(lint): add godot to linters --- .golangci.yml | 7 +++- README.md | 24 +++++++++++++ tlv/decoder.go | 88 +++++++++++++++++++++++++++++------------------- tlv/doc.go | 20 +++++------ tlv/node.go | 66 ++++++++++++++++++++++-------------- tlv/node_test.go | 9 +++-- tlv/standard.go | 16 ++++++--- 7 files changed, 152 insertions(+), 78 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 4d123ef..f809f5e 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -20,10 +20,11 @@ linters: - goconst # Enforces constants are created for repeated strings - gocritic # Offers an opinionated set of best practices - gocyclo # Analyzes code complexity (cyclomatic) - - gosimple # Offers code simplification suggestions + - godot # Enforces comments always end with a period - gofmt # Enforces source code is properly formatted - gomnd # Forbids magic numbers from being used without declaration - gosec # Inspects source code for security problems + - gosimple # Offers code simplification suggestions - govet # Reports suspicious construct (e.g. Printf with bad parameter count) - ineffassign # Detects unused existing variable assignments - lll # Limits maximum line lengths @@ -72,6 +73,10 @@ linters-settings: - whyNoLint gocyclo: min-complexity: 8 + godot: + exclude: + - "^(FIXME|TODO) " # technical comments not meant for go doc + - "\\*$" # multiline comments with asterisk indentation gofmt: simplify: true govet: diff --git a/README.md b/README.md index 69e8a02..c43bc54 100644 --- a/README.md +++ b/README.md @@ -125,6 +125,25 @@ nodes** found in the payload. > ⚠️  The decoder works in an all or none strategy when dealing with multiple messages. +### Manually-created nodes use the default decoder configuration + +When a `tlv.Node` is created by declaring the struct, all methods that require context, such as `GetNodes` +or `GetUint8` (or any other integer parser), will use the **standard decoder** definitions. See above for +more details on the decoder. To create a node with custom decoder configuration, first create a decoder +and call the `NewNode` method on it. + + +```go +var node tlv.Node + +node = tlv.Node{Tag: Tag(0x1234), Value: []byte{1}} +node.GetNodes() // uses the standard decoder configuration + +customDecoder := tlv.MustCreateDecoder(1, 1, binary.LittleEndian) +node = customDecoder.NewNode(Tag(0x1234), []byte{1}) +node.GetNodes() // uses the customDecoder configuration +``` + ## Caveats ### No bit parity or checksum @@ -145,6 +164,11 @@ means none of it can be trusted. ## Changelog +* **`v1.1.0`** (2023-06-01) + * [#23](https://github.com/pauloavelar/go-tlv/pull/23): nil pointer errors on manually-created nodes + * fix panics when calling value getters on a node without a decoder reference + * provide functions to create a Node with the proper configuration (standard or custom) + * **`v1.0.0`** (2022-07-01) * **Breaking** change: parser has been renamed to decoder * [#10](https://github.com/pauloavelar/go-tlv/issues/10): add support to custom tag and length sizes diff --git a/tlv/decoder.go b/tlv/decoder.go index b25faad..24fa5c5 100644 --- a/tlv/decoder.go +++ b/tlv/decoder.go @@ -8,21 +8,25 @@ import ( "github.com/pauloavelar/go-tlv/tlv/internal/utils" ) -// Decoder is a TLV decoder with custom configuration. +// Decoder is a configurable TLV decoder instance. type Decoder interface { - // DecodeReader decodes the whole reader to a list of TLV nodes + // DecodeReader decodes the entire reader data to a list of TLV [Nodes]. DecodeReader(reader io.Reader) (Nodes, error) - // DecodeBytes decodes a byte array to a list of TLV nodes + // DecodeBytes decodes a byte array to a list of TLV [Nodes]. DecodeBytes(data []byte) (Nodes, error) - // DecodeSingle decodes a byte array to a single TLV node + // DecodeSingle decodes a byte array to a single TLV [Node]. DecodeSingle(data []byte) (res Node, read uint64, err error) + // NewNode creates a new node using the decoder configuration. + NewNode(tag Tag, value []byte) Node + // GetByteOrder returns the decoder endianness configuration. + GetByteOrder() binary.ByteOrder } type decoder struct { - tagSize uint8 - lengthSize uint8 - minNodeSize uint8 - binaryParser binary.ByteOrder + tagSize uint8 + lengthSize uint8 + minNodeSize uint8 + byteOrder binary.ByteOrder } const ( @@ -32,7 +36,7 @@ const ( maxLenSize = 8 // 2^8 = 256 ) -// MustCreateDecoder creates a decoder using custom configuration or panics in case of any errors. +// MustCreateDecoder creates a [Decoder] using custom configuration or panics in case of any errors. func MustCreateDecoder(tagSize, lengthSize uint8, byteOrder binary.ByteOrder) Decoder { res, err := CreateDecoder(tagSize, lengthSize, byteOrder) if err != nil { @@ -42,8 +46,8 @@ func MustCreateDecoder(tagSize, lengthSize uint8, byteOrder binary.ByteOrder) De return res } -// CreateDecoder creates a decoder using custom configuration. -// Hint: `tagSize` and `lengthSize` must be numbers between 1 and 8. +// CreateDecoder creates a [Decoder] using custom configuration. +// Hint: tagSize and lengthSize must be numbers between 1 and 8. func CreateDecoder(tagSize, lengthSize uint8, byteOrder binary.ByteOrder) (Decoder, error) { if tagSize < minTagSize || tagSize > maxTagSize { return nil, errors.NewInvalidSizeError("tag", tagSize, minTagSize, maxTagSize) @@ -54,29 +58,29 @@ func CreateDecoder(tagSize, lengthSize uint8, byteOrder binary.ByteOrder) (Decod } res := &decoder{ - tagSize: tagSize, - lengthSize: lengthSize, - minNodeSize: tagSize + lengthSize, - binaryParser: byteOrder, + tagSize: tagSize, + lengthSize: lengthSize, + minNodeSize: tagSize + lengthSize, + byteOrder: byteOrder, } return res, nil } -// DecodeReader decodes the full contents of a Reader as TLV nodes. +// DecodeReader decodes the full contents of a [io.Reader] as TLV [Nodes]. // Note: the current implementation loads the entire Reader data into memory. -func (p *decoder) DecodeReader(reader io.Reader) (Nodes, error) { +func (d *decoder) DecodeReader(reader io.Reader) (Nodes, error) { data, err := io.ReadAll(reader) if err != nil { return nil, err } - return p.DecodeBytes(data) + return d.DecodeBytes(data) } -// DecodeBytes decodes a byte array as TLV nodes. -func (p *decoder) DecodeBytes(data []byte) (Nodes, error) { - node, read, err := p.DecodeSingle(data) +// DecodeBytes decodes a byte array as TLV [Nodes]. +func (d *decoder) DecodeBytes(data []byte) (Nodes, error) { + node, read, err := d.DecodeSingle(data) if err != nil { return nil, err } @@ -85,7 +89,7 @@ func (p *decoder) DecodeBytes(data []byte) (Nodes, error) { return Nodes{node}, nil } - next, err := p.DecodeBytes(data[read:]) + next, err := d.DecodeBytes(data[read:]) if err != nil { return nil, err } @@ -93,28 +97,42 @@ func (p *decoder) DecodeBytes(data []byte) (Nodes, error) { return append(Nodes{node}, next...), nil } -// DecodeSingle decodes a byte array as a single TLV node. -func (p *decoder) DecodeSingle(data []byte) (res Node, read uint64, err error) { - if len(data) < int(p.minNodeSize) { +// DecodeSingle decodes a byte array as a single TLV [Node]. +func (d *decoder) DecodeSingle(data []byte) (res Node, read uint64, err error) { + if len(data) < int(d.minNodeSize) { return res, 0, errors.NewMessageTooShortError(data) } - tag := utils.GetPaddedUint64(p.binaryParser, data[:p.tagSize]) - length := utils.GetPaddedUint64(p.binaryParser, data[p.tagSize:p.minNodeSize]) - messageLength := uint64(p.minNodeSize) + length + tag := utils.GetPaddedUint64(d.byteOrder, data[:d.tagSize]) + length := utils.GetPaddedUint64(d.byteOrder, data[d.tagSize:d.minNodeSize]) + messageLength := uint64(d.minNodeSize) + length if len(data) < int(messageLength) { - return res, 0, errors.NewLengthMismatchError(length, data, p.minNodeSize) + return res, 0, errors.NewLengthMismatchError(length, data, d.minNodeSize) } node := Node{ - Tag: Tag(tag), - Length: Length(length), - Value: data[p.minNodeSize:messageLength], - Raw: data[:messageLength], - decoder: p, - binParser: p.binaryParser, + Tag: Tag(tag), + Length: Length(length), + Value: data[d.minNodeSize:messageLength], + Raw: data[:messageLength], + decoder: d, } return node, messageLength, nil } + +// NewNode creates a new [Node] using the [Decoder] configuration. +func (d *decoder) NewNode(tag Tag, value []byte) Node { + return Node{ + Tag: tag, + Length: Length(len(value)), + Value: value, + decoder: d, + } +} + +// GetByteOrder returns the [Decoder] endianness configuration. +func (d *decoder) GetByteOrder() binary.ByteOrder { + return d.byteOrder +} diff --git a/tlv/doc.go b/tlv/doc.go index 3f5cf86..c03564f 100644 --- a/tlv/doc.go +++ b/tlv/doc.go @@ -1,26 +1,26 @@ /* -Package tlv holds all logic to decode TLV messages into `Nodes`. +Package tlv holds all logic to decode TLV messages into [Nodes]. There are two ways to use this package: with the standard decoder or creating -a custom Decoder with custom tag/length sizes and byte order. The standard -decoder uses 2-byte tags, 2-byte lengths and big endian byte order. +a custom [Decoder] with custom tag/length sizes and byte order. The standard +decoder uses 2-byte tags, 2-byte lengths and big endian [binary.ByteOrder]. -Nodes are a representation of a collection of decoded TLV messages. -Specific messages can be filtered by Tag and indexes can be accessed +[Nodes] are a representation of a collection of decoded TLV messages. +Specific messages can be filtered by [Tag] and indexes can be accessed directly in an array-like syntax: firstNode := nodes[0] -Node is a representation of a single TLV message, comprised of a Tag, -a Length and a Value. The struct has many helper methods to parse the +[Node] is a representation of a single TLV message, comprised of a [Tag], +a [Length] and a Value. The struct has many helper methods to parse the value as different types (e.g. integers, strings, dates and booleans), -as well as nested TLV Nodes. +as well as nested TLV [Nodes]. node.GetNodes() node.GetPaddedUint8() -Note: Nodes decoded with a custom configuration retain the configuration -when parsing their values as nodes, so messages always have consistent +Note: [Nodes] decoded with a custom configuration retain the configuration +when parsing their values as other nodes, so messages always have consistent tag/length sizes and byte order. */ package tlv diff --git a/tlv/node.go b/tlv/node.go index 2ee4b77..b93ae71 100644 --- a/tlv/node.go +++ b/tlv/node.go @@ -9,34 +9,33 @@ import ( "github.com/pauloavelar/go-tlv/tlv/internal/utils" ) -// Node structure used to represent a decoded TLV message +// Node structure used to represent a decoded TLV message. type Node struct { Tag Tag Length Length Value []byte Raw []byte - decoder Decoder - binParser binary.ByteOrder + decoder Decoder } -// Tag node identifier composed by 1 to 8 bytes (uint64) +// Tag node identifier composed by 1 to 8 bytes (uint64). type Tag uint64 -// Length value size in bytes +// Length value size in bytes. type Length uint64 -// String converts the node bytes to base64 +// String converts the node bytes to base64. func (n *Node) String() string { return base64.StdEncoding.EncodeToString(n.Raw) } -// GetNodes parses the value as decoded TLV nodes +// GetNodes parses the value as decoded TLV nodes. func (n *Node) GetNodes() (Nodes, error) { - return n.decoder.DecodeBytes(n.Value) + return n.getSafeDecoder().DecodeBytes(n.Value) } -// GetBool parses the value as boolean if it has enough bytes +// GetBool parses the value as boolean if it has enough bytes. func (n *Node) GetBool() (res, ok bool) { if len(n.Value) < sizes.Bool { return false, false @@ -45,18 +44,18 @@ func (n *Node) GetBool() (res, ok bool) { return n.Value[0] != 0, true } -// GetPaddedBool parses the value as boolean regardless of its size +// GetPaddedBool parses the value as boolean regardless of its size. func (n *Node) GetPaddedBool() bool { res, _ := n.GetBool() return res } -// GetString parses the value as UTF8 text +// GetString parses the value as UTF-8 text. func (n *Node) GetString() string { return string(n.Value) } -// GetDate parses the value as date if it has enough bytes +// GetDate parses the value as date if it has enough bytes. func (n *Node) GetDate() (res time.Time, ok bool) { if len(n.Value) == 0 { return res, false @@ -66,7 +65,7 @@ func (n *Node) GetDate() (res time.Time, ok bool) { return time.Unix(int64(epoch), 0).UTC(), true } -// GetUint8 parses the value as uint8 +// GetUint8 parses the value as uint8. func (n *Node) GetUint8() (res uint8, ok bool) { if len(n.Value) < sizes.Uint8 { return 0, false @@ -75,7 +74,7 @@ func (n *Node) GetUint8() (res uint8, ok bool) { return n.Value[0], true } -// GetPaddedUint8 parses the value as uint8 regardless of size +// GetPaddedUint8 parses the value as uint8 regardless of size. func (n *Node) GetPaddedUint8() uint8 { if len(n.Value) < sizes.Uint8 { return 0 @@ -84,47 +83,62 @@ func (n *Node) GetPaddedUint8() uint8 { return n.Value[0] } -// GetUint16 parses the value as uint16 if it has enough bytes +// GetUint16 parses the value as uint16 if it has enough bytes. func (n *Node) GetUint16() (res uint16, ok bool) { if len(n.Value) < sizes.Uint16 { return 0, false } - return n.binParser.Uint16(n.Value), true + return n.getByteOrder().Uint16(n.Value), true } -// GetPaddedUint16 parses the value as uint16 regardless of size +// GetPaddedUint16 parses the value as uint16 regardless of size. func (n *Node) GetPaddedUint16() uint16 { padding := utils.GetPadding(sizes.Uint16, len(n.Value)) - return n.binParser.Uint16(append(padding, n.Value...)) + + return n.getByteOrder().Uint16(append(padding, n.Value...)) } -// GetUint32 parses the value as uint32 if it has enough bytes +// GetUint32 parses the value as uint32 if it has enough bytes. func (n *Node) GetUint32() (res uint32, exists bool) { if len(n.Value) < sizes.Uint32 { return 0, false } - return n.binParser.Uint32(n.Value), true + return n.getByteOrder().Uint32(n.Value), true } -// GetPaddedUint32 parses the value as uint32 regardless of size +// GetPaddedUint32 parses the value as uint32 regardless of size. func (n *Node) GetPaddedUint32() uint32 { padding := utils.GetPadding(sizes.Uint32, len(n.Value)) - return n.binParser.Uint32(append(padding, n.Value...)) + + return n.getByteOrder().Uint32(append(padding, n.Value...)) } -// GetUint64 parses the value as uint64 if it has enough bytes +// GetUint64 parses the value as uint64 if it has enough bytes. func (n *Node) GetUint64() (res uint64, ok bool) { if len(n.Value) < sizes.Uint64 { return 0, false } - return n.binParser.Uint64(n.Value), true + return n.getByteOrder().Uint64(n.Value), true } -// GetPaddedUint64 parses the value as uint64 regardless of size +// GetPaddedUint64 parses the value as uint64 regardless of size. func (n *Node) GetPaddedUint64() uint64 { padding := utils.GetPadding(sizes.Uint64, len(n.Value)) - return n.binParser.Uint64(append(padding, n.Value...)) + + return n.getByteOrder().Uint64(append(padding, n.Value...)) +} + +func (n *Node) getSafeDecoder() Decoder { + if n.decoder != nil { + return n.decoder + } + + return stdDecoder +} + +func (n *Node) getByteOrder() binary.ByteOrder { + return n.getSafeDecoder().GetByteOrder() } diff --git a/tlv/node_test.go b/tlv/node_test.go index 7622d2f..4d151eb 100644 --- a/tlv/node_test.go +++ b/tlv/node_test.go @@ -1,7 +1,6 @@ package tlv import ( - "encoding/binary" "testing" "time" @@ -261,6 +260,12 @@ func TestNode_GetPaddedUint64(t *testing.T) { } } +func TestNode_GetUint16_WhenTheNodeIsDeclaredManually(t *testing.T) { + node := &Node{Value: []byte{0x12, 0x34}} + require.Equal(t, uint16(0x1234), node.GetPaddedUint16()) +} + func newNode(value []byte) *Node { - return &Node{binParser: binary.BigEndian, Value: value} + node := NewNode(Tag(0x01), value) + return &node } diff --git a/tlv/standard.go b/tlv/standard.go index 084c943..b369df1 100644 --- a/tlv/standard.go +++ b/tlv/standard.go @@ -7,20 +7,28 @@ import ( "github.com/pauloavelar/go-tlv/tlv/internal/sizes" ) +// stdByteOrder is the default endianness for parsing numbers. +var stdByteOrder = binary.BigEndian + // stdDecoder uses 2 bytes for tags and lengths and parses them as big endian. -var stdDecoder = MustCreateDecoder(sizes.Uint16, sizes.Uint16, binary.BigEndian) +var stdDecoder = MustCreateDecoder(sizes.Uint16, sizes.Uint16, stdByteOrder) -// DecodeReader decodes the whole reader as a list of TLV nodes. +// DecodeReader decodes the entire [io.Reader] data as a list of TLV nodes. func DecodeReader(reader io.Reader) (Nodes, error) { return stdDecoder.DecodeReader(reader) } -// DecodeBytes decodes a byte array as a list of TLV nodes. +// DecodeBytes decodes a byte array as a list of TLV [Nodes]. func DecodeBytes(data []byte) (Nodes, error) { return stdDecoder.DecodeBytes(data) } -// DecodeSingle decodes a byte array as a single TLV node. +// DecodeSingle decodes a byte array as a single TLV [Node]. func DecodeSingle(data []byte) (res Node, read uint64, err error) { return stdDecoder.DecodeSingle(data) } + +// NewNode creates a [Node] with the default [Decoder] configuration. +func NewNode(tag Tag, value []byte) Node { + return stdDecoder.NewNode(tag, value) +}