From f55d1a1b37da81d7314bf388c10d6f4c8eaaafe0 Mon Sep 17 00:00:00 2001 From: Janick Fischer Date: Mon, 2 Mar 2026 09:50:28 +0100 Subject: [PATCH 1/7] fix(common): skip unknown TLV tags instead of failing The TLV parser returned a hard error on unknown tags, which broke forward-compatibility when new config settings were added to payloads. Unknown tags are now skipped using their length field, with a warning logged via slog. Closes #180 --- pkg/common/helpers.go | 4 +++- pkg/common/helpers_test.go | 11 +++++++---- pkg/decoder/tagxl/v1/decoder_test.go | 7 +++---- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/pkg/common/helpers.go b/pkg/common/helpers.go index fb03580e..cf8d5f76 100644 --- a/pkg/common/helpers.go +++ b/pkg/common/helpers.go @@ -4,6 +4,7 @@ import ( "encoding/hex" "errors" "fmt" + "log/slog" "math" "unsafe" @@ -201,7 +202,8 @@ func Decode(payloadHex *string, config *PayloadConfig) (any, error) { if found { index += length } else { - return nil, fmt.Errorf("unknown tag %x", tag) + slog.Warn("skipping unknown tag", "tag", fmt.Sprintf("%x", tag), "length", length) + index += length } } diff --git a/pkg/common/helpers_test.go b/pkg/common/helpers_test.go index 73aa45ee..2933d182 100644 --- a/pkg/common/helpers_test.go +++ b/pkg/common/helpers_test.go @@ -141,10 +141,13 @@ func TestDecode(t *testing.T) { expectedErr: "validation failed for Time", }, { - payload: "ffffff040100", - config: tagConfig, - expected: nil, - expectedErr: "unknown tag 4", + payload: "ffffff040100", + config: tagConfig, + expected: Port2Payload{ + Time: nil, + Power: nil, + Sensor: nil, + }, }, { payload: "ffffff010200", diff --git a/pkg/decoder/tagxl/v1/decoder_test.go b/pkg/decoder/tagxl/v1/decoder_test.go index e659531c..68cd0dce 100644 --- a/pkg/decoder/tagxl/v1/decoder_test.go +++ b/pkg/decoder/tagxl/v1/decoder_test.go @@ -117,10 +117,9 @@ func TestDecode(t *testing.T) { expectedErr: "port not supported: port 151 tag ff", }, { - port: 151, - payload: "4c0501ff020000", - expected: Port151Payload{}, - expectedErr: "unknown tag ff", + port: 151, + payload: "4c0501ff020000", + expected: Port151Payload{}, }, { port: 151, From f8c8e6a8a8a55825dabeb4c7c8e9810b52df39cd Mon Sep 17 00:00:00 2001 From: Janick Fischer Date: Mon, 2 Mar 2026 10:37:26 +0100 Subject: [PATCH 2/7] test(tagxl): add mixed known+unknown TLV test case Validates that known tags still decode correctly when an unknown tag appears in-between, complementing the existing unknown-only test. --- pkg/decoder/tagxl/v1/decoder_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pkg/decoder/tagxl/v1/decoder_test.go b/pkg/decoder/tagxl/v1/decoder_test.go index 68cd0dce..e3f87508 100644 --- a/pkg/decoder/tagxl/v1/decoder_test.go +++ b/pkg/decoder/tagxl/v1/decoder_test.go @@ -121,6 +121,14 @@ func TestDecode(t *testing.T) { payload: "4c0501ff020000", expected: Port151Payload{}, }, + { + port: 151, + payload: "4c0d0345020a92ff03aabbcc4e0107", + expected: Port151Payload{ + Battery: helpers.Float32Ptr(2.706), + DataRate: helpers.DataRatePtr(decoder.DataRateTagXLADR), + }, + }, { port: 151, payload: "4c040140010a", From 465382f219ee87bf6e920a46f4ede8797fa45e80 Mon Sep 17 00:00:00 2001 From: Janick Fischer Date: Mon, 2 Mar 2026 10:46:44 +0100 Subject: [PATCH 3/7] fix(common): use int for TLV loop index to prevent uint8 overflow The TLV parsing loop declared index and payloadLength as uint8, which could wrap around on large payloads and cause out-of-bounds access. Also adds a bounds check before advancing index past a TLV value. Co-Authored-By: Claude Opus 4.6 --- pkg/common/helpers.go | 20 +++++++++++--------- pkg/common/helpers_test.go | 2 +- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/pkg/common/helpers.go b/pkg/common/helpers.go index cf8d5f76..c8133b8c 100644 --- a/pkg/common/helpers.go +++ b/pkg/common/helpers.go @@ -155,21 +155,25 @@ func Decode(payloadHex *string, config *PayloadConfig) (any, error) { errs := []error{} if len(config.Tags) != 0 { - var index uint8 = 3 - var payloadLength = uint8(len(payloadBytes)) + var index int = 3 + var payloadLength = len(payloadBytes) for index+2 < payloadLength { - var found = false var tag = payloadBytes[index] index++ - var length = payloadBytes[index] + var length = int(payloadBytes[index]) index++ + if index+length > payloadLength { + return nil, fmt.Errorf("TLV tag 0x%02x at offset %d declares length %d, but only %d bytes remain", tag, index-2, length, payloadLength-index) + } + + var found bool for _, tagConfig := range config.Tags { if tagConfig.Tag == tag { found = true config.Features = append(config.Features, tagConfig.Feature) - value, err := extractFieldValue(payloadBytes, int(index), int(length), false, tagConfig.Hex) + value, err := extractFieldValue(payloadBytes, index, length, false, tagConfig.Hex) if err != nil { return nil, err } @@ -199,12 +203,10 @@ func Decode(payloadHex *string, config *PayloadConfig) (any, error) { } } } - if found { - index += length - } else { + if !found { slog.Warn("skipping unknown tag", "tag", fmt.Sprintf("%x", tag), "length", length) - index += length } + index += length } return targetValue.Interface(), errors.Join(errs...) diff --git a/pkg/common/helpers_test.go b/pkg/common/helpers_test.go index 2933d182..3f46f4cc 100644 --- a/pkg/common/helpers_test.go +++ b/pkg/common/helpers_test.go @@ -153,7 +153,7 @@ func TestDecode(t *testing.T) { payload: "ffffff010200", config: tagConfig, expected: nil, - expectedErr: "field out of bounds", + expectedErr: "TLV tag", }, { payload: "ffffff030100", From b89fc9edbc6df078791b17ff852e432c1f4ef475 Mon Sep 17 00:00:00 2001 From: Janick Fischer Date: Mon, 2 Mar 2026 10:57:57 +0100 Subject: [PATCH 4/7] test(common): fix silent pass in table-driven test error assertions The error assertions in TestDecode, TestExtractFieldValue, TestConvertFieldValue, and TestInsertFieldBytes would silently pass when expectedErr was set but no error was returned. Now each loop explicitly fails on missing expected errors and on unexpected errors. Co-Authored-By: Claude Opus 4.6 --- pkg/common/helpers_test.go | 44 +++++++++++++++++++++++++++++++------- 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/pkg/common/helpers_test.go b/pkg/common/helpers_test.go index 3f46f4cc..cf6ef67b 100644 --- a/pkg/common/helpers_test.go +++ b/pkg/common/helpers_test.go @@ -166,8 +166,15 @@ func TestDecode(t *testing.T) { for _, test := range tests { t.Run(test.payload, func(t *testing.T) { decodedData, err := Decode(StringPtr(test.payload), &test.config) - if err != nil && !strings.Contains(err.Error(), test.expectedErr) { - t.Fatalf("expected %s received %s", test.expectedErr, err) + if test.expectedErr != "" { + if err == nil { + t.Fatalf("expected error containing %q but got nil", test.expectedErr) + } + if !strings.Contains(err.Error(), test.expectedErr) { + t.Fatalf("expected error containing %q received %s", test.expectedErr, err) + } + } else if err != nil { + t.Fatalf("unexpected error: %s", err) } if !reflect.DeepEqual(decodedData, test.expected) { t.Fatalf("expected: %+v received: %+v", test.expected, decodedData) @@ -242,8 +249,15 @@ func TestExtractFieldValue(t *testing.T) { for _, test := range tests { t.Run(fmt.Sprintf("%v_%v_%v", test.payload, test.start, test.length), func(t *testing.T) { result, err := extractFieldValue(test.payload, test.start, test.length, test.optional, test.hexadecimal) - if err != nil && err.Error() != test.expectedErr { - t.Fatalf("expected: %s received: %s", test.expectedErr, err.Error()) + if test.expectedErr != "" { + if err == nil { + t.Fatalf("expected error %q but got nil", test.expectedErr) + } + if err.Error() != test.expectedErr { + t.Fatalf("expected: %s received: %s", test.expectedErr, err.Error()) + } + } else if err != nil { + t.Fatalf("unexpected error: %s", err) } if !reflect.DeepEqual(result, test.expected) { t.Fatalf("expected: %v received: %v", test.expected, result) @@ -376,8 +390,15 @@ func TestConvertFieldValue(t *testing.T) { for _, test := range tests { t.Run(fmt.Sprintf("%v_%v", test.value, test.fieldType), func(t *testing.T) { result, err := convertFieldValue(test.value, test.fieldType, nil) - if err != nil && err.Error() != test.expectedErr { - t.Fatalf("expected: %s received: %s", test.expectedErr, err.Error()) + if test.expectedErr != "" { + if err == nil { + t.Fatalf("expected error %q but got nil", test.expectedErr) + } + if err.Error() != test.expectedErr { + t.Fatalf("expected: %s received: %s", test.expectedErr, err.Error()) + } + } else if err != nil { + t.Fatalf("unexpected error: %s", err) } if !reflect.DeepEqual(result, test.expected) { t.Fatalf("expected: %v received: %v", test.expected, result) @@ -453,8 +474,15 @@ func TestInsertFieldBytes(t *testing.T) { for _, test := range tests { set, bytes, err := insertFieldBytes(test.value, test.length, test.transform) - if err != nil && err.Error() != test.expectedErr { - t.Fatalf("expected: %s received: %s", test.expectedErr, err.Error()) + if test.expectedErr != "" { + if err == nil { + t.Fatalf("expected error %q but got nil", test.expectedErr) + } + if err.Error() != test.expectedErr { + t.Fatalf("expected: %s received: %s", test.expectedErr, err.Error()) + } + } else if err != nil { + t.Fatalf("unexpected error: %s", err) } if !set && err == nil { t.Fatalf("expected set to be true when error is nil") From 20de9f08ed895341d5208cb8cff7b980079b58c6 Mon Sep 17 00:00:00 2001 From: Janick Fischer Date: Tue, 3 Mar 2026 13:58:43 +0100 Subject: [PATCH 5/7] feat(common): add Prometheus metric and zap logging for unknown TLV tags Replace slog with logger.Logger (zap) and add a truvami_common_unknown_tlv_tags_total counter metric to track unknown TLV tags encountered during decoding. Co-Authored-By: Claude Opus 4.6 --- pkg/common/helpers.go | 9 +++++++-- pkg/common/metrics.go | 13 +++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) create mode 100644 pkg/common/metrics.go diff --git a/pkg/common/helpers.go b/pkg/common/helpers.go index c8133b8c..b7a46e13 100644 --- a/pkg/common/helpers.go +++ b/pkg/common/helpers.go @@ -4,7 +4,6 @@ import ( "encoding/hex" "errors" "fmt" - "log/slog" "math" "unsafe" @@ -12,6 +11,8 @@ import ( "time" "github.com/go-playground/validator" + "github.com/truvami/decoder/internal/logger" + "go.uber.org/zap" ) func HexStringToBytes(hexString string) ([]byte, error) { @@ -204,7 +205,11 @@ func Decode(payloadHex *string, config *PayloadConfig) (any, error) { } } if !found { - slog.Warn("skipping unknown tag", "tag", fmt.Sprintf("%x", tag), "length", length) + tagHex := fmt.Sprintf("0x%02x", tag) + unknownTLVTagsTotal.WithLabelValues(tagHex).Inc() + if logger.Logger != nil { + logger.Logger.Warn("skipping unknown tag", zap.String("tag", tagHex), zap.Int("length", length)) + } } index += length } diff --git a/pkg/common/metrics.go b/pkg/common/metrics.go new file mode 100644 index 00000000..fee44256 --- /dev/null +++ b/pkg/common/metrics.go @@ -0,0 +1,13 @@ +package common + +import ( + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" +) + +var ( + unknownTLVTagsTotal = promauto.NewCounterVec(prometheus.CounterOpts{ + Name: "truvami_common_unknown_tlv_tags_total", + Help: "The total number of unknown TLV tags encountered during decoding", + }, []string{"tag"}) +) From 1b5cbd4acb7b97771839ae5f5de69ffaaefc2736 Mon Sep 17 00:00:00 2001 From: Janick Fischer Date: Tue, 3 Mar 2026 14:19:52 +0100 Subject: [PATCH 6/7] fix(common): remove redundant type in variable declaration Satisfies staticcheck QF1011: omit type int from declaration as it is inferred from the right-hand side. Co-Authored-By: Claude Opus 4.6 --- pkg/common/helpers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/common/helpers.go b/pkg/common/helpers.go index b7a46e13..f821d5e9 100644 --- a/pkg/common/helpers.go +++ b/pkg/common/helpers.go @@ -156,7 +156,7 @@ func Decode(payloadHex *string, config *PayloadConfig) (any, error) { errs := []error{} if len(config.Tags) != 0 { - var index int = 3 + var index = 3 var payloadLength = len(payloadBytes) for index+2 < payloadLength { var tag = payloadBytes[index] From d0fbd94e8525d9fd77bd8aa8c462b7610103b283 Mon Sep 17 00:00:00 2001 From: Janick Fischer Date: Tue, 3 Mar 2026 14:33:09 +0100 Subject: [PATCH 7/7] fix(common): error on incomplete TLV header instead of silently skipping The loop condition `index+2 < payloadLength` required 3 bytes remaining, silently dropping trailing bytes when only 1 or 2 remained. Changed to iterate while `index < payloadLength` with an explicit 2-byte header check that returns a descriptive error. Co-Authored-By: Claude Opus 4.6 --- pkg/common/helpers.go | 11 +++++++---- pkg/common/helpers_test.go | 3 ++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/pkg/common/helpers.go b/pkg/common/helpers.go index f821d5e9..cb159457 100644 --- a/pkg/common/helpers.go +++ b/pkg/common/helpers.go @@ -158,11 +158,14 @@ func Decode(payloadHex *string, config *PayloadConfig) (any, error) { if len(config.Tags) != 0 { var index = 3 var payloadLength = len(payloadBytes) - for index+2 < payloadLength { + for index < payloadLength { + if payloadLength-index < 2 { + return nil, fmt.Errorf("incomplete TLV header at offset %d: need 2 bytes but only %d remain", index, payloadLength-index) + } + var tag = payloadBytes[index] - index++ - var length = int(payloadBytes[index]) - index++ + var length = int(payloadBytes[index+1]) + index += 2 if index+length > payloadLength { return nil, fmt.Errorf("TLV tag 0x%02x at offset %d declares length %d, but only %d bytes remain", tag, index-2, length, payloadLength-index) diff --git a/pkg/common/helpers_test.go b/pkg/common/helpers_test.go index cf6ef67b..fd086be3 100644 --- a/pkg/common/helpers_test.go +++ b/pkg/common/helpers_test.go @@ -125,10 +125,11 @@ func TestDecode(t *testing.T) { payload: "ffffff0000", config: tagConfig, expected: Port2Payload{ - Time: nil, + Time: Uint32Ptr(0), Power: nil, Sensor: nil, }, + expectedErr: "validation failed for Time", }, { payload: "ffffff000400000000",